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

fix(list-group): disabled button items #1444

Merged
merged 7 commits into from Dec 10, 2017
Merged

fix(list-group): disabled button items #1444

merged 7 commits into from Dec 10, 2017

Conversation

tmorehouse
Copy link
Member

@tmorehouse tmorehouse commented Dec 9, 2017

When tag was set to 'button' and the disabled prop was also set, the rendered button was not being disabled (although it had the disabled class and appeared disabled). This fix ensures the disabled attribute is set when rendered as a button and the disabled prop is true.

Adds new prop button, as a shortcut, to force render as a button list-group-item-action

Also extends the documentation with additional examples

@codecov
Copy link

codecov bot commented Dec 9, 2017

Codecov Report

Merging #1444 into dev will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev    #1444   +/-   ##
=======================================
  Coverage   47.41%   47.41%           
=======================================
  Files         135      135           
  Lines        2529     2529           
  Branches      789      789           
=======================================
  Hits         1199     1199           
  Misses        950      950           
  Partials      380      380
Impacted Files Coverage Δ
src/components/list-group/list-group-item.js 100% <100%> (ø) ⬆️

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 f95b872...47d1434. Read the comment docs.

@tmorehouse tmorehouse changed the title fix(list-group): diabled button items fix(list-group): disabled button items Dec 9, 2017
@tmorehouse
Copy link
Member Author

tmorehouse commented Dec 9, 2017

I'm wondering if maybe we should add in a boolean button prop to force the list-item to be rendered as a button?

Any thoughts on this @pi0 and @alexsasharegan ?

@alexsasharegan
Copy link
Member

@tmorehouse , that seems like a good idea. For links, is the component rendering based on the presence of an href or to prop?

@tmorehouse
Copy link
Member Author

tmorehouse commented Dec 9, 2017

Yeah, It renders a b-link if either to or href is set, otherwise it renders a div by default (which can be overridden by the tag prop)

But if the proposed new prop button is set to true, then it always renders a <button> action item (and ignores any link props or the tag prop)

@tmorehouse tmorehouse merged commit c037b38 into dev Dec 10, 2017
@tmorehouse tmorehouse deleted the feat/badges branch December 10, 2017 00:03
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.

None yet

2 participants