deps(RHINENG-3032, RHINENG-5932): Upgrade to React 18 & PatternFly 5 #2141
Conversation
|
Waiting for RedHatInsights/frontend-components#1967 to be released so that I can continue with the cypress tests upgrade. |
gkarat
force-pushed
the
rhineng-3032
branch
2 times, most recently
from
b5c2fa2 to
3a1b59d
Compare
This is trade-off to not to block the upgrade of PF. Will be enabled back in RedHatInsights#2146.
gkarat
changed the title
There was a problem hiding this comment.
Looks great! While testing, I also did not encounter any major issues. But one small thing that caught my attention is when you select all hosts using the bulk select dropdown, there is not a loading icon inside it. But in the stage environment, we had it. Not having the icon seems to confuse users, because the selection takes time, but there is no indication that there is an action taking place.
| "@redhat-cloud-services/frontend-components-utilities": "^3.7.6", | ||
| "@redhat-cloud-services/host-inventory-client": "1.2.11", | ||
| "@data-driven-forms/common": "^3.22.2", | ||
| "@data-driven-forms/pf4-component-mapper": "^3.22.2", |
There was a problem hiding this comment.
Interesting, do we still need this pf4 mapper after we migrate to version 5?
There was a problem hiding this comment.
Looks like the package has already migrated to PF5 https://github.com/data-driven-forms/react-forms/blob/master/packages/pf4-component-mapper/package.json
| @@ -11,7 +11,7 @@ jobs: | |||
| - stage: Lint | |||
| script: npm run lint && commitlint-travis | |||
| - stage: Test | |||
| script: npm run test && npm run test:ct | |||
There was a problem hiding this comment.
I see we have still some cypress tests. Should we keep cypress tests running in travis?
There was a problem hiding this comment.
Will be re-enabled in #2146
| @@ -46,19 +46,18 @@ const TopBar = ({ | |||
| showTags, | |||
| }) => { | |||
| const dispatch = useDispatch(); | |||
| const navigate = useInsightsNavigate(); | |||
| const navigate = useInsightsNavigate('inventory'); | |||
There was a problem hiding this comment.
Maybe it is not worth it, but just an idea: can we use the appName from the package.json file? That would maybe help to have a consistent app name everywhere.
|
🎉 This PR is included in version 1.64.8 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
🪩 ✨
Implements https://issues.redhat.com/browse/RHINENG-3032, https://issues.redhat.com/browse/RHINENG-5932.
This upgrades React to use version 18, PatternFly libraries to the version 5 and also bumps related packages like RTL, FEC, redux, cypress. This PR mainly ensures that the UI doesn't introduce any visual nor functional errors, the unit tests are running fine. This PR temporarily disables Cypress tests in Travis CI runs; they will be groomed in #2146.
How to test
Make sure the application is built both for development and production environment. Ensure you are able to run it locally, the components (tables, dropdown, modals, empty states, etc.) are not broken and visually are up-to-date with PF5 or stay the same.