-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Customer Home: Add a welcome banner. #39557
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.
Thanks @kwight, giving tentative approval here 👍
Verified that the banner dismisses as a user preference, and dispatches the expected analytics.
I think the following screens are okay, but might be visually a lot to process:
- New User, launches site:
- New User completes checklist:
As an aside, what do folks think about adding white-space:no-wrap
to the buttons on the page. It looks pretty weird when they wrap to two lines:
I agree with @gwwar. Those screens are a lot to process visually. The congratulatory messages should ideally be using the same component. It's a bit strange that they're in different locations with their own visual styles. Can we align those, @danhauk? I'm also questioning whether both the welcome and congratulatory banners should be displayed together. If you've reached the launched your site step, is a welcome banner still relevant?
We should definitely do that. :) |
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Sections (~297 bytes added 📈 [gzipped])
Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
I agree. If there's a congratulatory banner present we should mark the welcome banner as dismissed so it doesn't show up. I'm not sure this will really be an issue if we add an age check to only show the welcome banner for users who have older sites as mentioned in #39557 (comment), but it's best to add that check to be safe. @kwight Can we also suppress any upsells to give priority to this welcome banner and avoid stacking large banner cards? Things like this: |
Yep, agree; #39609 for consideration. @lcollette |
2087eb7
to
a040c14
Compare
To summarize our conditions, we display the welcome banner if:
Also, we only display the upsell banner if we are not displaying the welcome banner. I think I've got that all covered now 👍 |
Thanks Kirk, this is good to go after removing the site launch check. |
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.
Thanks, Kirk!
* Moves all Home notices into their own components. * It also consolidates all of them into the same celebrate format we started to use as a pattern for this kind of notices. * The introductory welcome banner added for users that have never been redirected there (#39557) has been removed here in order to clean up a bit further the codebase. Likely, most of active users have already seen that banner so it should be safe to remove it
Add a welcoming banner to Customer Home, so that users that have never been redirected there before can understand why they're there and how the page can help them.
Fixes #39178
Testing Instructions
localStorage.debug = 'calypso:analytics:*'
.X
to close./me/preferences
was made./me/preferences
with{"dismissible-card-home-welcome-banner-dismissed":null}
in thecalypso_preferences
body.