Wizard: prevent reloading of Wizard on enter#4258
Conversation
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider extracting the inline onSubmit handler into a named function (e.g., handleSubmit) to keep the JSX cleaner and make the intent of preventing default submission easier to reuse or adjust if the step’s behavior changes later.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the inline onSubmit handler into a named function (e.g., handleSubmit) to keep the JSX cleaner and make the intent of preventing default submission easier to reuse or adjust if the step’s behavior changes later.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #4258 +/- ##
==========================================
- Coverage 71.54% 71.52% -0.02%
==========================================
Files 199 199
Lines 7524 7526 +2
Branches 2813 2813
==========================================
Hits 5383 5383
- Misses 1867 1869 +2
Partials 274 274
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
Oh, this is great catch! This might be an issue in general, not just for the hostname input, right? |
|
@regexowl Yes, and we should maybe try to catch this even in our playwright tests, what do you think? I did not see this happening anywhere else, but the "Form" component behaves like this normally. |
In LabelInput, the Wizard requires users to press enter. Although the hostname step does not need it, users might press enter without knowing it is not needed, which previously trigerred a re-render. This commit fixes the re-render.
ebbfb8c to
e2eed47
Compare
Agree, we should
Just tried and the repositories search input behaves the same way. What solves it is adding a handler for the submit and then passing that to the Managed to resolve it for the Repositories and packages step: #4263 (had to gate the forms first and then add the handler) We can either start gating the other steps in the same way, or we can put a big pin in this and revisit once the steps are rendered correctly without the nesting. What do you think? |
|
@regexowl I am okay with revisiting after the revamp is done. Well, we could use the preventDefault for the fields where we noticed this behaviour, and create a ticket in order to not forget fixing this later in a more elegant way + adding tests. What do you think? |
Sure! We can always revise the problematic inputs later. Let's get this in, thank you! 🚀 |
|
Opened a task for the tests: https://redhat.atlassian.net/browse/HMS-10462 we can revise the functionality within it |
I noticed this bug during the UX call.
In LabelInput, the Wizard requires users to press enter. Although the hostname step does not need it, users might press enter without knowing it is not needed, which previously trigerred a re-render. This commit fixes the re-render.