Display modal with more info about the new homescreen #4890
Conversation
samueljseay
added
focus: home screen
client/task-list/style.scss
Outdated
| .components-guide__finish-button, | ||
| .components-guide__forward-button, | ||
| .components-guide__back-button { | ||
| position: initial; |
There was a problem hiding this comment.
There isn't a lot of control you can have over the rendering of the buttons in the Guide, so I had to mimic styles from other button types like secondary in the styles to match the designs.
I think we should be able to retain some of the existing logic to satisfy when to display this guide. From the issue:
So the existing modal with the 🚀 is shown after completing the profiler, not sure about if its shown after skipping the profiler. The other use case of someone visiting the home screen for the first time might require some additional logic - that flow would be a user whom already has a store setup, and then opts-in to the homescreen, or after we make it the default on all stores, sees it for the first time. Thinking we might need to track that view in an option somewhere. Perhaps we could land this with the coverage for the profiler completion scenario, then do a follow-up for the other scenario. Up to you Sam. |
|
@timmyc yeah I backed out of my original implementation and put this "in progress" to move the modal out of task-list (where the old one was). I'm adjusting the logic to cover all the scenarios now, which I think makes more sense.
Yes this requirement especially is what required a significant refactor. The task list may not be in place when we need to show the modal so the modal needs to be higher up the component tree. |
Adjust the rendering of the welcome modal for all scenarios covered in the requirements.
|
@timmyc I've refactored the modal now, moving it outside the task list, but using the original option. Note these acceptance criteria from the issue:
The way I see these 2 requirements, it basically boils down to: if its your first time on the homescreen (or you've never dismissed the modal before), show the modal. What do you think? |
joshuatf
left a comment
joshuatf
left a comment
There was a problem hiding this comment.
Great refactor, Sam! This is testing well for me, just left a couple quick fixes in the code, but pre-approving.
The way I see these 2 requirements, it basically boils down to: if its your first time on the homescreen (or you've never dismissed the modal before), show the modal. What do you think?
I think the logic for showing this makes sense. Tested coming from onboarding profiler flows and with onboarding disabled and both cases worked as expected.
|
|
||
| jest.mock( 'lib/tracks', () => ( { recordEvent: jest.fn() } ) ); | ||
|
|
||
| describe( 'WelcomeModal', () => { |
joshuatf
added
needs: author feedback
|
@joshuatf I've addressed your points, thanks so much for that! |
samueljseay
added
[Status] Ready to Merge
and removed
needs: author feedback
Fixes #4590
This implements a paged modal via the Gutenberg
Guidecomponent. Some styling adjustment was needed to match the designs exactly.This also introduces 3 SVG based illustrations for display in the modal. According to this comment there is a need to color them based on the theme at some point, but based on a current lack of requirements for that, I felt that could be done at a separate time (that was suggested in the comment as well).
Accessibility
Screenshots
Detailed test instructions: