Skip to content

fix(dropdown, button, link): various bug fixes and aria fixes (Fixes #1814,#1817) #2159

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

Jump to bottom
Merged
merged 21 commits into from
Nov 12, 2018
Merged

fix(dropdown, button, link): various bug fixes and aria fixes (Fixes #1814,#1817) #2159

merged 21 commits into from
Nov 12, 2018

Conversation

tmorehouse
Copy link
Member

@tmorehouse tmorehouse commented Nov 10, 2018 •
edited
Loading

Description of Pull Request:

  • Various bug fixes for dropdown
  • Fixes for ARIA compliance on button/link when a non button/link tag is requested (assistive technology users would find dropdowns, buttons, and links were inaccessible via keyboard navigation and missing proper role attributes). Fixes ARIA issues introduced by PR feat(button): Make button tag configurable #1929

Fixes #1814
Fixes #1817

Eliminates the need for the clickout.js mixin

PR checklist:

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Enhancement to an existing feature
  • ARIA accessibility
  • Documentation update
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

The PR fulfills these requirements:

  • It's submitted to the dev branch, not the master branch
  • When resolving a specific issue, it's referenced in the PR's title (i.e. fixes #xxxx[,#xxxx], where "xxxx" is the issue number)
  • PR titles should following the Conventional Commits naming convention (i.e. "fix(alert): not alerting during SSR render", "docs(badge): Updated pill examples, fix typos", "chore: fix typo in docs", etc)

If new features/enhancement/fixes are added or changed:

  • Includes documentation updates (including updating the component's package.json for slot and event changes)
  • New/updated tests are included and passing (if required)
  • Existing test suites are passing
  • The changes have not impacted the functionality of other components or directives
  • ARIA Accessibility has been taken into consideration (does it affect screen reader users or keyboard only users? clickable items should be in the tab index, etc)

@tmorehouse
fix(dropdown): add touchstart handler to proper elements
10c1a26
@tmorehouse
fix(button): Add aria compliance when tag prop is neither 'button' or…
713f924 
… 'a'

Ensure that the nonstandard tag is available in tab index and has an appropriate role, and that it is in tab order when not disabled
@tmorehouse
fix(button): ensure the appropriate role and tab index set on non lin…
f4f5dfb 
…ks or buttons

Ensure that the nonstandard tag is available in tab index and has an appropriate role, and that it is in tab order when not disabled.

Fixes for ARIA compliance
@tmorehouse
button.js lint
db1db17
@tmorehouse
button.js lint
4e5f2fe
@tmorehouse
lint
b26ddeb
@tmorehouse
button.js check for props.tag existence first
7a8ff1c
@tmorehouse
Update button.js
21a31ec
@tmorehouse
Update button.js
461d368
@tmorehouse tmorehouse mentioned this pull request Nov 10, 2018
Merged
@codecov
Copy link

codecov bot commented Nov 10, 2018 •
edited
Loading

Codecov Report

Merging #2159 into dev will increase coverage by 0.81%.
The diff coverage is 64.1%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #2159      +/-   ##
==========================================
+ Coverage   65.04%   65.85%   +0.81%     
==========================================
  Files         159      158       -1     
  Lines        2958     2932      -26     
  Branches      812      803       -9     
==========================================
+ Hits         1924     1931       +7     
+ Misses        748      727      -21     
+ Partials      286      274      -12
Impacted Files Coverage Δ
src/components/button/button.js 86.36% <100%> (+9.44%) ⬆️
src/mixins/dropdown.js 3.96% <6.66%> (-3.29%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3165e14...3bad47d. Read the comment docs.

@tmorehouse
dropdown mixin: only enable mouseover events when not inside a navbar…
b88ebc5 
…-nav

As per latest bootstrap V4 JS
@tmorehouse
dropdown mixin: listen for focusout on this.$el rather than this.$ref…
9fd2891 
…s.menu

Allows for clicking the toggle button to close the menu.

Currently it was closing an immediately re-opening the menu
@tmorehouse tmorehouse mentioned this pull request Nov 12, 2018
Closed
@tmorehouse tmorehouse changed the title [WIP] fix(dropdown, button, link): various bug fixes and aria fixes [WIP] fix(dropdown, button, link): various bug fixes and aria fixes (Fixes #1814,#1817) Nov 12, 2018
@tmorehouse tmorehouse mentioned this pull request Nov 12, 2018
Closed
@tmorehouse tmorehouse changed the title [WIP] fix(dropdown, button, link): various bug fixes and aria fixes (Fixes #1814,#1817) fix(dropdown, button, link): various bug fixes and aria fixes (Fixes #1814,#1817) Nov 12, 2018
@tmorehouse tmorehouse merged commit e79270d into dev Nov 12, 2018
@tmorehouse tmorehouse deleted the tmorehouse/dropdown branch November 12, 2018 22:23
@tmorehouse tmorehouse mentioned this pull request Nov 14, 2018
Closed
89 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Type: Accessibility Type: Fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

clickout.js need some optimization dropdown wont close when click on component that uses stopPropagation
1 participant
@tmorehouse