Skip to content

chore(Upgrade guide): removed verbiage on button aria-disabled #4494

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

Merged
edonehoo merged 3 commits into from
Mar 12, 2025
Merged

chore(Upgrade guide): removed verbiage on button aria-disabled #4494

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6c8abbb
chore(Upgrade guide): removed verbiage on button aria-disabled
Mar 10, 2025
b075f66
Updated verbiage for ariadisabled being conditionally rendered
Mar 12, 2025
66247c8
Feedback from Erin
Mar 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/documentation-site/patternfly-docs/content/get-started/upgrade.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,8 @@ If you have previously implemented any breakpoint logic based on a pixel value,
#### Button

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.
Copy link
Copy Markdown
Collaborator

@edonehoo edonehoo Mar 12, 2025

Choose a reason for hiding this comment

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

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.

- **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"`.
Copy link
Copy Markdown
Collaborator

@edonehoo edonehoo Mar 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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 for aria-disabled in buttons, when the expectation is for aria-disabled to be false.
- Match aria-disabled to isDisabled when component="button".


1. *Cannot find button attributes when using byText*
- **Cause:** We added a wrapping `div` around button text. The RTL `byText` query returns that wrapper instead of the button element itself, where button's attributes live.
Expand All @@ -199,4 +199,4 @@ Reach out to us on Slack
Ask a question in GitHub Discussions
</Button>

**Note:** If you use a custom solution to replicate PatternFly styling (without using PatternFly components), then your product will need to be re-skinned. This may be a large undertaking, so we encourage you to get help from the PatternFly team.
**Note:** If you use a custom solution to replicate PatternFly styling (without using PatternFly components), then your product will need to be re-skinned. This may be a large undertaking, so we encourage you to get help from the PatternFly team.