fix(ui-scripts,emotion): fix primitives and semantics override functionality#2495
Open
fix(ui-scripts,emotion): fix primitives and semantics override functionality#2495
Conversation
HerrTopi
changed the title
|
HerrTopi
force-pushed
the
fix-theme-overrides
branch
from
4e09919 to
348cc7a
Compare
note: its not complete, the WithStyleProps still has wrong types in most v2 components
…ides and add freeze functionlionality to old components
matyasf
force-pushed
the
fix-theme-overrides
branch
from
348cc7a to
7b25cc6
Compare
matyasf
reviewed
Apr 8, 2026
| </InstUISettingsProvider> | ||
| ``` | ||
|
|
||
| ### 8. Per-component `themeOverride` prop |
Collaborator
There was a problem hiding this comment.
I'd rename this feature to "Componet's themeOverride prop". Now its unclear what this refers to, first I thought that its about the component level override in InstUISettingsProvider
matyasf
reviewed
Apr 8, 2026
Collaborator
matyasf
left a comment
matyasf
left a comment
There was a problem hiding this comment.
ui-themes will need a new folder structure, its a chaos now
Comment on lines
+54
to
+55
| // ThemeOverrideProp is the old type, ThemeOverrideValue is the new one | ||
| themeOverride?: ThemeOverrideProp['themeOverride'] | ThemeOverrideValue, |
Collaborator
There was a problem hiding this comment.
If this is legacy why does it handle the new themes?
Am I right that code that handles new themes needs to be deleted from this file?
|
|
||
| import { useTheme } from './useTheme' | ||
| import { getComponentThemeOverride } from './getComponentThemeOverride' | ||
| // TODO: rework useStyle to use the new getComponentThemeOverride signature |
Collaborator
There was a problem hiding this comment.
and to support frozen themes
Comment on lines
-234
to
-237
| // passing themeOverrides is needed for components like Button | ||
| // that have no makeStyles of their own and only pass themeOverrides | ||
| // to the underlying component (e.g.: BaseButton) | ||
| themeOverride={themeOverride} |
Collaborator
There was a problem hiding this comment.
Deleting this might be an issue. As the comment says, how do we handle the case in Button?
| // if a new theme has been added to the lib since this component version has been frozen, it can't be used with this | ||
| // theme, so we throw an error. Solution: upgrade to the latest version, it will support it | ||
| if (frozenTheme && !frozenTheme[themeKey]) { | ||
| throw new Error( |
Collaborator
There was a problem hiding this comment.
This crashes the whole React render tree. I'd just emit an error to the console.
| **/ | ||
| @withDeterministicId() | ||
| @withStyle(generateStyle) | ||
| @withStyle(generateStyle, null, frozenThemes) |
Collaborator
There was a problem hiding this comment.
I like this solution, its apparent that something is different
Comment on lines
+87
to
+92
| ...(hasShadow && | ||
| { | ||
| // boxShadow: boxShadowObjectsToCSSString( | ||
| // sharedTokens.boxShadow.elevation4 | ||
| // ) | ||
| }) |
Collaborator
There was a problem hiding this comment.
I guess this is removed unintentionally?
Comment on lines
+51
to
+57
| "./v11_7": { | ||
| "src": "./src/themes/frozenThemes/v11_7/index.ts", | ||
| "types": "./types/themes/frozenThemes/v11_7/index.d.ts", | ||
| "import": "./es/themes/frozenThemes/v11_7/index.js", | ||
| "require": "./lib/themes/frozenThemes/v11_7/index.js", | ||
| "default": "./es/themes/frozenThemes/v11_7/index.js" | ||
| } |
Collaborator
There was a problem hiding this comment.
I like this generally, good idea
Comment on lines
+30
to
+35
| export default { | ||
| canvas: legacyCanvas, | ||
| 'canvas-high-contrast': legacyCanvasHighContrast, | ||
| light, | ||
| dark | ||
| } |
Collaborator
There was a problem hiding this comment.
Its a bit weird, that we always export the same stuff from /vXY and / except here.
Its OK that we don't export legacy themes, but I would export these the same way as the main index file:
export {
dark,
light,
canvas,
canvasHighContrast
}
export default canvas
boxShadowObjectsToCSSString would also be better in some of our utility packages, but that might create lots of extra dependencies, so lets leave it for now here.
matyasf
reviewed
Apr 9, 2026
There was a problem hiding this comment.
Pull request overview
This PR migrates the “newTheme” token pipeline from static objects to lazy recomputation functions (semantics(primitives) → components(semantics) → sharedTokens(semantics)), adds version-pinned “frozen” theme snapshots for backward compatibility, and updates the Emotion theming utilities/components to support the new themeOverride override mechanism.
Changes:
- Updated
withStyle/useStyle/getTheme/InstUISettingsProviderto recompute primitives/semantics/component tokens and apply overrides viathemeOverride. - Added frozen theme snapshots and a new
@instructure/ui-themes/v11_7export for version-pinned themes. - Updated many component style generators and tests to use
ReturnType<NewComponentTypes[...](themes are now functions) and updated docs to describe new override patterns.
Reviewed changes
Copilot reviewed 147 out of 148 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ui-view/src/View/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-view/src/ContextView/v2/styles.ts | Update theme param type to function return type |
| packages/ui-truncate-text/src/TruncateText/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-tree-browser/src/TreeBrowser/v2/TreeCollection/styles.ts | Update componentTheme type to function return type |
| packages/ui-tree-browser/src/TreeBrowser/v2/TreeButton/styles.ts | Update componentTheme type to function return type |
| packages/ui-tree-browser/src/TreeBrowser/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-tray/src/Tray/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-tooltip/src/Tooltip/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-toggle-details/src/ToggleGroup/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-toggle-details/src/ToggleDetails/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-themes/src/themes/frozenThemes/v11_7/light/sharedTokens.ts | Add frozen v11_7 light sharedTokens generator |
| packages/ui-themes/src/themes/frozenThemes/v11_7/light/primitives.ts | Add frozen v11_7 light primitives snapshot |
| packages/ui-themes/src/themes/frozenThemes/v11_7/light/index.ts | Add frozen v11_7 light theme entry (primitives/semantics/components/sharedTokens) |
| packages/ui-themes/src/themes/frozenThemes/v11_7/light/components/alert.ts | Add frozen v11_7 light Alert component tokens generator |
| packages/ui-themes/src/themes/frozenThemes/v11_7/legacyCanvasHighContrast/sharedTokens.ts | Add frozen v11_7 legacyCanvasHighContrast sharedTokens generator |
| packages/ui-themes/src/themes/frozenThemes/v11_7/legacyCanvasHighContrast/primitives.ts | Add frozen v11_7 legacyCanvasHighContrast primitives snapshot |
| packages/ui-themes/src/themes/frozenThemes/v11_7/legacyCanvasHighContrast/index.ts | Add frozen v11_7 legacyCanvasHighContrast theme entry |
| packages/ui-themes/src/themes/frozenThemes/v11_7/legacyCanvasHighContrast/components/alert.ts | Add frozen v11_7 legacyCanvasHighContrast Alert tokens generator |
| packages/ui-themes/src/themes/frozenThemes/v11_7/legacyCanvas/sharedTokens.ts | Add frozen v11_7 legacyCanvas sharedTokens generator |
| packages/ui-themes/src/themes/frozenThemes/v11_7/legacyCanvas/primitives.ts | Add frozen v11_7 legacyCanvas primitives snapshot |
| packages/ui-themes/src/themes/frozenThemes/v11_7/legacyCanvas/index.ts | Add frozen v11_7 legacyCanvas theme entry |
| packages/ui-themes/src/themes/frozenThemes/v11_7/legacyCanvas/components/alert.ts | Add frozen v11_7 legacyCanvas Alert tokens generator |
| packages/ui-themes/src/themes/frozenThemes/v11_7/index.ts | Add frozen v11_7 theme bundle export (canvas / light / dark, etc.) |
| packages/ui-themes/src/themes/frozenThemes/v11_7/dark/sharedTokens.ts | Add frozen v11_7 dark sharedTokens generator |
| packages/ui-themes/src/themes/frozenThemes/v11_7/dark/primitives.ts | Add frozen v11_7 dark primitives snapshot |
| packages/ui-themes/src/themes/frozenThemes/v11_7/dark/index.ts | Add frozen v11_7 dark theme entry |
| packages/ui-themes/src/themes/frozenThemes/v11_7/dark/components/alert.ts | Add frozen v11_7 dark Alert tokens generator |
| packages/ui-themes/src/themes/frozenThemes/v11_7/componentTypes/index.ts | Add generated component-types module for v11_7 frozen themes |
| packages/ui-themes/src/themes/frozenThemes/v11_7/componentTypes/alert.ts | Add Alert component token type for v11_7 frozen themes |
| packages/ui-themes/src/themes/frozenThemes/v11_7/commonTypes.ts | Add v11_7 frozen-theme common types (SharedTokens/BaseTheme) |
| packages/ui-themes/package.json | Add ./v11_7 export entry for frozen themes |
| packages/ui-text/src/Text/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-text-input/src/TextInput/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-text-area/src/TextArea/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-tag/src/Tag/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-tabs/src/Tabs/v2/Tab/styles.ts | Update componentTheme type to function return type |
| packages/ui-tabs/src/Tabs/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-tabs/src/Tabs/v2/Panel/styles.ts | Update componentTheme type to function return type |
| packages/ui-table/src/Table/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-table/src/Table/v2/RowHeader/styles.ts | Update componentTheme type to function return type |
| packages/ui-table/src/Table/v2/Row/styles.ts | Update componentTheme type to function return type |
| packages/ui-table/src/Table/v2/Head/styles.ts | Update componentTheme type to function return type |
| packages/ui-table/src/Table/v2/ColHeader/styles.ts | Update componentTheme type to function return type |
| packages/ui-table/src/Table/v2/Cell/styles.ts | Update componentTheme type to function return type |
| packages/ui-table/src/Table/v2/Body/styles.ts | Update componentTheme type to function return type |
| packages/ui-spinner/src/Spinner/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-source-code-editor/src/SourceCodeEditor/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-side-nav-bar/src/SideNavBar/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-side-nav-bar/src/SideNavBar/v2/SideNavBarItem/styles.ts | Update componentTheme type to function return type |
| packages/ui-scripts/lib/build/buildThemes/setupThemes.js | Update theme codegen to emit semantics/components/sharedTokens as functions |
| packages/ui-rating/src/RatingIcon/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-range-input/src/RangeInput/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-radio-input/src/RadioInput/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-progress/src/ProgressCircle/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-progress/src/ProgressBar/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-popover/src/Popover/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-pill/src/Pill/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-pagination/src/Pagination/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-pagination/src/Pagination/v2/PaginationPageInput/styles.ts | Update componentTheme type to function return type |
| packages/ui-overlays/src/Mask/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-options/src/Options/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-options/src/Options/v2/Separator/styles.ts | Update componentTheme type to function return type |
| packages/ui-options/src/Options/v2/Item/styles.ts | Update componentTheme type to function return type |
| packages/ui-number-input/src/NumberInput/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-navigation/src/AppNav/v2/styles.ts | Update componentTheme type to function return type (formatting adjusted) |
| packages/ui-navigation/src/AppNav/v2/Item/styles.ts | Update componentTheme type to function return type |
| packages/ui-modal/src/Modal/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-modal/src/Modal/v2/ModalHeader/styles.ts | Update componentTheme type to function return type |
| packages/ui-modal/src/Modal/v2/ModalHeader/tests/ModalHeader.test.tsx | Update tests to call component theme generators with semantics(primitives) |
| packages/ui-modal/src/Modal/v2/ModalFooter/styles.ts | Update componentTheme type to function return type |
| packages/ui-modal/src/Modal/v2/ModalFooter/tests/ModalFooter.test.tsx | Update tests to call component theme generators with semantics(primitives) |
| packages/ui-modal/src/Modal/v2/ModalBody/styles.ts | Update componentTheme type to function return type |
| packages/ui-modal/src/Modal/v2/ModalBody/tests/ModalBody.test.tsx | Update tests to call component theme generators with semantics(primitives) |
| packages/ui-metric/src/Metric/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-menu/src/Menu/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-menu/src/Menu/v2/MenuItemSeparator/styles.ts | Update componentTheme type to function return type |
| packages/ui-menu/src/Menu/v2/MenuItemGroup/styles.ts | Update componentTheme type to function return type |
| packages/ui-menu/src/Menu/v2/MenuItem/styles.ts | Update componentTheme type to function return type |
| packages/ui-list/src/List/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-list/src/List/v2/ListItem/styles.ts | Update componentTheme type to function return type |
| packages/ui-list/src/InlineList/v2/InlineListItem/styles.ts | Update componentTheme type to function return type |
| packages/ui-link/src/Link/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-instructure/src/NutritionFacts/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-instructure/src/DataPermissionLevels/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-instructure/src/AiInformation/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-icons/src/styles.ts | Update componentTheme type to function return type in helpers/generateStyle |
| packages/ui-heading/src/Heading/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-grid/src/GridRow/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-grid/src/GridCol/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-grid/src/Grid/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-form-field/src/FormFieldMessage/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-form-field/src/FormFieldLayout/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-flex/src/Flex/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-file-drop/src/FileDrop/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-drilldown/src/Drilldown/v2/styles.ts | Update componentTheme type to function return type (and param name in comment) |
| packages/ui-drawer-layout/src/DrawerLayout/v2/DrawerTray/styles.ts | Update componentTheme type to function return type |
| packages/ui-drawer-layout/src/DrawerLayout/v2/DrawerContent/styles.ts | Update componentTheme type to function return type |
| packages/ui-color-picker/src/ColorPreset/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-color-picker/src/ColorPicker/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-color-picker/src/ColorMixer/v2/Slider/styles.ts | Update componentTheme type to function return type |
| packages/ui-color-picker/src/ColorMixer/v2/RGBAInput/styles.ts | Update componentTheme type to function return type |
| packages/ui-color-picker/src/ColorMixer/v2/ColorPalette/styles.ts | Update componentTheme type to function return type |
| packages/ui-color-picker/src/ColorIndicator/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-color-picker/src/ColorContrast/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-checkbox/src/Checkbox/v2/ToggleFacade/styles.ts | Update componentTheme type to function return type |
| packages/ui-checkbox/src/Checkbox/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-checkbox/src/Checkbox/v2/CheckboxFacade/styles.ts | Update componentTheme type to function return type |
| packages/ui-calendar/src/Calendar/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-calendar/src/Calendar/v2/Day/styles.ts | Update componentTheme type to function return type |
| packages/ui-byline/src/Byline/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-buttons/src/CloseButton/v2/styles.ts | Update BaseButton theme type reference and minor object formatting |
| packages/ui-buttons/src/BaseButton/v2/styles.ts | Update componentTheme type to function return type and minor formatting |
| packages/ui-breadcrumb/src/Breadcrumb/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-breadcrumb/src/Breadcrumb/v2/props.ts | Update WithStyleProps theme param type to function return type |
| packages/ui-billboard/src/Billboard/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-badge/src/Badge/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-avatar/src/Avatar/v2/styles.ts | Update componentTheme type to function return type |
| packages/ui-alerts/src/Alert/v2/styles.ts | Update Alert theme type usage and adjust shadow logic |
| packages/ui-alerts/src/Alert/v2/README.md | Update docs example to demonstrate overrides |
| packages/ui-alerts/src/Alert/v2/index.tsx | Use frozen theme bundle with withStyle for version-pinning |
| packages/shared-types/src/BaseTheme.ts | Add themeOverride field on BaseTheme |
| packages/emotion/src/withStyleLegacy.tsx | Route legacy path to getComponentThemeOverrideLegacy |
| packages/emotion/src/withStyle.tsx | Implement recomputation + themeOverride handling and frozen theme support |
| packages/emotion/src/useTheme.ts | Minor formatting change |
| packages/emotion/src/useStyleLegacy.ts | Route legacy path to getComponentThemeOverrideLegacy |
| packages/emotion/src/useStyle.ts | Partial migration toward new override mechanism (still TODO) |
| packages/emotion/src/InstUISettingsProvider/index.tsx | Add themeOverride prop and pass through to getTheme |
| packages/emotion/src/index.ts | Export legacy override helper under new name |
| packages/emotion/src/getTheme.ts | Extend theme resolution to merge new themeOverride separately from legacy overrides |
| packages/emotion/src/getComponentThemeOverrideLegacy.ts | Add legacy override resolution helper (new file) |
| packages/emotion/src/getComponentThemeOverride.ts | Change semantics of getComponentThemeOverride to resolve per-component overrides only |
| packages/emotion/src/EmotionTypes.ts | Rename ThemeOrOverride → ThemeOrLegacyOverride |
| packages/emotion/src/tests/useTheme.test.tsx | Update test typing after ThemeOrLegacyOverride rename |
| packages/emotion/src/tests/getComponentThemeOverride.test.tsx | Point test to legacy override resolver |
| packages/docs/src/withStyleForDocs.tsx | Update docs build to use legacy override resolver export name |
| packages/docs/src/Preview/props.ts | Remove unused themes prop wiring |
| packages/docs/src/Preview/index.tsx | Update preview to pull themes from @instructure/ui-themes exports |
| packages/docs/src/Playground/index.tsx | Remove themes threading into Preview |
| packages/docs/src/Document/index.tsx | Cast componentTheme for updated theme typing |
| packages/docs/src/App/index.tsx | Minor whitespace adjustment |
| packages/docs/buildScripts/utils/buildVersionMap.mts | Skip packages without expected export file; tweak error string formatting |
| docs/guides/using-theme-overrides.md | Update guide content to show new override usage (needs cleanup) |
| docs/guides/new-theme-overrides.md | Add new guide describing v11.7+ override patterns |
Comments suppressed due to low confidence (2)
docs/guides/using-theme-overrides.md:149
- This example section now nests two
InstUISettingsProviderblocks back-to-back with effectively the samethemeOverride, and the first one contains an emptyPill: {}override. This makes the guide confusing and harder to follow (and the surrounding text still describescomponentOverrides, notthemeOverride). Consider removing the redundant provider/empty override and aligning the narrative with the actual API being demonstrated.
<InstUISettingsProvider
themeOverride={{
components:{
Alert: {
infoIconBackground: "darkblue",
infoBorderColor: "darkblue"
},
Pill:{
}
}
}}
>
<InstUISettingsProvider
themeOverride={{
components:{
Alert: {
infoIconBackground: "darkblue",
infoBorderColor: "darkblue"
},
Pill:{
infoTextColor:"red"
}
}
}}
>
packages/ui-themes/src/themes/frozenThemes/v11_7/componentTypes/alert.ts:51
- This file declares
Alertas a type alias but also doesexport default Alert. Exporting a type as a runtime value will fail TypeScript compilation (especially withisolatedModules). Remove the default export and use only named type exports (e.g.export type Alert = ...).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+262
to
+266
| accentSea: semantics.color.stroke.accent.sea, | ||
| accentSky: semantics.color.stroke.accent.sky, | ||
| accentStone: semantics.color.background.accent.stone, | ||
| accentViolet: semantics.color.background.accent.violet, | ||
| visualSeparator: semantics.color.background.container |
There was a problem hiding this comment.
accentStone and accentViolet under stroke are mapped to semantics.color.background.accent.* instead of semantics.color.stroke.accent.*. This makes stroke accent tokens follow background accents and is inconsistent with other themes (e.g. legacyCanvas uses semantics.color.stroke.accent.stone/violet). Switch these assignments to semantics.color.stroke.accent.stone and .violet.
Comment on lines
+262
to
+266
| accentSea: semantics.color.stroke.accent.sea, | ||
| accentSky: semantics.color.stroke.accent.sky, | ||
| accentStone: semantics.color.background.accent.stone, | ||
| accentViolet: semantics.color.background.accent.violet, | ||
| visualSeparator: semantics.color.background.container |
There was a problem hiding this comment.
accentStone and accentViolet under stroke are mapped to semantics.color.background.accent.* instead of semantics.color.stroke.accent.*. This breaks the meaning of stroke accent tokens and diverges from other themes (e.g. legacyCanvas uses semantics.color.stroke.accent.stone/violet). Map these to semantics.color.stroke.accent.stone / .violet.
Comment on lines
+25
to
+32
| import type Alert from './alert' | ||
|
|
||
| type ComponentTypes = { | ||
| Alert: (semantics: any) => Alert | ||
| } | ||
|
|
||
| export type { ComponentTypes } | ||
| export default ComponentTypes |
There was a problem hiding this comment.
ComponentTypes is declared as a type alias, but the file also does export default ComponentTypes. Exporting a type as a value will fail TypeScript compilation (especially with isolatedModules). Remove the default export, or change ComponentTypes to a value (e.g. an interface exported as type-only) and only export types from this module.
Comment on lines
84
to
+92
| borderStyle: componentTheme.borderStyle, | ||
| borderRadius: componentTheme.borderRadius, | ||
| ...variantStyles[variant!].alert, | ||
| ...(hasShadow && { | ||
| boxShadow: boxShadowObjectsToCSSString( | ||
| sharedTokens.boxShadow.elevation4 | ||
| ) | ||
| }) | ||
| ...(hasShadow && | ||
| { | ||
| // boxShadow: boxShadowObjectsToCSSString( | ||
| // sharedTokens.boxShadow.elevation4 | ||
| // ) | ||
| }) |
There was a problem hiding this comment.
The hasShadow branch no longer sets any boxShadow (the previous implementation is commented out), so hasShadow becomes a no-op while still being read from props. Either restore the boxShadow calculation (e.g. via boxShadowObjectsToCSSString(sharedTokens.boxShadow.elevation4) and keep sharedTokens as a parameter) or remove the prop/branch if shadow support is being dropped.
Comment on lines
+17
to
+23
| <InstUISettingsProvider theme={{ | ||
| componentOverrides: { | ||
| "Alert": { | ||
| dangerIconBackground: 'purple' | ||
| } | ||
| } | ||
| }}> |
There was a problem hiding this comment.
This example uses theme={{ componentOverrides: { Alert: ... }}}, but v11.7+ components styled via the updated withStyle read provider overrides from InstUISettingsProvider's themeOverride prop (and/or theme.themeOverride), not from legacy componentOverrides. As written, the override likely won't apply to this Alert. Update the example to use themeOverride={{ components: { Alert: { ... }}}} (or a per-component themeOverride prop).
Suggested change
| <InstUISettingsProvider theme={{ | |
| componentOverrides: { | |
| "Alert": { | |
| dangerIconBackground: 'purple' | |
| } | |
| } | |
| }}> | |
| <InstUISettingsProvider themeOverride={{ | |
| components: { | |
| Alert: { | |
| dangerIconBackground: 'purple' | |
| } | |
| } | |
| }}> |
Comment on lines
204
to
208
| type BaseTheme = { | ||
| key: string | ||
| description?: string | ||
| themeOverride?: { primitives: any; semantics: any } | ||
| } & BaseThemeVariables |
There was a problem hiding this comment.
BaseTheme.themeOverride is typed as { primitives: any; semantics: any }, but the new override flow in withStyle.tsx also expects themeOverride.components (and potentially other sections). This mismatch is already forcing @ts-ignore usage. Update the type to reflect the actual supported shape (e.g. optional primitives, semantics, components, and potentially sharedTokens).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The PR converts the theme system from static token objects to lazy functions (semantics(primitives), components(semantics), sharedTokens(semantics)), enabling override-by-recomputation. It also
introduces frozen themes so older component version's themes stay pinned to a snapshot.
This solution stores the primitives, semantics and the affected components, for example
packages/ui-themes/src/themes/frozenThemes/v11_7/dark/primitives.tsThis is just an example, it assumes that there will be a
v3Alertin the releaseNote that when one overrides things in
InstUISettingsProviderthat were deleted or changed things can break, we should not recommend this.whats missing:
sharedTokensoverride is not solved in withStyle, it also needs examplesuseStyleis not completedThe task is an architectural review, this should not be merged now