-
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
Plans: Update styling for Jetpack CRM card to match other product cards #57553
Conversation
Link to Calypso live: https://calypso.live?image=registry.a8c.com/calypso/app:build-18501 |
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Sections (~395 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. |
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.
As for the grid, the Jetpack Free card doesn't take the full height at medium size viewport: @jeffgolenski I wonder what we can do to address this so that the Free card doesn't look wonky next to other cards. I tried stretching the card out vertically in the screenshot below, but as you can see, the buttons don't line up. Should Free also have a "Start for Free" price display to match CRM Free? |
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.
a6fff51
to
df7c881
Compare
@mattgawarecki Here's a really quick mock. During that middle breakpoint when the 2 columns looks like this... Can you make CRM and Jetpack Free both span 100% and stack like this? (keep all the same styles that currently exist, just change the structure) When the small mobile breakpoint kicks in, we'll keep it exactly as you have it now. |
df7c881
to
977ea6d
Compare
@jeffgolenski Okie dokie, it took a hot minute but I think I've got it! I updated the PR so you can see the changes live as well, but here are some screenshots: Calypso GreenCalypso Blue |
@mattgawarecki Just gave it a go! Looks good on every viewport! Ship it! |
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.
LGTM!
This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/6950343 Thank you @mattgawarecki for including a screenshot in the description! This is really helpful for our translators. |
Translation for this Pull Request has now been finished. |
Resolves
1200412004370260-as-1201272180272104
.Relates to #57362.
Changes proposed in this Pull Request
JetpackCrmFreeCard
component into two versions: one with, and one without, the price display.JetpackProductCard
component.abovePriceText
on theJetpackProductCard
component.hideSavingLabel
prop on free product price displays.Testing instructions
NOTE: Please test this PR in both Calypso Blue and Calypso Green, for both wide and narrow screen layouts.
Reference screenshots