System default dark mode#92
Conversation
Signed-off-by: kumaradityaraj <sedulous.0007@gmail.com>
kumaradityaraj
requested review from
fantonangeli,
handreyrc and
lornakelly
and removed request for
Copilot and
handreyrc
|
@kumaradityaraj Thanks for the PR. Looks like this only handles the first part of ticket for system default but the second part needs to be done "implement an attribute in the component to override this behaviour and force a specific theme" |
Signed-off-by: kumaradityaraj <sedulous.0007@gmail.com>
There was a problem hiding this comment.
Pull request overview
Implements theme switching for the diagram editor by wiring React Flow’s colorMode to a new state (defaulting to system), and removes the previous hardcoded diagram background styling.
Changes:
- Add
colorMode="system"support to@xyflow/reactReactFlowvia component state. - Add a UI
<select>to manually switch betweendark,light, andsystem. - Remove
.diagram-backgroundstyling from the diagram CSS; add a test-run artifact file.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx | Adds colorMode state, passes it to ReactFlow, and introduces a theme dropdown UI. |
| packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.css | Removes background styling previously applied via .diagram-background. |
| packages/serverless-workflow-diagram-editor/test-results/.last-run.json | Adds last test run result artifact. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <RF.Panel position="top-right"> | ||
| <select className="xy-theme__select" onChange={onChange} data-testid="colormode-select"> | ||
| <option value="dark">dark</option> | ||
| <option value="light">light</option> | ||
| <option value="system">system</option> | ||
| </select> | ||
| </RF.Panel> |
There was a problem hiding this comment.
Issue #35’s proposed implementation calls for an attribute/prop on the component to override system theme. The current approach hardcodes a theme dropdown into Diagram, which forces extra UI into all consumers and doesn’t provide a clean API for host apps (e.g., VS Code/web) to control theme. Consider moving the override to a prop (e.g., colorMode?: RF.ColorMode defaulting to "system") and keeping any manual toggle only in Storybook/demo code if needed.
| <select className="xy-theme__select" onChange={onChange} data-testid="colormode-select"> | ||
| <option value="dark">dark</option> | ||
| <option value="light">light</option> | ||
| <option value="system">system</option> | ||
| </select> |
There was a problem hiding this comment.
The theme select is uncontrolled, so it will display the first option ("dark") even though colorMode is initialized to "system". This causes the UI to be out of sync with the actual colorMode passed to ReactFlow. Make the <select> controlled (e.g., bind value to colorMode or set defaultValue appropriately) so the initial selection matches the state.
| <select className="xy-theme__select" onChange={onChange} data-testid="colormode-select"> | ||
| <option value="dark">dark</option> | ||
| <option value="light">light</option> | ||
| <option value="system">system</option> | ||
| </select> |
There was a problem hiding this comment.
The <select> used to switch color mode has no accessible name (no <label> and no aria-label/aria-labelledby). This will be flagged by accessibility tooling and makes the control hard to use with screen readers. Add an associated label or an aria label.
| @@ -63,6 +64,9 @@ export const Diagram = ({ divRef, ref }: DiagramProps) => { | |||
| (changes) => setEdges((edgesSnapshot) => RF.applyEdgeChanges(changes, edgesSnapshot)), | |||
| [], | |||
| ); | |||
| const onChange: React.ChangeEventHandler<HTMLSelectElement> = (evt) => { | |||
| setColorMode(evt.target.value as RF.ColorMode); | |||
| }; | |||
|
|
|||
| React.useImperativeHandle( | |||
| ref, | |||
| @@ -89,7 +93,15 @@ export const Diagram = ({ divRef, ref }: DiagramProps) => { | |||
| preventScrolling={true} | |||
| selectionOnDrag={true} | |||
| fitView | |||
| colorMode={colorMode} | |||
| > | |||
| <RF.Panel position="top-right"> | |||
| <select className="xy-theme__select" onChange={onChange} data-testid="colormode-select"> | |||
| <option value="dark">dark</option> | |||
| <option value="light">light</option> | |||
| <option value="system">system</option> | |||
| </select> | |||
There was a problem hiding this comment.
This change introduces new theme-selection behavior (state + UI + colorMode prop) but there are no tests asserting the default mode (system) and that switching the select updates the rendered theme/config. Since this component already has a unit test, please extend it to cover the new color mode control.
| @@ -19,7 +19,3 @@ | |||
| position: relative; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
The .diagram-background styles were removed from this stylesheet, but Diagram.tsx still renders <RF.Background className="diagram-background" ... />. With no matching CSS rule, the class is now a no-op and the intended background styling likely got lost. Either restore the .diagram-background rule (ideally using theme-aware CSS variables) or remove the className usage.
| .diagram-background { | |
| background: var(--diagram-background-color, #ffffff); | |
| color: var(--diagram-background-pattern-color, #e2e8f0); | |
| } |
| { | ||
| "status": "passed", | ||
| "failedTests": [] | ||
| } |
There was a problem hiding this comment.
test-results/.last-run.json looks like an auto-generated test artifact and typically shouldn't be committed (it will cause noisy diffs and can vary per machine/CI run). Consider removing it from the PR and adding test-results/ and/or .last-run.json to .gitignore if it is generated by your test runner.
| { | |
| "status": "passed", | |
| "failedTests": [] | |
| } |
|
@fantonangeli |
Closes Add support for dark and light theme
This feature implements dark mode by using system current status.
Screen.Recording.2026-04-15.at.2.27.00.PM.mov