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(button): set the disable modifier when component is not button #2683

Merged
merged 1 commit into from Sep 19, 2019

Conversation

@boaz0
Copy link
Member

boaz0 commented Aug 12, 2019

What:

closes #2666

If component is not button and isDisabled is true then add pf-m-disabled to class name.

//cc @mcoker

@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Aug 12, 2019

PatternFly-React preview: https://patternfly-react-pr-2683.surge.sh

Copy link
Contributor

mcoker left a comment

Looks good! Not sure if this is outside of the scope of this PR, but this is also a problem in the select, dropdown, and options menu. Should we fix those in this PR or is that a separate issue?

@boaz0 boaz0 force-pushed the boaz0:closes_2666 branch from d48ce8c to b831cff Sep 15, 2019
Signed-off-by: Boaz Shuster <boaz.shuster.github@gmail.com>
@boaz0 boaz0 force-pushed the boaz0:closes_2666 branch from b831cff to 2c7ac27 Sep 15, 2019
@boaz0

This comment has been minimized.

Copy link
Member Author

boaz0 commented Sep 16, 2019

fixed merge conflicts

@tlabaj

This comment has been minimized.

Copy link
Contributor

tlabaj commented Sep 16, 2019

@mcoker I can open up a follow up issue. Just to clarify, For the Select, the <div> containing the <button> should not have the pf-m-disabled modifier applied correct?

@tlabaj

This comment has been minimized.

Copy link
Contributor

tlabaj commented Sep 16, 2019

Issue #2937 has n=been opened to address the issue with the dropdown

@tlabaj
tlabaj approved these changes Sep 16, 2019
Copy link
Contributor

tlabaj left a comment

LGTM

@mcoker

This comment has been minimized.

Copy link
Contributor

mcoker commented Sep 16, 2019

@tlabaj Yeah, basically the toggle can either be a <div> or a <button>. When the toggle is a <button>, we should use the disabled attribute to disable it. If the toggle is a <div>, we should use .pf-m-disabled to disable it.

@mcoker
mcoker approved these changes Sep 16, 2019
Copy link
Contributor

mcoker left a comment

excellent!

@dlabaj
dlabaj approved these changes Sep 19, 2019
Copy link
Contributor

dlabaj left a comment

LGTM

@dlabaj dlabaj merged commit 39b359d into patternfly:master Sep 19, 2019
8 checks passed
8 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build_integration Your tests passed on CircleCI!
Details
ci/circleci: build_pf3_docs Your tests passed on CircleCI!
Details
ci/circleci: build_pf4_docs Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: test_jest_other Your tests passed on CircleCI!
Details
ci/circleci: test_jest_pf4 Your tests passed on CircleCI!
Details
ci/circleci: upload_docs Your tests passed on CircleCI!
Details
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Sep 19, 2019

Your changes have been released in:

  • @patternfly/react-core@3.104.8
  • @patternfly/react-docs@4.13.10
  • @patternfly/react-inline-edit-extension@2.11.44
  • demo-app-ts@3.0.7
  • @patternfly/react-table@2.20.24
  • @patternfly/react-topology@2.8.42
  • @patternfly/react-virtualized-extension@1.2.32

Thanks for your contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.