refactor: refresh webview after update brand color

[spartan-vutrannguyen]

Description

• Refresh webview after updating brand color to get the latest setting

Related Issues

Type of Change

• Bug fix
• New feature
• Documentation update
• Release
• Refactor
• Other (please describe):

Testing

Screenshots (if applicable)

Screen.Recording.2025-03-24.at.08.21.45.mov

Additional Notes

---

Important

Refresh webview after brand color updates in ThemeManager and add onColorChangeEnd handlers in color components.

• Behavior:
  • Refresh webview after updating brand color in ThemeManager.update() when shouldSaveToConfig is true.
  • Execute JavaScript to set theme and refresh selected elements in webview.
• Components:
  • Add onColorChangeEnd handler in ColorPanel, ColorPalletGroup, and ColorPopover to trigger updates when color editing is finished.
  • Modify ColorPopover to save changes on close.
• Functions:
  • Update updateCustom() in StyleManager to accept domIds parameter for targeted updates.

^{This description was created by for c52bff7. It will automatically update as commits are pushed.}

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to c52bff7 in 3 minutes and 8 seconds

More details

• Looked at 231 lines of code in 5 files
• Skipped 0 files when reviewing.
• Skipped posting 17 drafted comments based on config settings.

1. apps/studio/src/lib/editor/engine/style/index.ts:36

• Draft comment:
  Good addition: updateCustom now accepts a domIds param. Ensure callers update accordingly.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

2. apps/studio/src/lib/editor/engine/theme/index.ts:338

• Draft comment:
  Ensure the false branch in update() is intentional. Only when shouldSaveToConfig is true the config and webviews are updated.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

3. apps/studio/src/routes/editor/LayersPanel/BrandTab/ColorPanel/ColorPalletGroup.tsx:46

• Draft comment:
  New onColorChangeEnd prop is wired correctly; nice clear separation of live vs. end update.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

4. apps/studio/src/routes/editor/LayersPanel/BrandTab/ColorPanel/ColorPopover.tsx:84

• Draft comment:
  Consider if ColorPickerContent's onChangeEnd should use a distinct handler rather than reusing onColorChange. This might improve clarity between live preview and final update.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

5. apps/studio/src/routes/editor/LayersPanel/BrandTab/ColorPanel/index.tsx:43

• Draft comment:
  Clear separation of onColorChange (live) and onColorChangeEnd (final update with config refresh) using shouldSaveToConfig is implemented. Verify this behavior meets design expectations.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

6. apps/studio/src/lib/editor/engine/style/index.ts:36

• Draft comment:
  Added a 'domIds' parameter to updateCustom. Ensure that all callers pass the appropriate DOM IDs if needed.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

7. apps/studio/src/lib/editor/engine/theme/index.ts:353

• Draft comment:
  In the update method, config is updated and webviews are refreshed only if 'shouldSaveToConfig' is true. Confirm that omitting any update when false is intentional, and consider whether using a fixed 500ms delay via setTimeout is robust for syncing UI changes.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

8. apps/studio/src/routes/editor/LayersPanel/BrandTab/ColorPanel/ColorPalletGroup.tsx:46

• Draft comment:
  The new 'onColorChangeEnd' prop has been added and handled. Verify that this callback correctly distinguishes between intermediate (onColorChange) and final (onColorChangeEnd) updates, both for existing and newly added colors.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

9. apps/studio/src/routes/editor/LayersPanel/BrandTab/ColorPanel/ColorPopover.tsx:35

• Draft comment:
  In ColorPopover, the onChange and onChangeEnd handlers for ColorPickerContent both use the same handleColorChange. Consider whether onChangeEnd should trigger a distinct final update (via onColorChangeEnd) rather than duplicating onColorChange.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

10. apps/studio/src/routes/editor/LayersPanel/BrandTab/ColorPanel/index.tsx:44

• Draft comment:
  The separation between immediate updates (using shouldSaveToConfig=false) and final updates (shouldSaveToConfig=true) in themeManager.update is clear. Verify that this behavior aligns with the intended UX so that final commits trigger the webview refresh as expected.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

11. apps/studio/src/routes/editor/LayersPanel/BrandTab/ColorPanel/index.tsx:85

• Draft comment:
  Note: The PR title and description mention refreshing the webview after updating brand color, yet the issue reference discusses onboarding docs. Please confirm that the correct issue is linked and that documentation updates (if any) are addressed elsewhere.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

12. apps/studio/src/lib/editor/engine/theme/index.ts:349

• Draft comment:
  The comment 'If is selected element, update the color in real-time' could be improved by rephrasing it to 'If the selected element, update the color in real-time' for better clarity.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
  The comment is about improving grammar in a code comment, but the code comment itself appears to be incomplete/placeholder since there's no implementation between it and the next section. The grammar improvement is minor and doesn't affect functionality. More importantly, the comment appears to be documenting unimplemented functionality.
  The grammar improvement would make the comment clearer. And technically this is about changed code since this comment is in new code.
  While the grammar improvement is valid, we should focus on substantive code issues rather than minor grammar fixes in placeholder comments. The comment itself may be removed or changed when the actual implementation is added.
  Delete this comment. Grammar fixes in placeholder comments are not important enough to warrant a PR comment.

13. apps/studio/src/lib/editor/engine/theme/index.ts:350

• Draft comment:
  The comment 'Base on the class name, find the styles to update' should be corrected to 'Based on the class name, find the styles to update' to fix the typographical error.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 30% <= threshold 50%
  This comment is pointing out a typographical error in a comment within the code. While it is a minor issue, correcting typographical errors can improve code readability and maintainability. However, it is not directly related to the functionality or logic of the code, which is typically the focus of pull request reviews. Given the rules, this comment might be considered purely informative, as it doesn't suggest a change that affects the code's behavior or logic.

14. apps/studio/src/routes/editor/LayersPanel/BrandTab/ColorPanel/ColorPalletGroup.tsx:57

• Draft comment:
  It appears there's an inconsistency in naming: the file is named 'ColorPalletGroup.tsx' while the component is called 'BrandPalletGroup'. Given that we are dealing with color palettes, consider renaming 'Pallet' to 'Palette' for clarity and consistency.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

15. apps/studio/src/routes/editor/LayersPanel/BrandTab/ColorPanel/ColorPalletGroup.tsx:74

• Draft comment:
  The variable name 'existedName' might be better expressed as 'existingNames' (or a similar term) to more clearly convey its meaning. This is a minor typographical improvement for clarity.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

16. apps/studio/src/routes/editor/LayersPanel/BrandTab/ColorPanel/ColorPopover.tsx:12

• Draft comment:
  Consider renaming the parameter 'existedName' to 'existingNames' (or a similar plural-friendly term) to better convey that it holds an array of existing names. This would improve clarity with minimal impact on functionality.
• Reason this comment was not posted:
  Comment was on unchanged code.

17. apps/studio/src/routes/editor/LayersPanel/BrandTab/ColorPanel/index.tsx:9

• Draft comment:
  Typographical suggestion: The identifier 'BrandPalletGroup' (and related file name './ColorPalletGroup') might be intended to refer to a 'Palette' rather than 'Pallet'. Consider verifying whether the intended spelling should be 'Palette' to avoid confusion.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_CnLctS6iF2OiwlGR

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.