Skip to content

Better types for themes, small fixes#2487

Open
matyasf wants to merge 2 commits intomasterfrom
allow_semantic_override
Open

Better types for themes, small fixes#2487
matyasf wants to merge 2 commits intomasterfrom
allow_semantic_override

Conversation

@matyasf
Copy link
Copy Markdown
Collaborator

@matyasf matyasf commented Mar 24, 2026

  • Fix types for the themeOverride prop. Until now it was using theme types from the old engine.
  • Delete TextInput/v2/theme.ts, it was unused

To test:
Check if theme overrides are typed correctly for v1 and v2 components, e.g.

<Button themeOverride={{aiActiveTextColor: 'red'}}>aa</Button> // v2 Button, should show no TS error
const a = <Button themeOverride={{aiActiveBoxShadow: 'red'}}>asd</Button>  // v1 Button, should show no TS error

// v2 Avatars
const a = <Avatar name='a'
                  themeOverride={{ashBackgroundColor: 'red'}} />
const b = <Avatar name='a'
                  themeOverride={(_compTheme, theme) => {
                    return {ashTextColor: theme.newTheme.primitives.colors.green.green10}
                  }} />

@matyasf matyasf self-assigned this Mar 24, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 24, 2026

PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://instructure.design/pr-preview/pr-2487/

Built to branch gh-pages at 2026-04-28 19:41 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@matyasf matyasf requested a review from HerrTopi March 26, 2026 10:03
@matyasf matyasf force-pushed the allow_semantic_override branch 4 times, most recently from 950ab9d to c9f2d9e Compare April 23, 2026 14:18
@matyasf matyasf changed the title WIP refactor(many): allow overriding semantic theme layer in new components Better types for themes, small fixes Apr 23, 2026
@matyasf matyasf force-pushed the allow_semantic_override branch 2 times, most recently from de441d5 to 253a38d Compare April 23, 2026 21:42
{...(interaction === 'enabled' &&
!isEditable && {
themeOverride: (componentTheme) => ({
//@ts-expect-error TODO-theme-types
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only change here, dont know why this file was reformatted

@matyasf matyasf removed the request for review from HerrTopi April 24, 2026 08:39
@matyasf matyasf force-pushed the allow_semantic_override branch from 253a38d to 6c7faa5 Compare April 24, 2026 09:34
@matyasf matyasf requested review from HerrTopi and ToMESSKa April 24, 2026 09:53
Copy link
Copy Markdown
Contributor

@ToMESSKa ToMESSKa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the props.ts for Pagination and Table (and subcomponents) I see WithStyleProps<null, instead of WithStyleProps<ReturnType<NewComponentTypes['XXX']. Is that supposed to be like that?

Also for these components, the the themeOverride form ( e.g. <Avatar themeOverride={{borderColor: 'red'}}>aa) gives some TS error. I see that these use themeOverride?: ThemeOverrideValue where other components use WithStyleProps<ReturnType<NewComponentTypes['XXX']

  • AiInformation
  • DataPermissionLevels
  • NutritionFacts
  • Avatar
  • FormFieldLayout
  • NumberInput
  • RadioInput
  • Spinner
  • TextArea

@matyasf matyasf force-pushed the allow_semantic_override branch from 5475750 to 89b2370 Compare April 28, 2026 12:22
@matyasf
Copy link
Copy Markdown
Collaborator Author

matyasf commented Apr 28, 2026

In the props.ts for Pagination and Table (and subcomponents) I see WithStyleProps<null, instead of WithStyleProps<ReturnType<NewComponentTypes['XXX']. Is that supposed to be like that?

Only if the component has no theme (= generateStyle has no componentTheme parameter). I've fixed it where it was needed, good catch

Also for these components, the the themeOverride form ( e.g. <Avatar themeOverride={{borderColor: 'red'}}>aa) gives some TS error. I see that these use themeOverride?: ThemeOverrideValue where other components use WithStyleProps<ReturnType<NewComponentTypes['XXX']

  • AiInformation
  • DataPermissionLevels
  • NutritionFacts
  • Avatar
  • FormFieldLayout
  • NumberInput
  • RadioInput
  • Spinner
  • TextArea

I've fixed these too, useStyle was not typed properly

@matyasf matyasf requested a review from ToMESSKa April 28, 2026 12:24
@matyasf matyasf force-pushed the allow_semantic_override branch from 89b2370 to ab54902 Compare April 28, 2026 15:26
@matyasf matyasf force-pushed the allow_semantic_override branch from ab54902 to db78998 Compare April 28, 2026 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants