Skip to content

Commit d6e4cd5

Browse files
committed
fix(ui-heading): fix Heading rendering as H2 in most cases
Previously it was rendering h2 in lots of cases when it should not. Make the order more clear, add documentation to it To test: Check if Heading's DOM levels correspond to the logic stated in the documentation. Fixes INSTUI-4657
1 parent 6e7c1e2 commit d6e4cd5

File tree

3 files changed

+79
-64
lines changed

3 files changed

+79
-64
lines changed

packages/ui-heading/src/Heading/README.md

Lines changed: 32 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,33 +4,39 @@ describes: Heading
44

55
Heading is a component for creating typographic headings.
66

7-
## Variant
7+
### Variant
88

99
Variant covers almost all use cases for headings on pages. Their name reflects the places they meant to be used. It takes care of the style of the heading
1010

11-
NOTE 1: for legacy reasons, each `variant` has a default `level` set. This is not the recommended way and will be removed in a later major release. Please specify the `level` directly!
12-
13-
NOTE 2: when `variant` is set the `as` prop is ignored
14-
15-
IMPORTANT A11Y NOTE 1: there can be only one `h1` tag in a page
16-
17-
IMPORTANT A11Y NOTE 2: `h` tags can not skip a level, so for example an `h1` followed by an `h3` not allowed
11+
```js
12+
---
13+
type: embed
14+
---
15+
<Alert variant="info">
16+
<List margin="0 0 medium">
17+
<List.Item>For legacy reasons, each <code>variant</code> has a default <code>level</code> set. This is not the recommended way and will be removed in a later major release. Please always specify the <code>level</code>!</List.Item>
18+
<List.Item>When <code>variant</code> is set the <code>as</code> prop is ignored</List.Item>
19+
<List.Item>A11Y GUIDELINE: There can be only one <code>h1</code> tag in a page</List.Item>
20+
<List.Item>A11Y GUIDELINE: <code>h</code> tags can not skip a level, so for example an <code>h1</code> followed by an <code>h3</code> not allowed</List.Item>
21+
</List>
22+
</Alert>
23+
```
1824

1925
```js
2026
---
2127
type: example
2228
---
2329
<div>
24-
<Heading variant="titlePageDesktop"> titlePageDesktop </Heading><br/>
25-
<Heading variant="titlePageMobile"> titlePageMobile </Heading><br/>
26-
<Heading variant="titleSection"> titleSection </Heading><br/>
27-
<Heading variant="titleCardSection"> titleCardSection </Heading><br/>
28-
<Heading variant="titleModule"> titleModule </Heading><br/>
29-
<Heading variant="titleCardLarge"> titleCardLarge </Heading><br/>
30-
<Heading variant="titleCardRegular"> titleCardRegular </Heading><br/>
31-
<Heading variant="titleCardMini"> titleCardMini </Heading><br/>
32-
<Heading variant="label"> label </Heading><br/>
33-
<Heading variant="labelInline"> labelInline </Heading><br/>
30+
<Heading variant="titlePageDesktop" level="h1"> titlePageDesktop </Heading><br/>
31+
<Heading variant="titlePageMobile" level="h1"> titlePageMobile </Heading><br/>
32+
<Heading variant="titleSection" level="h2"> titleSection </Heading><br/>
33+
<Heading variant="titleCardSection" level="h2"> titleCardSection </Heading><br/>
34+
<Heading variant="titleModule" level="h2"> titleModule </Heading><br/>
35+
<Heading variant="titleCardLarge" level="h3"> titleCardLarge </Heading><br/>
36+
<Heading variant="titleCardRegular" level="h3"> titleCardRegular </Heading><br/>
37+
<Heading variant="titleCardMini" level="h4"> titleCardMini </Heading><br/>
38+
<Heading variant="label" level="h5"> label </Heading><br/>
39+
<Heading variant="labelInline" level="h6"> labelInline </Heading><br/>
3440
</div>
3541
```
3642

@@ -51,18 +57,21 @@ type: example
5157

5258
### Heading level
5359

54-
Generate content headings, from h1 to h5. Use the `margin` prop to add margin.
60+
What DOM element is output is determined in the following order:
5561

56-
- The `as` prop controls what html element is output. _(if not defined it will default to level)._
57-
- The `level` prop sets its appearance.
62+
1. (deprecated) If the variant prop is set, then the value of level prop. If variant is set but level is not, <h1>-<h6> based on the variant prop's value.
63+
2. The value of the `as` prop
64+
3. The value of the `level` prop
65+
4. `<h2>`
66+
67+
The `variant` and `level` props sets its appearance in this order.
5868

5969
```js
6070
---
6171
type: example
6272
---
6373
<div>
64-
<Heading level="h1" as="h2" margin="0 0 x-small">Heading level One</Heading>
65-
74+
<Heading level="h1" as="h3" margin="0 0 x-small">This renders as <code>&lt;h3&gt;</code></Heading>
6675
</div>
6776
```
6877

packages/ui-heading/src/Heading/index.tsx

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,7 @@
2525
import { Component } from 'react'
2626

2727
import { View } from '@instructure/ui-view'
28-
import {
29-
getElementType,
30-
passthroughProps,
31-
callRenderProp
32-
} from '@instructure/ui-react-utils'
28+
import { passthroughProps, callRenderProp } from '@instructure/ui-react-utils'
3329
import { testable } from '@instructure/ui-testable'
3430
import { IconAiColoredSolid } from '@instructure/ui-icons'
3531

@@ -40,6 +36,7 @@ import generateComponentTheme from './theme'
4036

4137
import { propTypes, allowedProps } from './props'
4238
import type { HeadingProps } from './props'
39+
import { AsElementType } from '@instructure/shared-types'
4340

4441
const variantLevels: Record<
4542
NonNullable<HeadingProps['variant']>,
@@ -72,8 +69,7 @@ class Heading extends Component<HeadingProps> {
7269
static defaultProps = {
7370
children: null,
7471
border: 'none',
75-
color: 'inherit',
76-
level: 'h2'
72+
color: 'inherit'
7773
} as const
7874

7975
ref: Element | null = null
@@ -201,22 +197,27 @@ class Heading extends Component<HeadingProps> {
201197
...props
202198
} = this.props
203199

204-
const propsForGetElementType = variant ? {} : this.props
205-
206-
const ElementType = getElementType(Heading, propsForGetElementType, () => {
207-
if (level === 'reset') {
208-
return 'span'
209-
} else if (level) {
210-
return level
200+
let ElementType: AsElementType = 'h2'
201+
if (variant) {
202+
// TODO deprecated, remove. `variant` should not set DOM level
203+
if (level) {
204+
if (level === 'reset') {
205+
ElementType = 'span'
206+
} else {
207+
ElementType = level
208+
}
209+
} else {
210+
ElementType = variantLevels[variant]
211211
}
212-
213-
if (variant) {
214-
return variantLevels[variant]
212+
} else if (props.as) {
213+
ElementType = props.as
214+
} else if (level) {
215+
if (level === 'reset') {
216+
ElementType = 'span'
215217
} else {
216-
return 'span'
218+
ElementType = level
217219
}
218-
})
219-
220+
}
220221
return (
221222
<View
222223
aria-label={this.getAriaLabel()}

packages/ui-heading/src/Heading/props.ts

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ type HeadingLevel<U extends keyof JSX.IntrinsicElements> = U
4343

4444
type HeadingOwnProps = {
4545
/**
46-
* transforms heading into an ai variant
46+
* transforms heading into an `ai` variant
4747
*/
4848
aiVariant?: 'stacked' | 'horizontal' | 'iconOnly'
4949
/**
@@ -58,19 +58,23 @@ type HeadingOwnProps = {
5858
* The font color to render, NOTE: `ai` color is deprecated. Use the `aiVariant` prop instead
5959
*/
6060
color?:
61-
| 'primary'
62-
| 'secondary'
63-
| 'primary-inverse'
64-
| 'secondary-inverse'
65-
| 'inherit'
66-
| 'ai'
61+
| 'primary'
62+
| 'secondary'
63+
| 'primary-inverse'
64+
| 'secondary-inverse'
65+
| 'inherit'
66+
| 'ai'
6767
/**
68-
* The *visual* appearance of the Heading: h1 is largest; h5 is smallest.
68+
* The level of the heading in the DOM: h1 is largest; h5 is smallest.
6969
*/
7070
level?: HeadingLevel<'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'> | 'reset'
7171
/**
72-
* Choose the element Heading should render as. Will default to the `level` prop
73-
* if not specified.
72+
* What DOM element is output is determined in the following order:
73+
* 1. (deprecated) If `variant` is set, then use the `level` prop, if that's
74+
* not set use `<h1>`-`<h6>` based on the `variant` prop's value
75+
* 2. The value of the `as` prop
76+
* 3. The value of the `level` prop
77+
* 4. `<h2>`
7478
*/
7579
as?: AsElementType
7680
/**
@@ -88,19 +92,20 @@ type HeadingOwnProps = {
8892
*/
8993
renderIcon?: Renderable
9094
/**
91-
* Sets appearance of the heading.
95+
* Sets appearance of the heading. Will also set its heading level, if not
96+
* specified by the `level` prop (deprecated, not recommended!)
9297
*/
9398
variant?:
94-
| 'titlePageDesktop'
95-
| 'titlePageMobile'
96-
| 'titleSection'
97-
| 'titleCardSection'
98-
| 'titleModule'
99-
| 'titleCardLarge'
100-
| 'titleCardRegular'
101-
| 'titleCardMini'
102-
| 'label'
103-
| 'labelInline'
99+
| 'titlePageDesktop'
100+
| 'titlePageMobile'
101+
| 'titleSection'
102+
| 'titleCardSection'
103+
| 'titleModule'
104+
| 'titleCardLarge'
105+
| 'titleCardRegular'
106+
| 'titleCardMini'
107+
| 'label'
108+
| 'labelInline'
104109
}
105110

106111
type PropKeys = keyof HeadingOwnProps

0 commit comments

Comments
 (0)