Skip to content

Conversation

@ToMESSKa
Copy link
Contributor

@ToMESSKa ToMESSKa commented Jan 15, 2026

INSTUI-4869

ISSUE:

Heading needs to be migrated to the new theming system

TEST PLAN:

<div>
  <Heading variant="titlePageDesktop" level="h1" renderIcon={<DiamondInstUIIcon/>}>titlePageDesktop</Heading><br/>
  <Heading variant="titlePageMobile" level="h1" renderIcon={<DiamondInstUIIcon/>}>titlePageMobile</Heading><br/>
  <Heading variant="titleSection" level="h2" renderIcon={<DiamondInstUIIcon/>}>titleSection</Heading><br/>
  <Heading variant="titleCardSection" level="h2" renderIcon={<DiamondInstUIIcon/>}>titleCardSection</Heading><br/>
  <Heading variant="titleModule" level="h2" renderIcon={<DiamondInstUIIcon/>}>titleModule</Heading><br/>
  <Heading variant="titleCardLarge" level="h3" renderIcon={<DiamondInstUIIcon/>}>titleCardLarge</Heading><br/>
  <Heading variant="titleCardRegular" level="h3" renderIcon={<DiamondInstUIIcon/>}>titleCardRegular</Heading><br/>
  <Heading variant="titleCardMini" level="h4" renderIcon={<DiamondInstUIIcon/>}>titleCardMini</Heading><br/>
  <Heading variant="label" level="h5" renderIcon={<DiamondInstUIIcon/>}>label</Heading><br/>
  <Heading variant="labelInline" level="h6" renderIcon={<DiamondInstUIIcon/>}>labelInline</Heading><br/>
</div>
  • check the icons gaps (gap between the icon and the text) and sizes @adamlobler (not in Figma)
<div>
  <Heading level="h1" renderIcon={<DiamondInstUIIcon/>}>titlePageDesktop</Heading><br/>
  <Heading level="h2" renderIcon={<DiamondInstUIIcon/>}>titlePageMobile</Heading><br/>
  <Heading level="h3" renderIcon={<DiamondInstUIIcon/>}>titleSection</Heading><br/>
  <Heading level="h4" renderIcon={<DiamondInstUIIcon/>}>titleCardSection</Heading><br/>
  <Heading level="h5" renderIcon={<DiamondInstUIIcon/>}>titleModule</Heading><br/>
  <Heading level="h6" renderIcon={<DiamondInstUIIcon/>}>titleCardLarge</Heading><br/>
</div>
  • check the AI variants

@github-actions
Copy link

PR Preview Action v1.8.0

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

Built to branch gh-pages at 2026-01-15 15:24 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@ToMESSKa ToMESSKa self-assigned this Jan 15, 2026
Comment on lines 45 to +46
fontWeight: componentTheme.weightImportant,
fontSize: componentTheme.titlePageDesktop,
lineHeight: componentTheme.lineHeight125
...componentTheme.titlePageDesktop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a composit token now

Comment on lines -141 to -166
if (aiVariant === 'stacked') {
return (
<>
<span css={this.props.styles?.igniteAIStacked}>
<IconAiColoredSolid />
<span css={this.props.styles?.igniteAI}>IgniteAI</span>
</span>
{children}
</>
)
}
if (aiVariant === 'horizontal') {
return (
<>
<IconAiColoredSolid />
<span css={this.props.styles?.igniteAI}>IgniteAI</span>
<StarsInstUIIcon color="ai" size="sm" />
{children}
</>
)
}
if (aiVariant === 'iconOnly') {
return (
<>
<IconAiColoredSolid />
&nbsp;{children}
</>
Copy link
Contributor Author

@ToMESSKa ToMESSKa Jan 15, 2026

Choose a reason for hiding this comment

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

I think this is some dead code/duplication as the 'stacked', 'horizontal' and 'iconOnly' AI cases are already checked above...(?)

@ToMESSKa ToMESSKa changed the title feat(ui-heading): rework Heading [v12] feat(ui-heading): rework Heading Jan 15, 2026
return (
<span css={[this.props.styles?.withIcon]} aria-hidden="true">
{callRenderProp(renderIcon)}&nbsp;{children}
{renderIconWithProps(renderIcon, iconSize, 'inherit')}
Copy link
Collaborator

Choose a reason for hiding this comment

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

there was a "none breaking space" here before the heading text that is missing

before:
Image

after:
Image

static defaultProps = {
children: null,
border: 'none',
color: 'inherit'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Now the default text color is "inherit" and as a result it gets the color value from somewhere else from the system, I think it would be more secure to have the "primary" color by default, wdyt?

That's why we have this text color here as well in dark mode:

Image

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