chore(Upgrade guide): removed verbiage on button aria-disabled#4494
Merged
edonehoo merged 3 commits intopatternfly:mainfrom Mar 12, 2025
Merged
chore(Upgrade guide): removed verbiage on button aria-disabled#4494edonehoo merged 3 commits intopatternfly:mainfrom
edonehoo merged 3 commits intopatternfly:mainfrom
Conversation
Collaborator
thatblindgeye
force-pushed
the
upgradeAriaDisabledButton
branch
from
087fd4b to
b075f66
Compare
Contributor
Author
|
@edonehoo would you mind taking a look at this update? i originally put this PR up before one of yours went in that updated this page a bit and just pushed a tweak based on a convo Titani and I had yesterday. |
edonehoo
reviewed
Mar 12, 2025
Collaborator
edonehoo
left a comment
edonehoo
left a comment
There was a problem hiding this comment.
I left a couple of options for line 176, because I wasn't 100% sure if I was reading that right. If both are wrong, lmk 😅
| 1. *Cannot find aria-disabled* | ||
| - **Cause:** We changed the `isDisabled` prop to assign a value for `disabled`, but none for `aria-disabled`. | ||
| - **Fix:** Remove tests that look for `aria-disabled` in buttons. | ||
| - **Cause:** We changed button's `isDisabled` prop to assign a value for `disabled`, but none for `aria-disabled` - except for when the `component` prop is passed anything other than "button" as a value. Additionally, `aria-disabled` will now only render when true. |
Collaborator
There was a problem hiding this comment.
Suggested change
| - **Cause:** We changed button's `isDisabled` prop to assign a value for `disabled`, but none for `aria-disabled` - except for when the `component` prop is passed anything other than "button" as a value. Additionally, `aria-disabled` will now only render when true. | |
| - **Cause:** We changed button's `isDisabled` prop to assign a value for `disabled`, but none for `aria-disabled`. The exception to this is when the `component` prop is passed anything other than "button" as a value. Additionally, `aria-disabled` will now only render when true. |
| - **Cause:** We changed the `isDisabled` prop to assign a value for `disabled`, but none for `aria-disabled`. | ||
| - **Fix:** Remove tests that look for `aria-disabled` in buttons. | ||
| - **Cause:** We changed button's `isDisabled` prop to assign a value for `disabled`, but none for `aria-disabled` - except for when the `component` prop is passed anything other than "button" as a value. Additionally, `aria-disabled` will now only render when true. | ||
| - **Fix:** Remove tests that look for `aria-disabled` in buttons when the expectation is for it to be false, or to match `isDisabled` when `component="button"`. |
Collaborator
There was a problem hiding this comment.
Suggested change
| - **Fix:** Remove tests that look for `aria-disabled` in buttons when the expectation is for it to be false, or to match `isDisabled` when `component="button"`. | |
| - **Fix:** Remove tests that look for `aria-disabled` in buttons when the expectation is either: | |
| - For `aria-disabled` to be false. | |
| - For `aria-disabled` to match `isDisabled` when `component="button"`. |
I'm not sure if I'm correctly interpreting the end of the sentence. I took it like there are 2 instances where you should remove these tests (the suggestion above and also below)
I can also interpret it like this:
Fix: Either:
- Remove tests that look foraria-disabledin buttons, when the expectation is foraria-disabledto be false.
- Matcharia-disabledtoisDisabledwhencomponent="button".
edonehoo
approved these changes
Mar 12, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on patternfly/patternfly-react#11619