Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors theme field definition access from a static export (FIELD_DEFS) to a dynamic generator (themeFields()), and updates consumers to use the new API. It also adjusts the ThemePanel sheet behavior/layout (non-modal sheet + width tweak).
Changes:
- Replaced
FIELD_DEFSimports/usages withthemeFields()across theme utilities and the ThemePanel. - Moved theme field definitions in
fields.tsinto athemeFields()function (and default export) instead of a static array. - Updated ThemePanel’s
Sheetto be non-modal and adjusted its responsive width class.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| resources/js/utils/theme.ts | Switches allowlist derivation to use themeFields() within applyThemeVars. |
| resources/js/fields.ts | Converts static field definitions into a themeFields() function and removes FIELD_DEFS. |
| resources/js/components/ThemePanel.vue | Updates field initialization/group derivation to use themeFields(); makes the sheet non-modal and tweaks width. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Apply only managed vars from the current mode — skip anything not in FIELD_DEFS | ||
| Object.entries(modeVars).forEach(([key, value]) => { | ||
| if (!MANAGED_VARS_SET.has(key)) return; | ||
| const vars = new Set(themeFieldsList.flatMap((f) => f.vars)); | ||
|
|
||
| if (!vars.has(key)) return; | ||
|
|
There was a problem hiding this comment.
vars is rebuilt inside the Object.entries(modeVars).forEach callback, which re-flattens themeFieldsList and allocates a new Set for every theme var. Compute the managed-vars Set once (outside the loop) and reuse it for the allowlist check. Also update the comment to no longer reference FIELD_DEFS since it was removed.
| Object.entries(lightVars).forEach(([key, value]) => { | ||
| if (!NON_COLOR_FIELD_VARS.has(key)) return; | ||
| const nonColorVars = new Set( | ||
| themeFieldsList.filter((f) => f.type !== 'color').flatMap((f) => f.vars), | ||
| ); | ||
|
|
||
| if (!nonColorVars.has(key)) return; | ||
|
|
There was a problem hiding this comment.
nonColorVars is recomputed inside the Object.entries(lightVars).forEach callback, which repeats the filter(...).flatMap(...) work for every entry. Build this Set once per applyThemeVars call (next to fontFieldVars) and reuse it inside the loop.
| const fields = reactive<FieldState[]>( | ||
| FIELD_DEFS.map((f) => ({ ...f, value: '' })), | ||
| themeFields().map((f) => ({ ...f, value: '' })), | ||
| ); | ||
|
|
||
| const uniqueGroups = FIELD_DEFS.filter((f) => f.group) | ||
| const uniqueGroups = themeFields().filter((f) => f.group) | ||
| .map((f) => f.group!) | ||
| .filter((g, i, arr) => arr.findIndex((x) => x.name === g.name) === i); |
There was a problem hiding this comment.
themeFields() is invoked twice during setup (for fields and uniqueGroups), which regenerates the full field definition list twice. Consider calling themeFields() once (e.g., const fieldDefs = themeFields()) and reusing it for both to avoid duplicate work and keep any per-call object instances consistent.
This pull request refactors how theme field definitions are accessed throughout the codebase by replacing the static
FIELD_DEFSarray with a newthemeFields()function. This change ensures that field definitions are generated dynamically when needed, improving maintainability and future extensibility. The update touches multiple files, including the main theme panel component and utility functions, to consistently use the new approach.Refactoring theme field definitions:
FIELD_DEFSarray with the newthemeFields()function inThemePanel.vueandtheme.ts, ensuring that field definitions are always retrieved dynamically. [1] [2] [3]fields.tsto exportthemeFieldsas a function that returns the field definitions, and removed the staticFIELD_DEFSexport. [1] [2] [3]Theme variable management improvements:
MANAGED_VARS_SET,NON_COLOR_FIELD_VARS,FONT_FIELD_VARS) to be dynamically derived from the result ofthemeFields(), ensuring that any future changes to field definitions are automatically reflected in theme variable logic. [1] [2] [3]Minor UI adjustment:
Sheetcomponent inThemePanel.vueto use:modal="false"and slightly changed its width class fromsm:w-110tosm:w-105for improved layout.