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.

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

Docs unordered lists dropdowns #28591

Merged
merged 14 commits into from
May 9, 2019

Conversation

Katherraptor
Copy link
Contributor

@Katherraptor Katherraptor commented Apr 2, 2019

Fixes #25728.

Documentation should suggest semantic ul and li elements in the dropdowns and navbar components to encourage semantic HTML markup. Added a mention that while examples here depict semantic ul elements custom markup is supported.

Replace snippets and examples in dropdown docs to use semantic ul and
li elements.
Replace dropdown snippets and examples in navbar docs to use semantic
ul and li elements.
Mention in the dropdown examples paragraph that custom markup is
supported even though shown examples use ul’s.
@patrickhlauke
Copy link
Member

i'd just mention that "compliance with the WAI-ARIA standard" is not a thing. this has nothing to do with ARIA, as these aren't aria menus or anything else other than a series of links. while the use of unordered lists here would be my personal preference as well, it's not necessarily wrong not to have them in a list, and wouldn't be, say, a hard fail of WCAG 1.3.1 either...

@tmorehouse
Copy link
Contributor

tmorehouse commented Apr 6, 2019

Moving the divider into the li would help:

<ul class="dropdown-menu" aria-labelledby="navbarDropdown">
  <li><a class="dropdown-item" href="#">Action</a></li>
  <li><hr class="dropdown-divider"></li>
  <li><a class="dropdown-item" href="#">Another action</a></li>
</ul>

Or

<ul class="dropdown-menu" aria-labelledby="navbarDropdown">
  <li><a class="dropdown-item" href="#">Action</a></li>
  <li><div role="separator" class="dropdown-divider" /></li>
  <li><a class="dropdown-item" href="#">Another action</a></li>
</ul>

The first example above would be most semantically correct without the need of a role attribute.

@mdo
Copy link
Member

mdo commented Apr 9, 2019

@bittner I'm clearing out some of your comments here—one is enough. 😅 You can also batch your comments in the future as a single review to reduce comment notifications and noise :).

@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@twbs twbs deleted a comment from bittner Apr 9, 2019
@bittner
Copy link

bittner commented Apr 9, 2019

@bittner I'm clearing out some of your comments here—one is enough. sweat_smile You can also batch your comments in the future as a single review to reduce comment notifications and noise :).

I'm sorry! 😏 I was really just trying to help by marking the related places. 💩 And, yes you're right.

Replace snippets and examples in dropdown docs to use semantic ul and
li elements.
Replace dropdown snippets and examples in navbar docs to use semantic
ul and li elements.
Mention in the dropdown examples paragraph that custom markup is
supported even though shown examples use ul’s.
Move dropdown-divider class onto hr inside list item instead of on list
item itself
@Katherraptor
Copy link
Contributor Author

Feedback addressed! I seem to have gotten my commits all in a tizzy (bit of a forking rookie) so sorry. Should probably squash

Copy link
Member

@MartijnCuppens MartijnCuppens left a comment

Choose a reason for hiding this comment

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

LGTM. @patrickhlauke, could you have a look at the latest changes here?

@XhmikosR XhmikosR added this to Inbox in v5 via automation May 4, 2019
@XhmikosR
Copy link
Member

XhmikosR commented May 9, 2019

@patrickhlauke: ping

Copy link
Member

@patrickhlauke patrickhlauke left a comment

Choose a reason for hiding this comment

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

LGTM

v5 automation moved this from Inbox to Approved May 9, 2019
@XhmikosR XhmikosR merged commit a5d7dff into twbs:master May 9, 2019
v5 automation moved this from Approved to Shipped May 9, 2019
@Katherraptor Katherraptor deleted the docs-unordered-lists-dropdowns branch May 9, 2019 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
v5
  
Shipped
Development

Successfully merging this pull request may close these issues.

Shouldn't we suggest unordered lists (ul li) for dropdown menus?
7 participants