Skip to content

ACM-13499 Remove deprecated PF5 components#4812

Merged
openshift-merge-bot[bot] merged 21 commits intostolostron:mainfrom
edewit:ACM-13499
Aug 25, 2025
Merged

ACM-13499 Remove deprecated PF5 components#4812
openshift-merge-bot[bot] merged 21 commits intostolostron:mainfrom
edewit:ACM-13499

Conversation

@edewit
Copy link
Copy Markdown
Contributor

@edewit edewit commented Jul 28, 2025

Signed-off-by: Erik Jan de Wit erikjan.dewit@gmail.com

📝 Summary

Ticket Summary (Title):

Remove deprecated PF5 components in patternfly-labs/react-form-wizard

Ticket Link:

https://issues.redhat.com/browse/ACM-13499

Type of Change:

  • 🐞 Bug Fix
  • ✨ Feature
  • 🔧 Refactor
  • 💸 Tech Debt
  • 🧪 Test-related
  • 📄 Docs

✅ Checklist

General

  • PR title follows the convention (e.g. ACM-12340 Fix bug with...)
  • Code builds and runs locally without errors
  • No console logs, commented-out code, or unnecessary files
  • All commits are meaningful and well-labeled
  • All new display strings are externalized for localization (English only)
  • (Nice to have) JSDoc comments added for new functions and interfaces

If Feature

  • UI/UX reviewed (if applicable)
  • All acceptance criteria met
  • Unit test coverage added or updated
  • Relevant documentation or comments included

If Bugfix

  • Root cause and fix summary are documented in the ticket (for future reference / errata)
  • Fix tested thoroughly and resolves the issue
  • Test(s) added to prevent regression

🗒️ Notes for Reviewers

@edewit edewit force-pushed the ACM-13499 branch 8 times, most recently from 16b0511 to e1652c7 Compare July 31, 2025 12:41
@edewit edewit requested a review from jeswanke July 31, 2025 16:38
@edewit edewit force-pushed the ACM-13499 branch 6 times, most recently from d3ddca2 to 92c3702 Compare August 7, 2025 10:26
@edewit
Copy link
Copy Markdown
Contributor Author

edewit commented Aug 7, 2025

/retest

@edewit edewit force-pushed the ACM-13499 branch 2 times, most recently from 55a905d to d084be9 Compare August 7, 2025 16:31
@edewit
Copy link
Copy Markdown
Contributor Author

edewit commented Aug 7, 2025

/retest

Copy link
Copy Markdown
Contributor

@KevinFCormier KevinFCormier left a comment

Choose a reason for hiding this comment

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

I left one inline comment, and I have these comments about styling. (In these screen captures, your updated code appears on the left.)

  1. I think we should have quotation marks around the value for the new option. PatternFly also shows this in their example: https://v5-archive.patternfly.org/components/menus/select/#multiple-typeahead-with-create-option
    image
  2. This styling seems a little odd, because "No results found" looks like a currently selected value, and the menu footer doesn't look right. Are you using MenuFooter? https://v5-archive.patternfly.org/components/menus/select#with-a-footer
    image
  3. Previously, WizSelect only had a clear button if it was not a required field. Is this change intentional?
    image
  4. When I addd a new option to this Values dropdown, the text I typed remains in place:
    image

Comment thread frontend/packages/react-form-wizard/src/inputs/InputSelect.tsx Outdated
@edewit
Copy link
Copy Markdown
Contributor Author

edewit commented Aug 19, 2025

I fixed 1-3 but for four I think this is the right thing to keep what you typed as it's a multi select and you might want to select multiple options that contain your text

@edewit edewit requested a review from KevinFCormier August 19, 2025 11:56
@edewit
Copy link
Copy Markdown
Contributor Author

edewit commented Aug 19, 2025

/retest

Copy link
Copy Markdown
Contributor

@KevinFCormier KevinFCormier left a comment

Choose a reason for hiding this comment

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

Thanks for the updates. These look great!

There's one more bug I'm seeing.

  1. Select or enter a URL
  2. Click into the field again without selecting the text and try to type

No characters are entered, and the cursor moves to the end. The dropdown shows "Create new option..." with the last character typed appended to the current value. Typing additional characters just replaces this character.

image

@edewit edewit requested a review from KevinFCormier August 21, 2025 08:27
@KevinFCormier
Copy link
Copy Markdown
Contributor

@edewit Thanks for fixing that bug I mentioned. One other thing is that we are offering to create a new option as soon as the user clicks in the field. I don't think we should offer to create the empty string option.

Also, can you either rebase your branch or merge the latest changes from main? You will need this to pass the Konflux enterprise-contract checks.

image

edewit and others added 15 commits August 25, 2025 16:25
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: John Swanke <jswanke@redhat.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
don't show "no results" as selected
removed double MenuFooter

Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>
@KevinFCormier
Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm label Aug 25, 2025
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Aug 25, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edewit, KevinFCormier

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [KevinFCormier,edewit]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants