Skip to content

[web] Update to PatternFly v5#759

Merged
dgdavid merged 33 commits intomasterfrom
patternfly-v5
Oct 3, 2023
Merged

[web] Update to PatternFly v5#759
dgdavid merged 33 commits intomasterfrom
patternfly-v5

Conversation

@dgdavid
Copy link
Contributor

@dgdavid dgdavid commented Sep 18, 2023

Problem

PatternFly React has released its 5th version back in 27th July 2023. It comes with a lot of improvements and a bunch of breaking changes.

Agama it's young enough to be stuck in PatternFly v4.

Solution

Move to PatternFly 5 by following the upgrade guide and checking that everything work and look as expected.

Testing

  • Added a new unit test, if needed
  • Adapted needed unit tests
  • Tested manually

Note for reviewers

Please, deploy the branch locally and test the UI from the end user perspective as part of the review. Thank you

To use the PF/EmptyStateHeader component and PF/EmptyState#variant property.

Based on changes made by https://github.com/patternfly/pf-codemods.
To stop using `isSmall` and `isLarge` props and use `size` instead.

Changes made by https://github.com/patternfly/pf-codemods
Patternfly 5 has swapped the #onChange params of quite a lot components
for putting the event first. This is more aligned with the native HTML
change event, https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/change_event.

This PR, based on changes proposed by https://github.com/patternfly/pf-codemods,
update Agama components using below PF elements

  * FormSelect
    -  patternfly/patternfly-react#8998
  * Switch
    -  patternfly/patternfly-react#9037
  * TextInput
    - patternfly/patternfly-react#9064
    - patternfly/patternfly-react#9196
To be aligned with the change made to adapt PF/TextInput#onChange prop
To set the proper value after PatternFly 5 added the event as first arg
for these callback props.

See patternfly/patternfly-react#8955 and
patternfly/patternfly-react#8955

Commit based on changes made by https://github.com/patternfly/pf-codemods/
PF/ToolbarContent#alignment prop has been removed and the former
PF/ToolbarItem#alignment renamed to #align.

See patternfly/patternfly-react#8563

Changes made by https://github.com/patternfly/pf-codemods
Instead, the text is now rendered by a new Agama/FormValidationError
helper component that internally makes use of PF/FormHelperText and
PF/HelperText for wrapping it.

It fixes the `helperTextInvalid prop for FormGroup has been removed`
error thrown by https://github.com/patternfly/pf-codemods.

More info:

  - patternfly/patternfly-react#8810
  - https://www.patternfly.org/components/forms/form#examples
For fixing the `validated prop for FormGroup has been removed` error
thrown by https://github.com/patternfly/pf-codemods during migration to
PatternFly 5.

See patternfly/patternfly-react#8810
Use the new PF/Dropdown and PF/MenuToggle instead.

Related to patternfly/patternfly-react#8835 and
others.
For making the icon looks at it was previously with PatternFly v4.

Commit also adds space between paragrahps and use the
PF/EmptyStateFooter when needed.
By overriding the default PatternFly styles
For fetching the value from second argument. The first one is the event
that trigered the change.

Related to commit a6282f6
Sadly, the ActionsRow menu does not have aria-label anymore.
By running `npm update --lockfile-version 2`
@coveralls
Copy link

coveralls commented Sep 20, 2023

Coverage Status

coverage: 75.116% (+0.4%) from 74.747% when pulling b9b14dd on patternfly-v5 into 96e9308 on master.

Because it became a dummy wrapper after the changes introduced to the
Dropdown comonent by PatternFly.

The only thing that might make some sense to extract (from
Agama/WifiNetworkMenu) is the custom toggler if we end up using it in
more places.
@dgdavid dgdavid marked this pull request as ready for review October 2, 2023 07:58
Copy link
Contributor

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

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

Thanks! I have tried and it seems to work quite well.

@joseivanlopez

This comment was marked as resolved.

@dgdavid

This comment was marked as resolved.

@dgdavid dgdavid merged commit 00312cd into master Oct 3, 2023
@dgdavid dgdavid deleted the patternfly-v5 branch October 3, 2023 09:43
@dgdavid dgdavid mentioned this pull request Oct 4, 2023
dgdavid added a commit that referenced this pull request Nov 17, 2023
[web] Fix broken storage links

During [migration to PF5](#759), specifically when [moving to the new PF/Dropdown](2edc4e2#diff-88232d7f99420122a9e8c24ae183bcf9a9b98451cbb5db44dea461a2d5ed6727), a few options were unintentionally broken when their `href` prop were not properly renamed to `to`.

This [makes the PF/MenuItem to use a `<button>` instead of `<a>`](https://github.com/patternfly/patternfly-react/blob/7d2fd2678f993459f67a04d173a8d16c16ef50a3/packages/react-core/src/components/Menu/MenuItem.tsx#L139) inside an `<li>` when building the menu item, assigning the `href` HTML attribute to the latest, which has no effect at all.

As a result, users cannot navigate to these options through the UI, as it was the case in https://bugzilla.suse.com/show_bug.cgi?id=1217281
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants