Skip to content

Add missing role="button" in .navbar's .dropdown-toggle#26831

Merged
patrickhlauke merged 1 commit into
v4-devfrom
XhmikosR-patch-1
Jul 9, 2018
Merged

Add missing role="button" in .navbar's .dropdown-toggle#26831
patrickhlauke merged 1 commit into
v4-devfrom
XhmikosR-patch-1

Conversation

@XhmikosR

@XhmikosR XhmikosR commented Jul 9, 2018

Copy link
Copy Markdown
Member

Fixes #26830

@XhmikosR XhmikosR requested a review from patrickhlauke July 9, 2018 13:31
@XhmikosR XhmikosR changed the title Add missing role=button in dropdown-toggle example Add missing role="button" in .navbar's .dropdown-toggle Jul 9, 2018
@XhmikosR XhmikosR requested a review from mdo July 9, 2018 13:40
@patrickhlauke patrickhlauke merged commit 8b50a72 into v4-dev Jul 9, 2018
@XhmikosR XhmikosR deleted the XhmikosR-patch-1 branch July 9, 2018 13:42
@patrickhlauke

Copy link
Copy Markdown
Member

oops, just realised perhaps intent was not to merge due to closeness of new release...sorry if that was the case/if that creates issues now :(

@XhmikosR

XhmikosR commented Jul 9, 2018

Copy link
Copy Markdown
Member Author

Nah, this is good I believe; it's a trivial change, don't worry.

@mdo mdo mentioned this pull request Jul 9, 2018
@thgh

thgh commented Oct 7, 2018

Copy link
Copy Markdown
Contributor

Why does Bootstrap use <a> instead of <button> for a button functionality?

@patrickhlauke

Copy link
Copy Markdown
Member

mainly, for ease/consistency of styling. as it's then still exposed correctly thanks to the role="button", it's not a major issue.

of course, authors can use an actual <button>, e.g. something like

<button class="btn btn-link nav-link dropdown-toggle" href="#" id="navbarDropdownMenuLink" role="button" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">Dropdown link</button>

but currently the baseline for the text in the links and the text in the button don't always correctly line up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants