Skip to content

Conversation

@leannerivera
Copy link
Contributor

@leannerivera leannerivera commented Apr 1, 2019

Closes #1178

Formerly, these components were using an <h1> tag which is not accessible as there should not be more than one <h1> tag on the page. The default has been changed so that these components use an <h3> tag instead.

Test

Screen Shot 2019-04-01 at 3 14 54 PM

Changelog

Changed

  • <h3> is now default. <h2> - <h6> is supported.

@joseegm
Copy link
Contributor

joseegm commented Apr 1, 2019

Test Version for this PR was deployed

Built with commit 80251ed

https://deploy-preview-1367--fundamental.netlify.com

jbadan
jbadan previously requested changes Apr 1, 2019
Copy link
Contributor

@jbadan jbadan left a comment

Choose a reason for hiding this comment

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

Check the netlify error: there's a syntax issue in the md file change.

Did you check your changes on the docs site visually? It doesn't run for me when I pull this down.

@leannerivera
Copy link
Contributor Author

leannerivera commented Apr 2, 2019

Check the netlify error: there's a syntax issue in the md file change.

Did you check your changes on the docs site visually? It doesn't run for me when I pull this down.

Yes, I checked while I was working with each one. Netlify error should be fixed with latest push. :)

Apologies thought you meant playground. Gonna check that now.

@jbadan
Copy link
Contributor

jbadan commented Apr 2, 2019

Looks like changes were made to ActionBar and Modal component documentation pages.

Missing in Menu, MegaMenu, SideNavigation and Panel components.

@leannerivera
Copy link
Contributor Author

Looks like changes were made to ActionBar and Modal component documentation pages.

Missing in Menu, MegaMenu, SideNavigation and Panel components.

This has been fixed -- thanks.

@leannerivera leannerivera dismissed jbadan’s stale review April 2, 2019 20:20

fixed with latest push

@jbadan
Copy link
Contributor

jbadan commented Apr 2, 2019

@leannerivera please do not dismiss a blocking review until the person blocking it has 👍 the changes

jbadan
jbadan previously requested changes Apr 2, 2019
Copy link
Contributor

@jbadan jbadan left a comment

Choose a reason for hiding this comment

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

Panel has not been addressed in the docs site. see: https://sap.github.io/fundamental/layouts/panel.html

@jbadan
Copy link
Contributor

jbadan commented Apr 2, 2019

There's still a bunch of examples that need to be updated in the docs sites, can you do a global search for h1s and find the ones that are using the components we altered instead of me playing whack-a-mole with this?

For example, they're still in modal, contextual-menu, loading spinner among others.

Copy link
Contributor

@jbadan jbadan left a comment

Choose a reason for hiding this comment

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

This has a lot of my work now, so I'll give a partial ⛵️

@jbadan jbadan dismissed their stale review April 3, 2019 19:21

dismissing for external reviewer

@jbadan jbadan requested a review from a team April 3, 2019 19:21
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 9e7b8d5 into master Apr 4, 2019
@jbadan jbadan deleted the fix/update-default-header branch April 4, 2019 16:47
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.

5 participants