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

Customer Home: update card headings size and tags #40997

Closed
wants to merge 1 commit into from

Conversation

danhauk
Copy link
Contributor

@danhauk danhauk commented Apr 9, 2020

Changes proposed in this Pull Request

  • Update the card headings on Customer Home to be a smaller size. They were all fighting with the My Home page heading, especially on mobile. This also helps us move towards the explorations in My Home: Create a section for recommended tasks #40734.
  • Update the card headings to use a more semantic <h2> instead of <h1> all over the place.
  • General cleanup of some other styles.
  • Update headings to use title case and adding periods to subheadings per a recent decision.

@Automattic/dotcom-manage-design can you gut check me on the sizes here?

Before After
Screen Shot 2020-04-09 at 4 02 48 PM Screen Shot 2020-04-09 at 4 23 36 PM
Screen Shot 2020-04-09 at 4 45 43 PM Screen Shot 2020-04-09 at 4 21 56 PM
Screen Shot 2020-04-09 at 4 47 50 PM Screen Shot 2020-04-09 at 4 22 07 PM

Testing instructions

  • Checkout the branch or use the calypso.live link
  • Visit My Home to view updated headings
  • Test with an established site with no checklist
  • Test with a new site with checklist

@danhauk danhauk added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Feature] My Home The main dashboard for managing a WordPress.com site. labels Apr 9, 2020
@danhauk danhauk requested a review from a team as a code owner April 9, 2020 20:52
@danhauk danhauk self-assigned this Apr 9, 2020
@matticbot
Copy link
Contributor

@@ -51,7 +51,9 @@ const FreePhotoLibrary = ( { openSupportArticleDialogAndTrack, isSiteWPForTeamsP
/>
</button>
) }
<CardHeading>{ translate( 'Over 40,000 Free Photos' ) }</CardHeading>
<CardHeading tagName="h2" size={ 16 }>
{ translate( 'Over 40,000 free photos' ) }
Copy link

Choose a reason for hiding this comment

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

🆗 This change will be queued for retranslation. We'll use the existing translations in the meantime.

<CardHeading>{ translate( 'Go Mobile' ) }</CardHeading>
<h6 className="go-mobile__subheader">{ translate( 'Make updates on the go' ) }</h6>
<CardHeading tagName="h2" size={ 16 }>
{ translate( 'Go mobile' ) }
Copy link

Choose a reason for hiding this comment

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

⚠️ This change will be queued for retranslation. We'll display the English text until then.

<CardHeading tagName="h2" size={ 16 }>
{ translate( 'Go mobile' ) }
</CardHeading>
<h6 className="go-mobile__subheader">{ translate( 'Make updates on the go.' ) }</h6>
Copy link

Choose a reason for hiding this comment

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

🆗 This change will be queued for retranslation. We'll use the existing translations in the meantime.

@@ -35,9 +35,11 @@ export const GrowEarn = ( { siteSlug, expandToolsAndTrack, isSiteWPForTeamsProp

return (
<Card className="grow-earn">
<CardHeading>{ translate( 'Grow & Earn' ) }</CardHeading>
<CardHeading tagName="h2" size={ 16 }>
{ translate( 'Grow & earn' ) }
Copy link

Choose a reason for hiding this comment

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

🆗 This change will be queued for retranslation. We'll use the existing translations in the meantime.

<h6 className="grow-earn__card-subheader customer-home__card-subheader">
{ translate( 'Grow your audience and earn money' ) }
{ translate( 'Grow your audience and earn money.' ) }
Copy link

Choose a reason for hiding this comment

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

🆗 This change will be queued for retranslation. We'll use the existing translations in the meantime.

@@ -26,9 +26,11 @@ export const LaunchSite = ( { isPrimary, launchSiteAndTrack, siteId } ) => {

return (
<Card className="launch-site">
<CardHeading>{ translate( 'Site Privacy' ) }</CardHeading>
<CardHeading tagName="h2" size={ 16 }>
{ translate( 'Site privacy' ) }
Copy link

Choose a reason for hiding this comment

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

⚠️ This change will be queued for retranslation. We'll display the English text until then.

<h6 className="customer-home__card-subheader launch-site__card-subheader">
{ translate( 'Your site is private' ) }
{ translate( 'Your site is private.' ) }
Copy link

Choose a reason for hiding this comment

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

🆗 This change will be queued for retranslation. We'll use the existing translations in the meantime.

<h6 className="support__header customer-home__card-subheader">
{ translate( 'Get all the help you need' ) }
{ translate( 'Get all the help you need.' ) }
Copy link

Choose a reason for hiding this comment

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

🆗 This change will be queued for retranslation. We'll use the existing translations in the meantime.

@@ -22,7 +22,9 @@ const ChecklistSiteSetup = ( { checklistMode } ) => {
return (
<div className="checklist-site-setup">
<Card className="checklist-site-setup__heading">
<CardHeading>{ translate( 'Site Setup List' ) }</CardHeading>
<CardHeading tagName="h2" size={ 16 }>
{ translate( 'Site setup list' ) }
Copy link

Choose a reason for hiding this comment

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

🆗 This change will be queued for retranslation. We'll use the existing translations in the meantime.

@@ -160,15 +160,17 @@ export const QuickLinks = ( {
if ( ! isMobile() ) {
return (
<Card className="quick-links">
<CardHeading>{ translate( 'Quick Links' ) }</CardHeading>
<CardHeading tagName="h2" size={ 16 }>
{ translate( 'Quick links' ) }
Copy link

Choose a reason for hiding this comment

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

⚠️ This change will be queued for retranslation. We'll display the English text until then.
ℹ️ String reuse speeds up translation and improves consistency. The following string might make a good alternative and has already been translated 47 times:
translate( 'Quick Links' ) ES Score: 10

@jancavan
Copy link
Contributor

jancavan commented Apr 9, 2020

Should the card headers look more like/use the SectionHeader component?

Copy link
Member

@mmtr mmtr left a comment

Choose a reason for hiding this comment

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

It looks much better now ❤️

Although I feel there is more space than needed now around the "Site setup list" header.

Screen Shot 2020-04-10 at 17 31 27

Update headings to use title case

I think you wanted to say "sentence case" right?

@gwwar gwwar added the [Status] String Freeze Add the [Status] String Freeze label to your PR to ensure new strings are translated before merging label Apr 10, 2020
@gwwar
Copy link
Contributor

gwwar commented Apr 10, 2020

Thanks @danhauk giving ✅ for code changes.

Let's get a 👍 from @Automattic/dotcom-manage-design for design modifications before landing

@kwight kwight force-pushed the update/customer-home-card-headings branch from 17d7c62 to f2aff33 Compare April 17, 2020 17:03
@matticbot
Copy link
Contributor

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~24 bytes removed 📉 [gzipped])

name  parsed_size           gzip_size
home        +35 B  (+0.0%)      -24 B  (-0.0%)

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.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

@kwight
Copy link
Contributor

kwight commented Apr 17, 2020

@gwwar We have the String Freeze label on this PR, but I don't think we need to wait to merge (the changes are stylistic and/or punctuation, which wouldn't invalidate our existing translations) – are you hard-set on waiting?

@gwwar
Copy link
Contributor

gwwar commented Apr 17, 2020

It's 94% translated so it's not a blocker. I usually like adding the label for visible items, so we can try and ship updates with copy translated. (Strings get picked up on Monday so fastest turn around is usually tidying copy on Friday).

@kwight
Copy link
Contributor

kwight commented Apr 22, 2020

@danhauk Anything holding this back at the moment (maybe @jancavan 's comment?). It would be great to get it in sooner than later (I think it might block #41229 ?) .

@jancavan
Copy link
Contributor

Mine is more just a visual feedback, so not a blocker. Feel free to tackle it in a separate PR if need be. @kwight @danhauk

@kwight
Copy link
Contributor

kwight commented Apr 28, 2020

Not sure how this looks with #41229 merged – is it still needed @danhauk ?

@gwwar
Copy link
Contributor

gwwar commented Apr 29, 2020

Closing out this one since we're pretty close to new layout updates. Please reopen if this doesn't happen in a timely way.

@gwwar gwwar closed this Apr 29, 2020
@matticbot matticbot removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] String Freeze Add the [Status] String Freeze label to your PR to ensure new strings are translated before merging labels Apr 29, 2020
@sirreal sirreal deleted the update/customer-home-card-headings branch December 4, 2020 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] My Home The main dashboard for managing a WordPress.com site.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants