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

Simplify in-progress state when submitting User Input answers #6323

Closed
wpdarren opened this issue Dec 13, 2022 · 13 comments
Closed

Simplify in-progress state when submitting User Input answers #6323

wpdarren opened this issue Dec 13, 2022 · 13 comments
Labels
P1 Medium priority Type: Enhancement Improvement of an existing feature

Comments

@wpdarren
Copy link
Collaborator

wpdarren commented Dec 13, 2022

Bug Description

This was identified as of scope when QA-ing the User Input V2 tickets, but feel we could improve the UX/UI.

You complete the 3 questions and the overview page appears. When you submit the questions, the UX/UI feels a bit weird where everything disappears other than the header of the user input form and the loading animation.

I'm wondering if it would be better experience to actually have the loading animation on the submit button, rather than the answers disappearing and being replaced with the loading animation.

Or maybe we have a similar UI when Page Speed Insights is run again. Everything is disabled and the loading animation appears at the top.

See screencast below.

ui-2.mp4

Steps to reproduce

  1. Set UserInput in the tester plugin
  2. Setup Site Kit
  3. Go to the Site Kit Dashboard and User Input form will appear.
  4. Go through the questions and click on submit.
  5. Observe UX/UI behaviour.

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • Clicking the "Submit" button on the User Input Review page should not replace everything on the page with a Progress bar to indicate "in-progress" state.
  • Instead, there should be a spinner added to the submit button to indicate the "in-progress" state.
    • The "Submit" button should be disabled while the questions are being submitted.
    • The "Cancel" button should be disabled while the questions are being submitted.
    • Any answers (eg. the radio buttons, checkboxes, etc.) should not allow user interaction (be disabled) while questions are being submitted.
    • Everything else in the page should remain in place.
  • Once submitted, user should be navigated back to the dashboard.
  • If the submission fails (ie. network error), the "Submit" button, along with everything else that were disabled should be re-enabled again and the error message should appear alongside the submit button.

Implementation Brief

  • Inside google-site-kit/assets/js/components/user-input/UserInputQuestionnaire.js:
    • Remove isSavingSettings definition and isSavingSettings from condition to render <ProgressBar />
  • Inside google-site-kit/assets/js/components/user-input/UserInputPreview.js:
    • Use isSavingUserInputSettings in render to disable Submit Button + Back link, and render <Spinner />
  • Inside google-site-kit/assets/js/components/user-input/UserInputSelectOptions.js:

Test Coverage

  • VRT may need an update

QA Brief

  • Submit user input goals
  • Progress bar should be removed, inputs should be disabled and the spinner should indicate progress as per the screenshot

Screenshot 2023-01-08 at 00 54 46

Changelog entry

  • Update the in-progress state when submitting User Input answers.
@wpdarren wpdarren added Type: Enhancement Improvement of an existing feature UX Issues that require UX input labels Dec 13, 2022
@aaemnnosttv aaemnnosttv added P1 Medium priority UX Issues that require UX input and removed UX Issues that require UX input labels Dec 13, 2022
@aaemnnosttv aaemnnosttv changed the title Improve UX/UI when submitting User Input questions Simplify in-progress state when submitting User Input answers Dec 13, 2022
@aaemnnosttv aaemnnosttv removed the UX Issues that require UX input label Dec 13, 2022
@kuasha420 kuasha420 removed their assignment Dec 19, 2022
@tofumatt tofumatt self-assigned this Dec 20, 2022
@tofumatt
Copy link
Collaborator

ACs largely look good, just had to add that the cancel button and answers should also be disabled during submission. 👍🏻

Moved to IB.

@tofumatt tofumatt removed their assignment Dec 20, 2022
@sashadoes sashadoes self-assigned this Dec 20, 2022
@kuasha420
Copy link
Contributor

kuasha420 commented Dec 21, 2022

@tofumatt Thanks, I didn't include disabling everything because of the slack conversation, but I like everything being disabled. I've just updated the last point to enable everything back if the submission fails. Cheers.

@sashadoes sashadoes removed their assignment Dec 26, 2022
@aaemnnosttv aaemnnosttv self-assigned this Jan 4, 2023
@aaemnnosttv
Copy link
Collaborator

  • Declare isSavingSettings variable same as it was in UserInputQuestionnaire.js

@sashadoes looking at this, the isSavingUserInputSettings in UserInputQuestionnaire is actually used improperly (it does not take any arguments, it only returns internal state). Let's simplify this to reference the selector by name rather than copying the implementation from elsewhere which we don't want to copy for this reason :) While we're at it, let's add a separate point to correct the usage in the questionnaire component.

  • Locateitems being created (in line 122) and find a place where props get configured

For specific references, please use a permalink as line number and other references can easily get outdated between now and when the issue is executed.

@aaemnnosttv aaemnnosttv assigned sashadoes and unassigned aaemnnosttv Jan 4, 2023
@sashadoes sashadoes assigned aaemnnosttv and unassigned sashadoes Jan 5, 2023
@aaemnnosttv
Copy link
Collaborator

  • Use isSavingUserInputSettings in render to disable Submit Button, hide Back link, and render <Spinner />

The AC calls for the submit button, cancel button, and answers to be disabled and "Everything else in the page should remain in place." so I don't think we should hide the "Back" link. This should be disabled as well (essentially everything interactive in that panel should be disabled while in progress). I'll make this tiny change to the IB and then this is good to go 👍

IB ✅

@aaemnnosttv aaemnnosttv removed their assignment Jan 5, 2023
@sashadoes sashadoes self-assigned this Jan 6, 2023
@sashadoes sashadoes removed their assignment Jan 8, 2023
@hussain-t hussain-t assigned hussain-t and sashadoes and unassigned hussain-t Jan 11, 2023
@wpdarren
Copy link
Collaborator Author

QA Update: ❌

@sashadoes I have an observation detailed below.

The spinner animation does appear when you click on the save button, which is a much better user experience, but when I compare it to the screenshot in the QAB, the spinner looks different and not consistent with other buttons that we have the spinner on. See screencast below.

animation-1.mp4

@wpdarren wpdarren assigned wpdarren and sashadoes and unassigned wpdarren Jan 16, 2023
@sashadoes sashadoes mentioned this issue Jan 16, 2023
18 tasks
@sashadoes
Copy link
Collaborator

Addressed with the fix above by removing <SpinnerButton> and using <Button> with <Spinner> instead.
Going a bit further, I see the <SpinnerButton> is a more convenient way to use in such scenarios. However, it uses <CircularProgress> instead of <Spinner> which causes an above issue with inconsistency in design. On the other hand, there are lots of places where <Button> works together with <Spinner>. Not sure, we should keep both though...
Considering all the above, could you suggest if the following ticket to use <SpinnerButton> should be created or adjust <SpinnerButton> to use different spinners or maybe leave all as it is now? @aaemnnosttv @eugene-manuilov

@sashadoes sashadoes removed their assignment Jan 16, 2023
@eugene-manuilov
Copy link
Collaborator

The spinner animation does appear when you click on the save button, which is a much better user experience, but when I compare it to the screenshot in the QAB, the spinner looks different and not consistent with other buttons that we have the spinner on. See screencast below.

@wpdarren, the spinner that you see in your screencast is the right spinner that everyone should see every time we need a button with the spinner. If you see some buttons have spinners like on the screenshot in QAB, then it is a bug and incorrectly implemented spinner that we should fix to get our UI consistent.

@sashadoes, could you, please, close your PR #6414? The spinner that you added is the correct one.

@kuasha420
Copy link
Contributor

@eugene-manuilov Do you think it's worthwhile to create a separate issue to refactor incorrect implementations of button with spinner with the correct one?

@eugene-manuilov
Copy link
Collaborator

Yes, sure. I would let @wpdarren to create it since he has seen similar buttons in other places. Or if he can't, I'll create a ticket tomorrow.

@wpdarren
Copy link
Collaborator Author

Thanks @eugene-manuilov I will create a ticket because I do see the spinner in the QAB screenshot in other places in the plugin. This is the first time I have seen this one. c.c @kuasha420 @sashadoes

@wpdarren
Copy link
Collaborator Author

wpdarren commented Jan 16, 2023

QA Update: ⚠️

@sashadoes @eugene-manuilov I checked over the AC and noticed some aspects of this ticket that weren't covered in the QAB.

Any answers (eg. the radio buttons, checkboxes, etc.) should not allow user interaction (be disabled) while questions are being submitted.

The radio buttons, checkboxes were disabled but I am still able to click on the edit link to expand the questions. Would it not be better UI to make the edit links disabled too?

@eugene-manuilov
Copy link
Collaborator

The radio buttons, checkboxes were disabled but I am still able to click on the edit link to expand the questions. Would it not be better UI to make the edit links disabled too?

@wpdarren, I think that's fine since expanding or collapsing questions doesn't change anything. But if you want, you can double check with Evan or a UX designer.

@eugene-manuilov eugene-manuilov removed their assignment Jan 17, 2023
@wpdarren
Copy link
Collaborator Author

wpdarren commented Jan 17, 2023

QA Update: ✅

@eugene-manuilov thanks, I won't hold this ticket up, but will flag my observation with the UX team.

Verified:

  • Clicking the "Submit" button on the User Input Review page no longer replaces everything on the page with a Progress bar to indicate "in-progress" state.
  • There is instead a spinner added to the submit button to indicate the "in-progress" state.
  • Once submitted, user is navigated back to the dashboard.
  • If the submission fails (ie. network error), the "Submit" button, along with everything else that were disabled is re-enabled again and the error message should appear alongside the submit button.
animation-1.mp4

@aaemnnosttv aaemnnosttv self-assigned this Jan 27, 2023
@aaemnnosttv aaemnnosttv removed their assignment Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Medium priority Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

7 participants