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

Fix font problems in Idea Hub widget #4012

Closed
felixarntz opened this issue Sep 9, 2021 · 8 comments
Closed

Fix font problems in Idea Hub widget #4012

felixarntz opened this issue Sep 9, 2021 · 8 comments
Labels
Module: Idea Hub Google Idea Hub module related issues P1 Medium priority Type: Bug Something isn't working
Milestone

Comments

@felixarntz
Copy link
Member

felixarntz commented Sep 9, 2021

Bug Description

There are two problems with the title of the Idea Hub widget, one pointed out in #3810 (comment). In addition, the title font here is Google Sans, which was like that in the original Figma file, but this should actually use Roboto like we do elsewhere.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • Both the Idea Hub widget title and the experimental label (in general, not only for the Idea Hub widget) should use our regular Roboto font instead of Google Sans.
  • When the experimental label breaks into a new line, it shouldn't appear as if it was indented.

Implementation Brief

  • Inside assets/js/modules/idea-hub/components/dashboard/DashboardIdeasWidget/index.js:
    • Add the class .googlesitekit-subheading-1 to the h3.googlesitekit-idea-hub__title. This will change the font-family to Roboto.
    • Wrap the text inside h3.googlesitekit-idea-hub__title in a <span>, with a className such as .googlesitekit-idea-hub__title-text.
    • In other words the h3 should now have two children: the new span and the <Badge>.

Inside assets/sass/components/global/_googlesitekit-badge.scss:

  • Remove the margin from .googlesitekit-badge. This margin is the reason for the indentation.

Inside assets/sass/components/idea-hub/_googlesitekit-idea-hub-dashboard-ideas-widget.scss:

  • Increase the line-height of .googlesitekit-idea-hub__title to 1.5. This will create a bit more space between the lines when the badge wraps.
  • Create a new rule for the new span.googlesitekit-idea-hub__title-text, and give it a padding-right of 12px. This will create the space that the margin on the badge provided, but will not indent the badge when it wraps.

Test Coverage

  • No new tests required.

Visual Regression Changes

  • Images dealing with the Idea Hub dashboard widget might need updating.

QA Brief

  • Verify that the ACs above have been met

Changelog entry

  • Fix font problems in the Idea Hub widget.
@felixarntz felixarntz added Type: Bug Something isn't working P1 Medium priority Module: Idea Hub Google Idea Hub module related issues labels Sep 9, 2021
@felixarntz felixarntz assigned felixarntz and unassigned felixarntz Sep 9, 2021
@eclarke1 eclarke1 added this to the Sprint 58 milestone Sep 9, 2021
@eugene-manuilov
Copy link
Collaborator

Using createInterpolateElement, wrap the text inside h3.googlesitekit-idea-hub__title in a , with a className such as .googlesitekit-idea-hub__title-text.

@johnPhillips we don't need to use the createInterpolateElement function here. Nothing blocks us from wrapping the translated text directly:

<h3 className="googlesitekit-idea-hub__title">
-	{ __(
+	<span>{ __(
		'Ideas to write about, from actual questions people asked on Search',
		'google-site-kit'
-	) }
+	) }</span>

	<Badge
		label={ __( 'Experimental', 'google-site-kit' ) }
	/>
</h3>

@johnPhillips
Copy link
Contributor

Thanks @eugene-manuilov - IB amended.

@eugene-manuilov
Copy link
Collaborator

IB ✔️

@eugene-manuilov eugene-manuilov removed their assignment Sep 10, 2021
@johnPhillips johnPhillips self-assigned this Sep 10, 2021
@johnPhillips johnPhillips removed their assignment Sep 14, 2021
@tofumatt tofumatt assigned tofumatt and unassigned tofumatt Sep 14, 2021
@wpdarren wpdarren self-assigned this Sep 15, 2021
@wpdarren
Copy link
Collaborator

wpdarren commented Sep 15, 2021

QA Update: ❌

@johnPhillips unfortunately, there's one place where the experimental label font is still Google Sans. When you have connected Idea hub, go to the settings, and you will see the label as per screenshot.

image

@wpdarren wpdarren assigned johnPhillips and unassigned wpdarren Sep 15, 2021
@johnPhillips
Copy link
Contributor

@felixarntz Just to check before I address the QA feedback: I was operating under the assumption that this issue was about the dashboard widget only?
Happy to change the Badge to Roboto on the settings page as well, just let me know.
CC @wpdarren

@johnPhillips
Copy link
Contributor

@felixarntz disregard, sorry. It's in the ACs 🤦

@johnPhillips johnPhillips removed their assignment Sep 15, 2021
@aaemnnosttv aaemnnosttv self-assigned this Sep 20, 2021
@aaemnnosttv
Copy link
Collaborator

Ready for another pass @wpdarren 👍

@aaemnnosttv aaemnnosttv assigned wpdarren and unassigned aaemnnosttv Sep 20, 2021
@wpdarren
Copy link
Collaborator

QA Update: ✅

Verified:

  • Both the Idea Hub widget title and the experimental label should use our regular Roboto font instead of Google Sans. -- Screenshot 1 - Screenshot 2

  • When the experimental label breaks into a new line, it shouldn't appear as if it was indented. - Screenshot

  • The issue reported here is fixed.

image

@wpdarren wpdarren removed their assignment Sep 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: Idea Hub Google Idea Hub module related issues P1 Medium priority Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants