-
Notifications
You must be signed in to change notification settings - Fork 78
feat(FR-1866): import custom theme from JSON #4948
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
base: feat/FR-1849-add-branding-page
Are you sure you want to change the base?
feat(FR-1866): import custom theme from JSON #4948
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has required the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
|---|---|---|---|
| 🔴 | Statements | 4.89% (-0.01% 🔻) |
589/12036 |
| 🔴 | Branches | 4.46% (-0.02% 🔻) |
377/8460 |
| 🔴 | Functions | 2.74% (-0.01% 🔻) |
101/3690 |
| 🔴 | Lines | 4.72% (-0.01% 🔻) |
555/11754 |
Show files with reduced coverage 🔻
St.❔ |
File | Statements | Branches | Functions | Lines |
|---|---|---|---|---|---|
| 🔴 | ... / ThemeColorPicker.tsx |
0% | 0% (-100% 🔻) |
0% | 0% |
Test suite run success
173 tests passing in 13 suites.
Report generated by 🧪jest coverage report action from adcdc8a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements a comprehensive theme customization feature that allows users to import and export custom themes via JSON configuration. The key addition is a Monaco editor-based modal with real-time JSON schema validation, replacing the previous read-only code editor implementation. The feature includes proper i18n support across all 21 supported languages.
Key Changes:
- Replaced read-only code editor with editable Monaco editor featuring JSON schema validation
- Added Import/Export functionality for theme JSON files
- Refactored theme config API to distinguish between incremental updates and full replacements
- Added comprehensive i18n translations for theme-related messages
Reviewed changes
Copilot reviewed 29 out of 30 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
react/src/components/BrandingSettingItems/ThemeJsonConfigModal.tsx |
Complete rewrite: Replaced BAICodeEditor with Monaco editor, added import functionality, JSON schema validation, and proper error handling structure |
react/src/components/BrandingSettingList.tsx |
Updated to use renamed API methods and wrapped modal with BAIUnmountAfterClose for proper cleanup |
react/src/helper/customThemeConfig.ts |
Refactored API: Added updateUserCustomThemeConfig for incremental updates while exposing setUserCustomThemeConfig for full replacement |
react/src/components/BrandingSettingItems/ThemeColorPicker.tsx |
Updated to use renamed updateUserCustomThemeConfig method |
react/src/components/BrandingSettingItems/LogoSizeSettingItem.tsx |
Updated to use renamed updateUserCustomThemeConfig method |
react/src/components/BrandingSettingItems/LogoPreviewer.tsx |
Updated to use renamed updateUserCustomThemeConfig method |
react/package.json |
Added @monaco-editor/react dependency and ESLint rule to enforce custom useThemeMode hook usage |
pnpm-lock.yaml |
Lock file updates for Monaco editor and its dependencies (monaco-editor, dompurify, marked) |
resources/i18n/*.json (21 files) |
Added 4 new translation keys for theme JSON configuration messages across all supported languages |
.cspell.json |
Added "inmemory" to dictionary for Monaco's schema URI prefix |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| return { | ||
| themeConfig, | ||
| userCustomThemeConfig: mergedThemeConfig, | ||
| setUserCustomThemeConfig: updateThemeConfig, | ||
| userCustomThemeConfig, | ||
| mergedThemeConfig, | ||
| updateUserCustomThemeConfig: updateThemeConfig, | ||
| setUserCustomThemeConfig, | ||
| getThemeValue, | ||
| }; |
Copilot
AI
Jan 6, 2026
•
edited by ironAiken2
Loading
edited by ironAiken2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API refactoring introduces both updateUserCustomThemeConfig and setUserCustomThemeConfig with different behaviors, which could confuse API consumers.
updateUserCustomThemeConfig performs incremental updates with merging logic and path-based updates, while setUserCustomThemeConfig directly replaces the entire config. Consider either:
- Making
setUserCustomThemeConfiginternal/private (prefix with underscore or keep it unexported for internal use only) - Adding clear JSDoc documentation explaining when to use each function
- Renaming to make the distinction clearer (e.g.,
replaceThemeConfigvsupdateThemeConfig)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comprehensive revision of useUserCustomThemeConfig appears necessary.
Modifications:
- Revised behaviour where theme.json values were used when userCustomThemeConfig values were absent > Modified to use antd token default values when absent
- Naming revisions
- Revised naming for updateThemeConfig (which modifies specific fields of UserCustomThemeConfig) and setUserCustomThemeConfig (which modifies the entire configuration)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hook has been modified. To allow another reviewer to verify, I will not resolve this comment. Please resolve it after another reviewer has checked.
Modification details:
1. Does not utilise the repository's theme.json value.
The userCustomThemeConfig stored in localStorage only clones the theme.json settings during initial load and subsequently does not use theme.json values. This is to directly display any side effects that occur when modifying the JSON content manually within the preview window. Instead, it uses the default token value from antd.
2. themeConfig is no longer provided.
themeConfig is no longer provided in this hook. If you require themeConfig, please use useCustomThemeConfig.
90ec72f to
636a47e
Compare
c637eaf to
d308329
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 30 out of 31 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
react/src/components/BrandingSettingItems/ThemeJsonConfigModal.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 30 out of 31 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
636a47e to
cfccc87
Compare
d308329 to
4111d3b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 31 out of 32 changed files in this pull request and generated 6 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
cfccc87 to
d108b4e
Compare
react/src/components/BrandingSettingItems/ThemeJsonConfigModal.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
4111d3b to
dca87a5
Compare
d108b4e to
706f979
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 31 out of 32 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
react/src/components/BrandingSettingItems/ThemeJsonConfigModal.tsx
Outdated
Show resolved
Hide resolved
nowgnuesLee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you resolve copilot review first?
706f979 to
d2dd78d
Compare
dca87a5 to
361958e
Compare
d2dd78d to
3f60b15
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 31 out of 32 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
react/src/components/BrandingSettingItems/ThemeJsonConfigModal.tsx
Outdated
Show resolved
Hide resolved
agatha197
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please resolve the copilot's reviews.
361958e to
5737794
Compare
3f60b15 to
adcdc8a
Compare

resolves #4949 (FR-1866)
Summary
theme.schema.jsonandantdThemeConfig.schema.jsonChanges
New Features
BrandingPage.tsx): New dedicated page for branding settings, moved from Configurations pageThemeJsonConfigModal.tsx):$refresolution for nested schemasTechnical Details
inmemory://model/prefix to match Monaco's internal path resolution for$refreferencesmonacoRefto access editor markers at save time for immediate validation feedbackChecklist: (if applicable)