Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V2Wizard: Review step improvements (HMS-4085) #2072

Merged
merged 5 commits into from
Jun 6, 2024

Conversation

regexowl
Copy link
Collaborator

@regexowl regexowl commented May 8, 2024

Fixes #920

This improves the Review step as per recent mocks.

@regexowl
Copy link
Collaborator Author

regexowl commented Jun 6, 2024

/retest

Copy link
Collaborator

@mgold1234 mgold1234 left a comment

Choose a reason for hiding this comment

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

looks perfect, thank you Klara,
I think maybe we need to add a button at every revisit step, to give the user the ability to
"go back to review" step

@regexowl
Copy link
Collaborator Author

regexowl commented Jun 6, 2024

Great point! There is an option to jump back to Review by using the left navigation panel but it may not be obvious to the user:

review-jump

I think we should be able to add a button to the footer for the edit mode of wizard.

@regexowl
Copy link
Collaborator Author

regexowl commented Jun 6, 2024

@jrusz pr_check is unhappy with this one as well. Can you please give it a look when you have time? What could be the problem is a newly added modal that pops up when the user clicks on "Create blueprint" at the Review step:

image

It should pop up only once. Whether it was already seen or not is tracked in localStorage as imageBuilder.saveAndBuildModalSeen. When it's closed once it won't appear again and it shouldn't appear when the user uses the "Create blueprint and build image(s)" option.

The other two changes were:

  • expandables are expanded by default and "Revisit step" was added to each, allowing to jump to the specific step
  • the name of the blueprint was added to the header and the optional blueprint description is rendered right below it

@jrusz
Copy link
Collaborator

jrusz commented Jun 6, 2024

@jrusz pr_check is unhappy with this one as well. Can you please give it a look when you have time? What could be the problem is a newly added modal that pops up when the user clicks on "Create blueprint" at the Review step:

image

It should pop up only once. Whether it was already seen or not is tracked in localStorage as imageBuilder.saveAndBuildModalSeen. When it's closed once it won't appear again and it shouldn't appear when the user uses the "Create blueprint and build image(s)" option.

The other two changes were:

  • expandables are expanded by default and "Revisit step" was added to each, allowing to jump to the specific step
  • the name of the blueprint was added to the header and the optional blueprint description is rendered right below it

Yeah well since ephemeral env is always fresh it can't remember this option. I'll add this to the test suite.

EDIT: Actually it seems that the expanded sections is the issue here. I'll try to fix it all at once.

EDIT2: So in the end it was because of the title change and the modal. The expanded sections idk because I've accidentally removed the assert on them but I can't add it back until I figure out why does package search not work in ephemeral, but no time for that now. Fix is on the way.

@jrusz
Copy link
Collaborator

jrusz commented Jun 6, 2024

/retest

@regexowl
Copy link
Collaborator Author

regexowl commented Jun 6, 2024

Also all green, thank you @jrusz!

This makes all expandables be open by default and adds a Revisit button to each of the expandable headers.
When the button "Create blueprint" is clicked for the first time, a modal about "Save and build" functionality is opened.
When all the expandables are open by default the extra line breaks looked a bit weird so this removes them.
This updates tests with a `openAndDismissSaveAndBuildModal` function that handles closing the SaveAndBuild modal after clicking on Create blueprint for the first time.
@kelseamann
Copy link

hey! you all are awesome thanks for pointing this out. For the "Review and finish" button I believe the criteria now is {user is in the optional steps menu}. We can change that to {user has completed all required steps}. However, I can foresee an issue where the user goes back to a required step and removes some option, and then all of a sudden the button is jumping in and out of the menu.

My solution here would be- say it with me now- disable the button until they can use it. The "review" button can always live in the footer, I think that's a great idea, we will just disable it until the user has filled in all the required fields. Does this sound ok? @regexowl @mgold1234 I can make you a mock but the mock is essentially enable button if {all required fields are met}.

@kelseamann
Copy link

I also like this solution from an HCI perspective- if user can see the button the whole time but aren't able to access it, they get some little sense of completion from using it each time :)

@regexowl
Copy link
Collaborator Author

regexowl commented Jun 6, 2024

@kelseamann Sounds good to me! Great point with jumping in and out of the menu. Form based validation is in the works so we could maybe use it to disable/enable the button as needed when it's available.

As the button is not a blocker I've created an issue for it so we can merge this PR: #2127

I also like your note about little sense of completion - it's like: "You've made it to this point and unlocked fast travel, congrats!" 😄

@regexowl regexowl merged commit a23cc93 into osbuild:main Jun 6, 2024
5 checks passed
@regexowl regexowl deleted the review-step-update branch June 6, 2024 14:59
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.

Update Review step to have direct "Edit" links
5 participants