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

[Guided onboarding] Implement quit guide functionality #142542

Merged

Conversation

alisonelizabeth
Copy link
Contributor

@alisonelizabeth alisonelizabeth commented Oct 4, 2022

This PR implements the "Exit setup guide" functionality in the guide dropdown panel. Once the user confirms to quit a guide, an API request is made to delete the SO that is storing the guide's state.

Fixes #139804

How to test

  1. Enable guided onboarding via your kibana.dev.yml file: guidedOnboarding.ui: true
  2. Start up Kibana with the example plugins enabled: yarn start --run-examples
  3. Navigate to /app/guidedOnboardingExample
  4. Click the "Start search guide" button to trigger the guide
  5. Click the "Quit setup guide" link at the button of the dropdown panel and verify modal displays.
  6. Confirm clicking "Cancel" or the "x" icon closes the modal and maintains the guide state.
  7. Confirm clicking "Quit guide" exits the guide. The Setup guide button should be disabled (behavior will change once [Guided onboarding] Setup guide button show/hide behavior #141129 is implemented). Also, if you navigate back to /app/guidedOnboardingExample the Search guide should say "Continue".

Screenshots

Screen Shot 2022-10-10 at 9 31 13 AM

// Loading state while API request is in progress
Screen Shot 2022-10-05 at 9 04 00 AM

@alisonelizabeth alisonelizabeth added release_note:skip Skip the PR/issue when compiling release notes Team:Journey/Onboarding Platform Journey Onboarding team labels Oct 4, 2022
@gchaps gchaps added the ui-copy Review of UI copy with docs team is recommended label Oct 4, 2022
@gchaps
Copy link
Contributor

gchaps commented Oct 4, 2022

@kellyemurphy can you please review the copy in this PR?

setIsDeleting(false);
notifications.toasts.addError(error, {
title: i18n.translate('guidedOnboarding.quitGuideModal.errorToastTitle', {
defaultMessage: 'There was an error quitting the guide. Please try again.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want them to try again right away or should they wait a moment?

Copy link
Contributor Author

@alisonelizabeth alisonelizabeth Oct 5, 2022

Choose a reason for hiding this comment

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

Hmm... good question. I suppose it depends on the actual error. Do we have general guidelines around error messages when an API request fails?

Copy link
Contributor

Choose a reason for hiding this comment

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

The error message guidance that we've provided the platform (cloud) team in the past is that we want to A. tell them what happened, B. tell them what they can do about it, and C. only use please when we are asking for their patience. This message meets those guidelines, but it's just a matter of timing.

@alisonelizabeth alisonelizabeth marked this pull request as ready for review October 5, 2022 15:24
@alisonelizabeth alisonelizabeth requested a review from a team as a code owner October 5, 2022 15:24
@elasticmachine
Copy link
Contributor

Pinging @elastic/platform-onboarding (Team:Journey/Onboarding)

Copy link
Contributor

@cindychangy cindychangy left a comment

Choose a reason for hiding this comment

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

Thanks @alisonelizabeth, LGTM!

Copy link
Contributor

@yuliacech yuliacech left a comment

Choose a reason for hiding this comment

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

Thanks a lot for implementing this functionality, @alisonelizabeth!
The UI changes LGTM but I'm not sure if we should delete the progress of the guide though. I thought when the use quits the guide, the state of the guided onboarding is set to something like "inactive" so that we know that the button is not shown in the header anymore. But if the user comes back to the landing page, they can see their previous progress on all guides. And they could continue a guide from the step where they left.
If we delete the state, I'm not sure how we would handle when the user comes back to the landing page and tries to restart a guide, wdyt?

@alisonelizabeth
Copy link
Contributor Author

alisonelizabeth commented Oct 6, 2022

The UI changes LGTM but I'm not sure if we should delete the progress of the guide though. I thought when the use quits the guide, the state of the guided onboarding is set to something like "inactive" so that we know that the button is not shown in the header anymore. But if the user comes back to the landing page, they can see their previous progress on all guides. And they could continue a guide from the step where they left.
If we delete the state, I'm not sure how we would handle when the user comes back to the landing page and tries to restart a guide, wdyt?

That's a great question. @osmanis @cindychangy @bojanasan can you weigh in here? Based on the copy ("Quit this guide and discard progress?"), I thought the state would be discarded. However, I can't seem to find that in the design spec, so I might have made an assumption there 😅. If we do want to preserve the state, we should probably tweak the copy.

@cindychangy
Copy link
Contributor

cindychangy commented Oct 10, 2022

Good catch, @yuliacech is correct that when a user quits the guide - it should save their state. We have a page on the prototype to show this situation. If a user re-visits the landing page they should see their progress of all the guides, and the buttons will show different text based on that ("View guide", "Continue"...).

@alisonelizabeth: I agree given this, the copy should be tweaked as "discard progress" might be confusing if we are saving their state. @kellyemurphy do you have a suggestion on how to best tweak the title message: "Quit this guide and discard progress?".

@alisonelizabeth
Copy link
Contributor Author

Thanks @cindychangy! For now, I've changed the modal title to simply: "Quit this guide?". @kellyemurphy let me know if you'd like me to make any further changes.

@yuliacech would you mind taking another look when you have time? I updated the functionality via dc79bce.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
guidedOnboarding 16 17 +1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
guidedOnboarding 16.6KB 17.6KB +1.0KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@yuliacech yuliacech left a comment

Choose a reason for hiding this comment

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

Thanks a lot, @alisonelizabeth! Latest changes LGTM 👍

@alisonelizabeth
Copy link
Contributor Author

I'm going to go ahead and merge this PR. I can follow-up with a separate PR if there are any additional copy changes.

@alisonelizabeth alisonelizabeth merged commit 32d912e into elastic:main Oct 10, 2022
@alisonelizabeth alisonelizabeth deleted the guided_onboarding/quit_modal branch October 10, 2022 16:26
@kibanamachine kibanamachine added v8.6.0 backport:skip This commit does not require backporting labels Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Journey/Onboarding Platform Journey Onboarding team ui-copy Review of UI copy with docs team is recommended v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Guided onboarding] "Quit guide" functionality
8 participants