Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing <main> element to documentation #22364

Merged
merged 15 commits into from
May 5, 2017
Merged

Add missing <main> element to documentation #22364

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
6b35cc0
remove hard coded padding for dropdown-item in dropdown.scss create a…
Lausselloic Jan 9, 2017
6077120
remove hard coded padding for dropdown-item in dropdown.scss create a…
Lausselloic Jan 9, 2017
626a756
Merge branch 'v4-dev' of https://github.com/Lausselloic/bootstrap int…
Lausselloic Feb 20, 2017
ec75282
add a font color parameter to the badge variant mixin
Lausselloic Feb 24, 2017
dd6ed1d
Merge branch 'v4-dev' into v4-dev
Lausselloic Apr 6, 2017
59e7ddd
Revert "add a font color parameter to the badge variant mixin"
Lausselloic Apr 6, 2017
8254a53
fix(dropdow): fix invalid variable declaration
Lausselloic Apr 6, 2017
03d7ad4
Revert "Revert "add a font color parameter to the badge variant mixin""
Lausselloic Apr 6, 2017
aa13b3c
Revert "remove hard coded padding for dropdown-item in dropdown.scss …
Lausselloic Apr 6, 2017
34873e9
docs(accessibility): improve documentation pages accessibility. Add a…
Lausselloic Apr 6, 2017
bb21b0e
Revert "Revert "Revert "add a font color parameter to the badge varia…
Lausselloic Apr 6, 2017
f304347
Merge branch 'v4-dev' into v4-dev-a11y-docsimprovement
patrickhlauke Apr 9, 2017
55a9816
Merge branch 'v4-dev' into v4-dev-a11y-docsimprovement
Lausselloic May 3, 2017
3c2554e
push down main container to contzent excluding the nav
Lausselloic May 3, 2017
2cd31ff
Merge branch 'v4-dev' into v4-dev-a11y-docsimprovement
patrickhlauke May 5, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/_layouts/default.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@

{% include nav-home.html %}

<div id="content">
<main id="content" role="main">
Copy link
Contributor

@wolfy1339 wolfy1339 Apr 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

role="main" is not required here since we are using the <main> element

Copy link
Contributor Author

@Lausselloic Lausselloic Apr 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I always double the main element with the aria role for IE11 who doesn't take care about this element

Copy link
Member

Choose a reason for hiding this comment

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

agree, it's unnecessary normally, but notoriously IE11 needs this extra role to make it work (as in, expose it correctly to AT). as it doesn't do any harm, this PR looks fine to me as is

Copy link
Member

Choose a reason for hiding this comment

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

Just a question about that but it seems main tag isn't supported by IE10 (https://caniuse.com/#feat=html5semantic).
What's the lower release of IE we support in our documentation ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes main tag doesn't fit well on IE10, need to add a css

main { display:block; }

Bootstrap compatibility is IE1à and upper ( https://v4-alpha.getbootstrap.com/getting-started/browsers-devices/#desktop-browsers)
So maybe need to add a fix in the core _reboot.scss for all HTML5 semantic default display?

article, aside, footer, header, hgroup, main, nav, section {
  display: block;
}

Copy link
Member

Choose a reason for hiding this comment

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

ah, in v3 we had html5shiv take care of that, forgot we don't have that in v4 anymore.

i think it'd make sense to add the default block styling to reboot. i'll take care of that before merging this, assuming @mdo is ok with it

Copy link
Member

Choose a reason for hiding this comment

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

done #22573

{{ content }}
</div>
</main>

{% include footer.html %}
</body>
Expand Down
4 changes: 2 additions & 2 deletions docs/_layouts/docs.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@
<div class="col-12 col-md-3 push-md-9 bd-sidebar">
{% include nav-docs.html %}
</div>
<div class="col-12 col-md-9 pull-md-3 bd-content">
<main class="col-12 col-md-9 pull-md-3 bd-content" role="main">
<h1 class="bd-title" id="content">{{ page.title }}</h1>
{{ content }}
Copy link
Member

Choose a reason for hiding this comment

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

wondering if this may not be the more appropriate location for <main> (depends if we count the side navigation as content or not). if so, i'd suggest keeping the <div class="col-12..."> and wrapping its contents in a <main role="main">, rather than replacing the <div> altogether

Copy link
Member

Choose a reason for hiding this comment

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

@Lausselloic you ok to make those change to this PR? need a hand?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry haven't seen your comment. Yes I will do it

Copy link
Member

Choose a reason for hiding this comment

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

great stuff. will double-check it all works and merge. thanks :)

</div>
</main>
</div>
</div>

Expand Down