Set failed node's output as red #11358
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces a new highlighting mechanism to indicate error states by rendering failed nodes in red, while allowing blue highlighting for other cases. The changes include new type definitions for node highlighting, refactoring components to use a "highlighting" prop instead of a boolean flag, and updating consumer components and stories to support the new behavior.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/twenty-ui/src/json-visualizer/types/JsonNodeHighlighting.ts | Introduces the JsonNodeHighlighting type with allowed values. |
| packages/twenty-ui/src/json-visualizer/types/GetJsonNodeHighlighting.ts | Defines the function type for obtaining node highlighting. |
| packages/twenty-ui/src/json-visualizer/index.ts | Exports the new types for use in the UI. |
| packages/twenty-ui/src/json-visualizer/contexts/JsonTreeContext.tsx | Replaces shouldHighlightNode with the new getNodeHightlighting property. |
| packages/twenty-ui/src/json-visualizer/components/internal/JsonNodeValue.tsx | Refactors styling to use a highlighting prop mapped to theme colors. |
| packages/twenty-ui/src/json-visualizer/components/internal/JsonNodeLabel.tsx | Refactors label styling to use a highlighting prop instead of a boolean. |
| packages/twenty-ui/src/json-visualizer/components/JsonValueNode.tsx | Updates component to leverage the highlighting prop. |
| packages/twenty-ui/src/json-visualizer/components/JsonTree.tsx | Propagates the new getNodeHightlighting property in the tree component. |
| packages/twenty-ui/src/json-visualizer/components/JsonNode.tsx | Integrates highlighting in rendering of various node types. |
| packages/twenty-ui/src/json-visualizer/stories/JsonTree.stories.tsx | Adds stories for both blue and red highlighting. |
| packages/twenty-front/src/modules/workflow/workflow-steps/components/WorkflowRunStepOutputDetail.tsx | Switches to using getNodeHightlighting for error cases in workflow output. |
| packages/twenty-front/src/modules/workflow/workflow-steps/components/WorkflowRunStepInputDetail.tsx | Uses getNodeHightlighting to highlight nodes based on conditionals. |
Comments suppressed due to low confidence (1)
packages/twenty-ui/src/json-visualizer/contexts/JsonTreeContext.tsx:7
- The property name 'getNodeHightlighting' appears to have a spelling mistake. Consider renaming it to 'getNodeHighlighting' for consistency with the 'JsonNodeHighlighting' type.
getNodeHightlighting?: GetJsonNodeHighlighting;
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
The PR implements red highlighting for workflow step outputs with errors, updating both workflow and JSON visualizer components to support a dynamic, color-specific highlighting mechanism.
• /packages/twenty-front/src/modules/workflow/workflow-steps/components/WorkflowRunStepOutputDetail.tsx now applies a red highlight when an error is detected.
• /packages/twenty-front/src/modules/workflow/workflow-steps/components/WorkflowRunStepInputDetail.tsx switches to using getNodeHightlighting for consistency.
• /packages/twenty-ui/src/json-visualizer/components/JsonNode.tsx and JsonValueNode.tsx now rely on a descriptive highlighting prop instead of a boolean.
• /packages/twenty-ui/src/json-visualizer/components/JsonTree.tsx and its context propagate getNodeHightlighting.
• New types in /packages/twenty-ui/src/json-visualizer/types/ ensure only 'red' or 'blue' can be used.
12 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile
| export const BlueHighlighting: Story = { | ||
| args: { | ||
| value: { | ||
| name: 'John Doe', | ||
| age: 30, | ||
| }, | ||
| getNodeHightlighting: () => 'blue', | ||
| }, |
There was a problem hiding this comment.
logic: Verify that 'getNodeHightlighting' is the intended prop name. It may be misspelled (possibly should be 'getNodeHighlighting') and must match the consuming component's API.
| getNodeHightlighting, | ||
| shouldExpandNodeInitially, |
There was a problem hiding this comment.
logic: Check spelling: 'getNodeHightlighting' may be intended as 'getNodeHighlighting'. Ensure consistent usage across the codebase.
| getNodeHightlighting, | |
| shouldExpandNodeInitially, | |
| getNodeHighlighting, | |
| shouldExpandNodeInitially, |
| import { createContext } from 'react'; | ||
|
|
||
| export type ShouldExpandNodeInitiallyProps = { keyPath: string; depth: number }; | ||
|
|
||
| export type JsonTreeContextType = { | ||
| shouldHighlightNode?: (keyPath: string) => boolean; | ||
| getNodeHightlighting?: GetJsonNodeHighlighting; |
There was a problem hiding this comment.
syntax: Typo: 'getNodeHightlighting' likely should be 'getNodeHighlighting'.
| getNodeHightlighting?: GetJsonNodeHighlighting; | |
| getNodeHighlighting?: GetJsonNodeHighlighting; |
Closes twentyhq/core-team-issues#716