Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/custom preview #612

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

Feat/custom preview #612

wants to merge 13 commits into from

Conversation

SorsOps
Copy link
Member

@SorsOps SorsOps commented Dec 30, 2024

PR Type

Enhancement, Tests


Description

  • Introduced a new nodes-previews package for previewing nodes in the graph editor.
  • Added a Color Scale node with associated UI components and utilities.
  • Enhanced the graph editor to support dynamic tab loading and improved layout handling.
  • Updated import paths for various components to ensure compatibility and consistency.
  • Integrated preview nodes into the UI, including panel items and specifics.
  • Added smoke tests for the new nodes-previews package.
  • Configured ESLint, Prettier, TypeScript, and Vite for the nodes-previews package.
  • Added documentation and a license file for the nodes-previews package.

Changes walkthrough 📝

Relevant files
Enhancement
24 files
main.tsx
Added new theme styles and updated editor configuration   
+16/-2   
boolean.tsx
Updated import path for Checkbox component                             
+1/-1     
enumerated.tsx
Updated import path for Select component                                 
+1/-1     
editorTypes.ts
Added support for tabLoader in editor props                           
+3/-1     
layoutController.tsx
Introduced dynamic tab loading and updated layout handling
+88/-44 
useOpenPanel.tsx
Enhanced panel factory with group property                             
+2/-1     
specifics.tsx
Updated import path for Button component                                 
+1/-1     
color.tsx
Updated import path for Text component                                     
+1/-1     
variadicTokenSet.tsx
Updated import path for Stack component                                   
+1/-1     
file.tsx
Updated import path for Text component                                     
+1/-1     
index.ts
Added entry points for nodes and UI                                           
+2/-0     
colorScale.ts
Introduced Color Scale node definition                                     
+17/-0   
index.ts
Registered Color Scale node in system                                       
+9/-0     
index.tsx
Exported specifics for UI components                                         
+1/-0     
colorScale.tsx
Added Color Scale preview panel                                                   
+60/-0   
index.tsx
Added specifics for Color Scale preview                                   
+28/-0   
index.ts
Added utility functions for color handling                             
+16/-0   
data.tsx
Integrated preview specifics into editor data                       
+3/-1     
index.tsx
Updated data import path in editor component                         
+1/-1     
nodeTypes.tsx
Refactored node types to use flatten utility                         
+10/-10 
panelItems.tsx
Added preview nodes to panel items                                             
+18/-0   
page.tsx
Updated import path for Stack component                                   
+1/-1     
toast.tsx
Updated import path for Toast component                                   
+1/-1     
useToast.tsx
Updated import path for Toast component                                   
+1/-1     
Formatting
1 files
node.ts
Minor formatting adjustment in Node class                               
+0/-1     
Tests
1 files
smoke.test.ts
Added smoke test for nodes existence                                         
+8/-0     
Configuration changes
11 files
.dockerignore
Added Docker ignore file                                                                 
+4/-0     
.eslintignore
Added ESLint ignore file                                                                 
+11/-0   
.eslintrc.cjs
Added ESLint configuration                                                             
+6/-0     
.prettierignore
Added Prettier ignore file                                                             
+15/-0   
.prettierrc.json
Added Prettier configuration                                                         
+1/-0     
package.json
Added package.json for nodes-previews                                       
+60/-0   
tsconfig.docs.json
Added TypeScript configuration for documentation                 
+8/-0     
tsconfig.json
Added TypeScript configuration                                                     
+37/-0   
tsconfig.prod.json
Added production TypeScript configuration                               
+28/-0   
typedoc.json
Added Typedoc configuration                                                           
+11/-0   
vite.config.mts
Added Vite configuration for testing                                         
+9/-0     
Documentation
2 files
LICENCE
Added license file                                                                             
+373/-0 
readme.md
Added readme for nodes-previews                                                   
+3/-0     
Dependencies
1 files
package.json
Added dependency for graph-engine-nodes-preview                   
+2/-1     
Additional files
2 files
package.json +1/-0     
.DS_Store [link]   

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

Copy link

changeset-bot bot commented Dec 30, 2024

⚠️ No Changeset found

Latest commit: 0757c9b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@SorsOps SorsOps marked this pull request as draft December 30, 2024 14:32
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Theme Application

Ensure that the addition of the ts-theme-dark class and its associated styles do not conflict with existing themes or layouts in the graph editor.

<React.StrictMode>
  <div className="ts-theme-dark" id="graph-editor">
    <Editor panelItems={panelItems} />
Color Contrast Calculation

Validate the contrastingColor function to ensure it provides accurate contrast colors for all possible input values, especially edge cases.

function contrastingColor(value: string) {
	const black = new Color('srgb', [0, 0, 0]);
	const white = new Color('srgb', [1, 1, 1]);

	const background = new Color(value);
	const contrastBlack = Math.abs(background.contrast(black, 'APCA'));
	const contrastWhite = Math.abs(background.contrast(white, 'APCA'));

	if (contrastBlack > contrastWhite) {
		return '#000000';
	} else {
		return '#ffffff';
	}
Tab Loading Logic

Review the layoutLoader and loadTab functions for potential edge cases or performance issues when dynamically loading tabs.

const layoutLoader = (tab: TabBase): TabData => {
  const { id } = tab;
  switch (id) {
    case 'input':
      return {
        closable: true,
        cached: true,
        group: 'popout',
        id: 'input',
        title: 'Inputs',
        content: (
          <ErrorBoundary fallback={<ErrorBoundaryContent />}>
            <Inputsheet />
          </ErrorBoundary>
        ),
      };
    case 'outputs':
      return {
        closable: true,
        cached: true,
        group: 'popout',
        id: 'outputs',
        title: 'Outputs',
        content: (
          <ErrorBoundary fallback={<ErrorBoundaryContent />}>
            <OutputSheet />
          </ErrorBoundary>
        ),
      };

    case 'dropPanel':
      return {
        group: 'popout',
        id: 'dropPanel',
        title: 'Nodes',
        content: (
          <ErrorBoundary fallback={<ErrorBoundaryContent />}>
            <DropPanel />
          </ErrorBoundary>
        ),
        closable: true,
      };

    default:
      return tab as TabData;
  }
};

export const LayoutController = React.forwardRef<
  ImperativeEditorRef,
  EditorProps
>((props: EditorProps, ref) => {
  const {
    tabLoader,
    externalLoader,
    initialLayout,
    menuItems = defaultMenuDataFactory(),
  } = props;

  const registerDocker = useRegisterRef<DockLayout>('docker');
  const dispatch = useDispatch();

  const dockerRef = useSelector(dockerSelector) as MutableRefObject<DockLayout>;

  const loadTab = useCallback(
    (tab): TabData => {
      if (!tabLoader) {
        return layoutLoader(tab);
      }

      const loaded = tabLoader(tab);
      if (!loaded) {
        return layoutLoader(tab);
      }
      return loaded;
    },
    [tabLoader],
  );

Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Score
Possible issue
Add a unique key attribute to elements in a React map loop to avoid rendering issues

Add a unique key attribute to the div elements inside the inputs.scale.value.map
loop to prevent React rendering issues.

packages/nodes-previews/src/ui/panels/colorScale.tsx [29-51]

-{inputs.scale.value.map(color => {
+{inputs.scale.value.map((color, index) => {
   const hex = castToHex(color);
 
   return (
     <div
+      key={index}
       style={{
         display: 'grid',
         placeItems: 'center',
         width: '100%',
         minHeight: '100px',
         backgroundColor: hex
       }}
     >
       <Text
         style={{
           font: 'var(--font-body-xl)',
           color: contrastingColor(hex)
         }}
       >
         {hex}
       </Text>
     </div>
   );
 })}
Suggestion importance[1-10]: 9

Why: Adding a unique key attribute to elements in a React map loop is essential for preventing rendering issues and ensuring React's reconciliation process works correctly. This suggestion directly addresses a potential bug in the code.

9
Add fallback handling in the loadTab function to prevent runtime errors from undefined or invalid data

Ensure that the loadTab function handles cases where tabLoader or layoutLoader might
return undefined or invalid data to prevent runtime errors.

packages/graph-editor/src/editor/layoutController.tsx [322-334]

 const loadTab = useCallback(
   (tab): TabData => {
     if (!tabLoader) {
-      return layoutLoader(tab);
+      return layoutLoader(tab) || { id: tab.id, title: 'Unknown', content: <></> };
     }
 
     const loaded = tabLoader(tab);
-    if (!loaded) {
-      return layoutLoader(tab);
-    }
-    return loaded;
+    return loaded || layoutLoader(tab) || { id: tab.id, title: 'Unknown', content: <></> };
   },
   [tabLoader],
 );
Suggestion importance[1-10]: 8

Why: The suggestion adds robust fallback handling to the loadTab function, ensuring that undefined or invalid data does not cause runtime errors. This is critical for maintaining application stability and preventing crashes.

8
Add validation to the flatten function to handle undefined or null nodes arrays safely

Validate the nodes array before calling reduce in the flatten function to avoid
runtime errors when nodes is undefined or null.

packages/ui/src/components/editor/nodeTypes.tsx [7-11]

 const flatten = nodes =>
-  nodes.reduce((acc, node) => {
+  (nodes || []).reduce((acc, node) => {
     acc[node.type] = node;
     return acc;
   }, {});
Suggestion importance[1-10]: 8

Why: The validation ensures that the flatten function handles undefined or null nodes arrays gracefully, preventing potential runtime errors and improving code robustness.

8
General
Improve error handling in the castToHex function to provide better debugging and fallback behavior

Handle potential exceptions in the castToHex function more gracefully by logging or
providing a fallback value for debugging purposes.

packages/nodes-previews/src/utils/index.ts [10-14]

 export const castToHex = (col: Color) => {
   try {
     return toHex(toColor(col));
-  } catch {
-    return '';
+  } catch (error) {
+    console.error('Error converting color to hex:', error);
+    return '#000000'; // Fallback to black
   }
 };
Suggestion importance[1-10]: 7

Why: The improved error handling in the castToHex function enhances debugging by logging errors and provides a fallback value, which is useful for maintaining functionality in case of unexpected input.

7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants