Skip to content

Conversation

@jbadan
Copy link
Contributor

@jbadan jbadan commented Apr 2, 2019

Description

<h1> headings were used in following components:
* ActionBar
* MegaMenu (Deprecated - does not exist in FR)
* Menu
* Modal
* Panel
* SideNavigation

  • Tile

  • ProductTile

  • Default header is now <h3> with the option to customize using <h2>-<h6>

  • <h1> is not allowed due to a11y reasons (except in ActionBar)

  • updated unit tests for each component - I did not separate these into separate files as we have a story in the backlog to separate the testing files for each subcomponent

  • update snapshots for each component

  • chose not to add additional examples as it didn't seem necessary with the props table explaining the options. Open for other opinions!

fixes #249
Accompanied by changes in fundamental repo: SAP/fundamental#1367

No visual changes in examples

@jbadan jbadan requested a review from a team April 2, 2019 00:07
@jbadan
Copy link
Contributor Author

jbadan commented Apr 2, 2019

@greg-a-smith @bcullman Updated the pr to use headingLevel and changed the default text. I agree that ActionBar should be able to use <h1> tags. I've left the default for it to be <h3>, but it allows <h1> tags, unlike the other components.

Copy link
Contributor

@greg-a-smith greg-a-smith left a comment

Choose a reason for hiding this comment

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

Looks good. 🚢

@jbadan
Copy link
Contributor Author

jbadan commented Apr 3, 2019

@greg-a-smith Updated to include Tile and Product Tile components

Copy link
Contributor

@greg-a-smith greg-a-smith left a comment

Choose a reason for hiding this comment

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

Looks good. 🚢

@jbadan jbadan merged commit deaa643 into master Apr 3, 2019
@jbadan jbadan deleted the fix/headings branch April 3, 2019 21:19
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.

Allow for customizable heading levels in components

4 participants