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

Post Ads module setup success banner notification has incorrect "Got it" CTA link color #8514

Closed
1 task done
mxbclang opened this issue Apr 10, 2024 · 5 comments
Closed
1 task done
Labels
Module: Ads Google Ads module related issues P1 Medium priority Team S Issues for Squad 1 Type: Enhancement Improvement of an existing feature

Comments

@mxbclang
Copy link

mxbclang commented Apr 10, 2024

Bug Description

As reported by @10upsimon in Ads Module Bug Bash:

The link color in the product is gray vs. green in the Figma designs:

image


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

Acceptance criteria

  • Link/Button colour for "Subtle Notifications" CTA button should match the one in figma design

Implementation Brief

  • Update assets/sass/components/dashboard/_googlesitekit-subtle-notification.scss
    • For .mdc-button selector, apply the color of $c-site-kit-sk-600

Test Coverage

  • Update any failing VRT

QA Brief

  • Reset and set up the Ads module from scratch.
  • When the setup is complete you will see the subtle notification on the dashboard, confirm the Got It CTA text is green not grey.

Changelog entry

  • Update the CTA link color in the post Ads module setup success banner.
@mxbclang mxbclang added P1 Medium priority Type: Enhancement Improvement of an existing feature Team S Issues for Squad 1 Module: Ads Google Ads module related issues labels Apr 10, 2024
@eclarke1 eclarke1 added the Next Up Issues to prioritize for definition label Apr 22, 2024
@zutigrm zutigrm self-assigned this Apr 30, 2024
@zutigrm
Copy link
Collaborator

zutigrm commented Apr 30, 2024

Since this is a very small issue and AC, I also added IB and moved it to the IBR directly
cc: @10upsimon @tofumatt @eclarke1

@zutigrm zutigrm removed their assignment Apr 30, 2024
@tofumatt tofumatt self-assigned this May 2, 2024
@tofumatt
Copy link
Collaborator

tofumatt commented May 2, 2024

IB ✅

@tofumatt tofumatt removed their assignment May 2, 2024
@mxbclang mxbclang removed the Next Up Issues to prioritize for definition label May 6, 2024
@benbowler benbowler self-assigned this May 9, 2024
@benbowler benbowler removed their assignment May 13, 2024
@mohitwp mohitwp self-assigned this May 13, 2024
@mohitwp
Copy link
Collaborator

mohitwp commented May 15, 2024

QA Update ✅

  • Tested on dev environment.
  • Verified that Link/Button colour for "Subtle Notifications" CTA button matches the one in figma design

Latest:

image

image

develop:

image

image

@mohitwp mohitwp removed their assignment May 15, 2024
@aaemnnosttv
Copy link
Collaborator

@zutigrm @tofumatt @benbowler

  • For .mdc-button selector, apply the color of $c-site-kit-sk-600

While this technically the correct concrete value, we should reference the correct token that is used in the design rather than its underlying value. The design calls for content/on-primary-container which maps to the same color, but we should always reference tokens wherever possible rather than color values.

$c-content-on-primary-container: $c-site-kit-sk-600;

We can address this in a follow up, as there is another such instance to correct as well.

@aaemnnosttv
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: Ads Google Ads module related issues P1 Medium priority Team S Issues for Squad 1 Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

8 participants