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

docs(upgrade): Adds more detail to upgrade guide. #4332

Merged
merged 2 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 1 addition & 1 deletion packages/documentation-site/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"screenshots": "pf-docs-framework screenshots"
},
"dependencies": {
"@patternfly/documentation-framework": "6.0.0-alpha.117",
"@patternfly/documentation-framework": "6.0.0-alpha.118",
"@patternfly/react-catalog-view-extension": "6.0.0-prerelease.1",
"@patternfly/react-console": "6.0.0-prerelease.2",
"@patternfly/react-docs": "7.0.0-prerelease.32",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,22 @@ This codemod works both for CSS variables and React tokens. For example:
- A CSS variable: `--pf-v5-global--FontSize--lg` becomes `--pf-t--global--font--size--lg`
- A react token: `global_FontSize_lg` becomes `t_global_font_size_lg`

### 3. Update any pixel-based logic

In PatternFly 6, [we've transitioned from using pixels to using rem units,](/get-started/release-highlights#rem-units) which are relative units that adjust font size based on the HTML document root element size. The root size in PatternFly is 16px, meaning 16px would be 1rem, 24px would be 1.5rems, and so on.

If you have previously implemented any logic based on a pixel value, you will need to account for the fact that PatternFly 6 size and dimension tokens use rems. Dividing pixel values by 16 will give you the equivalent rem value to use.

### Potential test failures

There are a few test failures that you're likely to encounter:

1. **Button:** Cannot find `aria-disabled`
- We changed button's `isDisabled` prop to assign a value for `disabled`, but none for `aria-disabled`. As a result, any test that looks for `aria-disabled` may fail.
1. **Button:** Cannot find `disabled` when using `byText`
edonehoo marked this conversation as resolved.
Show resolved Hide resolved
- There's a new wrapping `div` around text in buttons. The RTL `byText` query returns that wrapper instead of the button element itself, which is where button's attributes live. Instead of `byText`, use `byRole` and pass the button text to `name`. This will return the top-level button element.
1. **Select (when using Jest):** Cannot find `role`
- You may get an "unable to find role" error if the Popper menu is set to `aria-disabled` after a selection is made, because the RTL query can't find the menu options. This error only seems to occur with Jest, rather than within the browser. To resolve this, either:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
1. **Select (when using Jest):** Cannot find `role`
- You may get an "unable to find role" error if the Popper menu is set to `aria-disabled` after a selection is made, because the RTL query can't find the menu options. This error only seems to occur with Jest, rather than within the browser. To resolve this, either:
1. **Select (when using React Testing Library):** Cannot find `role`
- You may get an "unable to find role" error if the Popper menu is set to `aria-disabled` after a selection is made, because the RTL query can't find the menu options. This error only seems to occur in unit tests, rather than within the browser. To resolve this, either:

It's more of an RTL thing than Jest, at least I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought we were talking about right-to-left ahhaha

Amazing, ty!

Copy link
Collaborator

Choose a reason for hiding this comment

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

That is very much not the first time that RTL vs RTL confusion has happened haha

- Pass in the `{hidden: true}` option.
- Change select's `appendTo` to `inline`.

Loading