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

Update secondary CTA links for consistency #5184

Closed
aaemnnosttv opened this issue May 5, 2022 · 4 comments
Closed

Update secondary CTA links for consistency #5184

aaemnnosttv opened this issue May 5, 2022 · 4 comments
Labels
Good First Issue Good first issue for new engineers P1 Medium priority Rollover Issues which role over to the next sprint Type: Enhancement Improvement of an existing feature

Comments

@aaemnnosttv
Copy link
Collaborator

aaemnnosttv commented May 5, 2022

Feature Description

As raised in #4764 (comment) and #4764 (comment) there is an inconsistency of styling secondary links. These should be updated to be consistent.


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

Acceptance criteria

  • The .googlesitekit-cta-link styles should be adjusted to include the styles from .googlesitekit-cta-link--inherit, basically so that all elements that use class googlesitekit-cta-link today will look the same as those that use classes googlesitekit-cta-link googlesitekit-cta-link--inherit today.
  • All usages of the class googlesitekit-cta-link--inherit as well as the style definition for this class should be removed.

Implementation Brief

  • Using assets/sass/components/global/_googlesitekit-cta-link.scss,

    • Update .googlesitekit-cta-link to include the styles from googlesitekit-cta-link--inherit
    • Remove the googlesitekit-cta-link--inherit class
  • Remove the googlesitekit-cta-link--inherit class used across the codebase.

  • Estimates a bit higher due to QA involved since it's a global change.

Test Coverage

  • No new tests to be added.
  • VRT images might need to be updated.

QA Brief

  • QAing this would require making sure all CTA links (not buttons, mostly secondary CTA links) look similar to the "Reset Site Kit" CTA here.
  • The font family of the said links should be inherited by default, i.e. 'Roboto'.
  • A few of the said links may have an intentionally different font size.

Changelog entry

  • N/A
@aaemnnosttv aaemnnosttv added P1 Medium priority Type: Enhancement Improvement of an existing feature labels May 5, 2022
@aaemnnosttv
Copy link
Collaborator Author

@felixarntz should we standardize on inherit: true/false?

@felixarntz
Copy link
Member

@aaemnnosttv They should all look like they had the googlesitekit-cta-link--inherit class. Since that should be the new default though, we should rather integrate that class's styles into the base googlesitekit-cta-link styles. I've defined ACs accordingly.

@aaemnnosttv aaemnnosttv removed their assignment May 11, 2022
@asvinb asvinb assigned asvinb and unassigned asvinb May 12, 2022
@asvinb asvinb added the Good First Issue Good first issue for new engineers label May 12, 2022
@aaemnnosttv aaemnnosttv self-assigned this May 16, 2022
@aaemnnosttv
Copy link
Collaborator Author

IB ✅

@aaemnnosttv aaemnnosttv removed their assignment May 16, 2022
@nfmohit nfmohit self-assigned this May 17, 2022
@kuasha420 kuasha420 assigned kuasha420 and unassigned kuasha420 May 17, 2022
@nfmohit nfmohit removed their assignment May 18, 2022
@eclarke1 eclarke1 added the Rollover Issues which role over to the next sprint label May 23, 2022
@mohitwp
Copy link
Collaborator

mohitwp commented Jun 1, 2022

QA Update ✅

Verified

  • Verified all CTA links.
  • The font family of the said links is 'Roboto'.
  • Now all CTA links (not buttons, mostly secondary CTA links) look similar to the "Reset Site Kit" CTA here.
  • Also verified when adsenseSetupV2, IdeaHub and DashboardSharing feature flag is active.

image

image

image

image

image

image

image

image

image

image

image

@mohitwp mohitwp removed their assignment Jun 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue Good first issue for new engineers P1 Medium priority Rollover Issues which role over to the next sprint Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

8 participants