-
Notifications
You must be signed in to change notification settings - Fork 803
Changed color of continue button of welcome modal to grey, added composable useTour and added TooltipTour component #13488
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
Changed color of continue button of welcome modal to grey, added composable useTour and added TooltipTour component #13488
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TooltipTour
looks great and is very responsive! One thing that is important to do is to continuously pull from the develop branch so that you have the latest changes and there won’t be any merge conflicts when you open a PR.
packages/kolibri-common/components/onboarding/TooltipContent.vue
Outdated
Show resolved
Hide resolved
packages/kolibri-common/components/onboarding/TooltipContent.vue
Outdated
Show resolved
Hide resolved
kolibri/plugins/learn/assets/src/views/ChannelCardGroupGrid.vue
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @shruti862 - nice work on this! It's very close to being ready to go. I know Liana is going to do a bit of rereview, but I have two blocking comments regarding dependencies - I think the package.json, package-lock.json and yarn-lock.json changes are not actually needed for this PR and perhaps are some artifacts of your linting or local setup. In that case, we won't want to include them in the files that we merge.
I did do some manual testing, and I know Liana has as well, and overall the interactions are looking good :) We will also ask our QA team to do some review, but I think we are getting close to being ready to merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like Marcella mentioned, this is close to being ready to go. Our goal is to get this PR in a mergeable state, so you can also remove the [DO NOT MERGE] section from the PR title. I've added a few more notes below.
kolibri/plugins/learn/assets/src/strings/kolibriOnboardingGuideStrings.js
Outdated
Show resolved
Hide resolved
Done :) |
Build Artifacts
|
…osable useTour and added TooltipTour component
0a78dcc
to
0c45754
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a few linting errors due to some items that need to be tweaked and cleaned up :)
kolibri/plugins/learn/assets/src/views/ChannelCardGroupGrid.vue
Outdated
Show resolved
Hide resolved
kolibri/plugins/learn/assets/src/views/ChannelCardGroupGrid.vue
Outdated
Show resolved
Hide resolved
packages/kolibri-common/components/onboarding/TooltipContent.vue
Outdated
Show resolved
Hide resolved
@radinamatic @pcenov Tagging QA to check for regressions. This PR introduces the onboarding tooltip tour for users setting up Kolibri with an “On My Own” super admin account, this is the only type of account that should see the onboarding tour. Full accessibility will be addressed in the next PR, we mainly need to ensure there are no regressions on the Library page, especially for first-time users who did not use the “On My Own” device setup.
|
The requested changes have been addressed
Tested the DEB asset in Firefox and Chrome on Ubuntu, no regressions observed on Library page. Looking forward to testing the keyboard navigation and other accessibility features of these onboarding messages 👍🏽 Not sure how these tooltips relate to , for example, accessibility of the modals, but a part from working on the keyboard navigation, we should also think about the what would be announced by the screen-reader (couldn't see anything in the Figma). One thing that comes to mind that could serve as a title for the tooltip, is the step order: |
Thank you for bringing that up, making sure the tooltip content is screen reader compatible is one of the next priorities. I think a non-visible title indicating the page and the onboarding step would be a good addition! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QA has confirmed there are no regressions and the previous comments have been resolved, this is good to merge!
Summary
This PR lays the groundwork for the onboarding TooltipTour experience for new users.
Specifically:
Changed the color of "Continue" button in the Welcome Modal to match the design spec (grey background).
Added a new useTour composable to manage the state of onboarding tooltip tours across the application.
1. It exposes reactive steps and a tourActive flag.
2. Provides helper methods:
--> registerStep({ key, content, stepIndex }) — for dynamically registering tooltip steps from various components.
--> startTour() — to activate and sort the steps before starting.
--> endTour() — to cleanly terminate the tour.
Created a reusable TooltipTour Vue component which uses Tippy.js to render tooltip elements relative to DOM targets.
Tooltips include:
--> Continue and Back buttons for navigation across steps
--> Close (“X”) button to end the tour which resets state (tourActive = false)
-->The tooltip content is dynamically injected using a TooltipContent Vue component instance.
Attaching screenshot & screencast for the work done so far:
Screencast.from.02-07-25.10.47.57.PM.IST.webm
References
Related issue: #13521
Design spec for onboarding tooltips : link
Reviewer guidance
Initial Testing: "On My Own" (super admin) device setup screen.