Skip to content

feat(button): Make button tag configurable #1929

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 2 commits into from
Jul 15, 2018
Merged

feat(button): Make button tag configurable #1929

merged 2 commits into from
Jul 15, 2018

Conversation

jacobmllr95
Copy link
Member

This PR makes the b-buttons tag confirgurable (e.g. to a div).

This can be very handy if you want the buttons styling but want to place another clickable element inside the button which is normally prevented by the interactive content model.

@codecov
Copy link

codecov bot commented Jul 8, 2018 •
edited
Loading

Codecov Report

Merging #1929 into dev will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #1929      +/-   ##
==========================================
+ Coverage   61.23%   61.24%   +0.01%     
==========================================
  Files         154      154              
  Lines        2884     2885       +1     
  Branches      796      796              
==========================================
+ Hits         1766     1767       +1     
  Misses        802      802              
  Partials      316      316
Impacted Files Coverage Δ
src/components/dropdown/dropdown.js 100% <ø> (ø) ⬆️
src/components/button/button.js 76.92% <100%> (+0.92%) ⬆️

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 d63e090...c13c8c8. Read the comment docs.

mosinve
mosinve requested changes Jul 13, 2018
View reviewed changes
Copy link
Member

@mosinve mosinve left a comment

Choose a reason for hiding this comment

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

Can you add unit test to this new feature?

@jacobmllr95
Copy link
Member Author

jacobmllr95 commented Jul 14, 2018 •
edited
Loading

@mosinve I've added the tests.

@mosinve mosinve merged commit afcadd9 into bootstrap-vue:dev Jul 15, 2018
@jacobmllr95 jacobmllr95 mentioned this pull request Nov 4, 2018
Closed
@tmorehouse tmorehouse mentioned this pull request Nov 10, 2018
Merged
16 tasks
@tmorehouse
Copy link
Member

Using a tag that is not an interactive element as the b-button root element tag (and not providing interactive element inside) makes the rendered "button" completely inaccessible to keyboard only users (and to blind users as there is no role associated with the non-standard tag).

When making changes like this (i.e. going to a non interactive element tag from an interactive tag), one should ensure that the element can be tabbed to via the keyboard.

PR #2159 addresses this issue

@jacobmllr95 jacobmllr95 deleted the feat/button-customizable-tag branch November 12, 2018 15:58
@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

@mosinve mosinve mosinve approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jacobmllr95 @tmorehouse @mosinve