Migration to Patternfly v6 #21611
Draft
Migration to Patternfly v6 #21611
+2,524
−2,417
Conversation
This file contains 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
jelly
added
the
no-test
jelly
reviewed
Feb 12, 2025
| <Toolbar className="ct-alert-toolbar"> | ||
| <ToolbarContent> | ||
| <ToolbarGroup spaceItems={{ default: "spaceItemsMd" }}> |
There was a problem hiding this comment.
So this no longer does any layout, what replaces this?
There was a problem hiding this comment.
IIRC I changed this after messing about with the wrap issue. Can check soon
|
|
||
| // REVIEW @Venefilyn: Is this still true? | ||
| /* Import our own fonts since the PF4 font-face rules are filtered out in build.js */ |
There was a problem hiding this comment.
This is a good first prio target :)
Venefilyn
force-pushed
the
pf6
branch
2 times, most recently
from
243148a to
4bc7573
Compare
Venefilyn
force-pushed
the
pf6
branch
2 times, most recently
from
5f90de6 to
81053eb
Compare
Venefilyn
force-pushed
the
pf6
branch
3 times, most recently
from
9183612 to
71f7cf5
Compare
Venefilyn
force-pushed
the
pf6
branch
9 times, most recently
from
2dacca0 to
67d6842
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Run `eslint:fix` to replace tabs with spaces.
Drop unused Alert import.
Fix rule-empty-line-before
stylelint liberty/use-logical-spec
sytlelint liberty/use-logical-spec
stylelint rule-empty-line-before
In Chromium, the menu is directly hanging off `<body>`, not as a child of `#zoom-control`.
Same "menu is now detached from element" change.
What remains is the bogus screen-reader text, but let's fix this more centrally.
Restrict the hammer.gif size, as otherwise it would take the entire screen width.
Dropdown menu is no longer attached to the element but attaches directly to body.
Don't consider `.pf-v6-screen-reader` sub-elements of ph_text() elements. Current PF6 version forces some extra screen-reader only junk: patternfly/patternfly-react#11678 We will fix this properly (via PF), but it breaks a number of tests.
Makes it so it only applies to HelperTextItems
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Acceptance criteria
Notes
Set up pushing to Freya's branch:
git pull --rebaseCurrent tasks
Easiest is if we fix the codebase per test case. It should reduce the merge conflicts as well. Some test cases might overlap like networkmanager test cases. The tests might fail due to selectors not being updated or because the page or component it's testing hasn't been upgraded to Patternfly v6. They may also fail purely because of pixel changes.
Assign yourself to a test case to fix and assign yourself like this.
check-a@VenefilynIf you're done and tests pass in GitHub, or it only fails because of pixel changes and these look ok, click the checkbox. If you see that tests already pass and no one is assigned then assign yourself anyway to show that you've verified them. If the pixel change shows brokenness, judge if it is a blocker (and must be fixed in this PR), or can be a followup -- then make sure it appears in the list below.
/test/verify- unit tests @martinpitt
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
---check-apps@Venefilyncheck-bots-api@Venefilyncheck-client@Venefilyncheck-connection@Venefilyncheck-embed@Venefilyncheck-examples@Venefilyncheck-kdumpcheck-lib@martinpittcheck-loopbackcheck-metrics@martinpittcheck-networkmanager-basic@martinpitt (needs followup fix, see below)check-networkmanager-bond@martinpittcheck-networkmanager-bridge@martinpittcheck-networkmanager-checkpoints@martinpittcheck-networkmanager-firewall@martinpitt (needs followup fix for table headers, see below)check-networkmanager-mac@martinpittcheck-networkmanager-mtu@martinpitt (needs followup fix, see below)check-networkmanager-other@martinpittcheck-networkmanager-settings@martinpitt (needs followup fix, see below)check-networkmanager-team@martinpittcheck-networkmanager-unmanaged@martinpittcheck-networkmanager-vlan@martinpittcheck-networkmanager-wireguard@martinpitt (needs followup fix, see below)check-packagekit@martinpittcheck-packagescheck-pages@martinpittcheck-reauthorizecheck-selinux@martinpitt (needs followup fix, see below)check-sessioncheck-shell-active-pages@Venefilyncheck-shell-host-switching@Venefilyncheck-shell-keys@Venefilyn(needs followup fix, auto-generated pf-screen-reader text)worked aroundcheck-shell-menu@Venefilyncheck-shell-multi-machine@Venefilyncheck-shell-multi-machine-key@Venefilyncheck-shell-multi-os@Venefilyncheck-sosreport@martinpittcheck-ssh-api@martinpittcheck-static-logincheck-storage-anaconda@tomasmatuscheck-storage-basic@martinpittcheck-storage-btrfs@tomasmatuscheck-storage-formatcheck-storage-hiddencheck-storage-ignoredcheck-storage-iscsicheck-storage-luks@martinpittcheck-storage-lvm2@tomasmatuscheck-storage-mdraid@tomasmatuscheck-storage-mounting@martinpitt (needs followup fix, see below)check-storage-msdoscheck-storage-multipathcheck-storage-nfs@tomasmatuscheck-storage-partitionscheck-storage-raid1check-storage-resizecheck-storage-scalingcheck-storage-smartcheck-storage-stratis@tomasmatus -- known issue; but the pixel failure cancels the naughty match; once we accept pixel update it'll be finecheck-storage-swapcheck-storage-unrecognizedcheck-storage-unusedcheck-storage-used@martinpitt (needs follow-up fix for dialog table headers and description list)check-storage-vdocheck-superuser@martinpitt (needs follow-up fix, see below; plus the multi-host shell crash, same as with check-shell-multi-machine)check-system-infocheck-system-journalcheck-system-realms@martinpitt fixed most of itwhat remains is the auto-generated bogus pf-screen-reader textworked aroundcheck-system-services@Venefilyncheck-system-shutdown-restart@martinpitt; okexcept for the bogus pf-screen-reader textworked aroundcheck-system-terminal@martinpittcheck-system-tunedcheck-testlibcheck-users@Venefilyn (auto-generated pf-screen-reader textworked around)check-users-roles@Venefilyncheck-ws-bastion@martinpitt (it's fine)Description
To redo codemod simply reset the commit, revert changes you don't want, and run
button-moveIcons-icon-propis excluded as it only makes it have a small margin (can be added later)menuToggle-warn-iconOnly-togglesame as above icon exclusionhelperTextItem-warn-screenReaderText-updateis excluded for now as things should work still for most casesAnd for renaming CSS classes
Items fixed or documented
When we are about to merge these should all be on our minds so we can fix either in this or preferably in upcoming PRs
<Content>, need to be verified manually post-merge TextContent + Text results in two Content components patternfly/pf-codemods#825@patternfly/patternfly/*-theme-dark.cssstuff is removed due to PF Tokensreminstead--pf-v5-global--spacer--need to be renamed to--pf-t--global--spacer--EmptyStateIconPropsandEmptyStateHeaderPropsare merged intoEmptyStatePropsiframesusePagecomponent as it should but we need an override to prevent whitespace in its grid Page - Allow main grid without sidebar and masthead patternfly/patternfly#7357--pf-t--global--icon--color--status--info--defaultAll new upstream issues deriving from this upgrade
Regressions which are acceptable to be fixed in a follow-up (before next release)
Related PRs