Skip to content

Conversation

circlecube
Copy link
Member

No description provided.

@circlecube circlecube requested a review from wpalani September 5, 2023 17:09
@circlecube
Copy link
Member Author

I'm seeing that many Title components are being set to h1, but I think when we pass a specific size this component should adjust automatically to a matching heading level.

This will help with accessibility for heading levels - I'm seeing some cypress a11y checks fail because all the headings are coming out as h1. We could update all uses of this component to specify the as prop, but I think a better dev experience is to know that by setting size we also get a heading level according to the size, and if we want to specify something different, we should then also set the as prop.

}, ref ) => {

// If `as` is default and `size` is set, update Component to proper heading level
if ( Component === 'h1' && typeof size !== undefined ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would the following case be valid?

<Title as="h1" size={3}>Example title</Title>

In this example, the user wants an h1 to be styled as a (presumably) smaller component, but to keep the semantic markup weight of an h1. I believe in this case, the Component would be re-rendered to h3 without a way to override it.

Perhaps one option to retain this backwards compatibility would be to set the defaultProps for as to undefined, and check for Component === undefined here and later defining a default when this block does not apply.

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.

2 participants