Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

[READY FOR REVIEW] feat(Button): adding disabled prop to Button #14

Merged
merged 1 commit into from
Jul 31, 2018

Conversation

bmdalex
Copy link
Collaborator

@bmdalex bmdalex commented Jul 25, 2018

Button(disabled prop)

This PR will introduce possibility to specify disabled buttons

TODO

  • Conformance test
  • Minimal doc site example
  • Stardust base theme
  • Teams Light theme
  • Teams Dark theme
  • Teams Contrast theme
  • Confirm RTL usage
  • W3 accessibility check
  • Stardust accessibility check
  • Update glossary props table
  • Update the CHANGELOG.md

API Proposal

disabled

disabled property will disable buttons by manipulating opacity CSS style, borders, colors, hover styles and preventing clicking

Know issue: Icons are not disabled because disabled property was not added to icons at the time of creating this PR. I created a PR for that #113
Icon buttons will become disabled out of the box after merging that one

screen shot 2018-07-18 at 21 31 15

<Button disabled content="Click me" />

will render

<button class="ui-button" disabled="">
  <span>Click me</span>
</button>

@codecov
Copy link

codecov bot commented Jul 25, 2018

Codecov Report

Merging #14 into master will decrease coverage by 0.03%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #14      +/-   ##
==========================================
- Coverage   82.77%   82.73%   -0.04%     
==========================================
  Files          59       59              
  Lines         830      840      +10     
  Branches      186      180       -6     
==========================================
+ Hits          687      695       +8     
- Misses        139      141       +2     
  Partials        4        4
Impacted Files Coverage Δ
src/components/Button/buttonVariables.ts 100% <ø> (ø) ⬆️
src/components/Button/buttonRules.ts 90.62% <100%> (+0.96%) ⬆️
src/components/Button/Button.tsx 80.55% <75%> (-2.21%) ⬇️

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 48d735a...86f23b2. Read the comment docs.

@bmdalex bmdalex changed the title Feat/button disabled prop feat(Button): adding disabled prop to Button Jul 25, 2018
@levithomason
Copy link
Member

  • add disabled attribute to the button to disable interaction
  • add opacity: 0.5 and cursor: not-allowed for styling
  • ensure hover style is not applied
  • ensure click handlers are not fired

The icon should not need to be styled individually here, I don't think...

@bmdalex bmdalex requested a review from mnajdova as a code owner July 30, 2018 19:44
@bmdalex bmdalex force-pushed the feat/button-disabled-prop branch 4 times, most recently from 27c19ef to c097d5a Compare July 30, 2018 20:52
key="btn-icon"
name={icon}
xSpacing={!content ? 'none' : iconIsAfterButton ? 'before' : 'after'}
color={primary ? 'white' : 'black'}
Copy link
Member

Choose a reason for hiding this comment

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

If we want the icon color to be the same as the text color, we can use 'currentColor' instead of the hard-coded value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@notandrew
currentColor would be a hex number right now and we're not supporting that for Icon component yet. I think @kuzhelov has an ongoing PR for that; we'll address this afterwards
fyi @mnajdova

@bmdalex bmdalex changed the title feat(Button): adding disabled prop to Button [READY FOR REVIEW] feat(Button): adding disabled prop to Button Jul 31, 2018
@bmdalex
Copy link
Collaborator Author

bmdalex commented Jul 31, 2018

@mnajdova @kuzhelov READY FOR REVIEW :)

Copy link
Contributor

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Good to be merged here.

@bmdalex bmdalex merged commit c7b7621 into master Jul 31, 2018
@bmdalex bmdalex deleted the feat/button-disabled-prop branch July 31, 2018 14:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants