google / site-kit-wp

Site Kit is a one-stop solution for WordPress users to use everything Google has to offer to make them successful on the web.
https://sitekit.withgoogle.com
Apache License 2.0
1.22k stars 279 forks source link

Margin on top of text of the Change groups CTA should be 32px and not 24px. #8862

Open kelvinballoo opened 2 weeks ago

kelvinballoo commented 2 weeks ago

Bug Description

Margin on top of text of the Change groups CTA should be 32px and not 24px.

Steps to reproduce

  1. Turn on audience segmentation featured flag
  2. Review the distance/margin above 'CTA Groups' CTA and the next item on the page

Screenshots

Details Implementation : 24px

Screenshot 2024-06-11 at 18 22 35

Figma: 32px

Screenshot 2024-06-11 at 22 08 17

Additional Context


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

Acceptance criteria

Implementation Brief

Test Coverage

QA Brief

Changelog entry

benbowler commented 1 week ago

Initially I looked into a more general solution, the reason this is happening is that we have this block in CSS where if multiple mdc-layout-grid divs are next to each other, the following elements have 0 top padding. We don't want to remove this though as this helps group other grid elements together across the plugin.

https://github.com/google/site-kit-wp/blob/2da2d713947d2648d6e0bb2b2a607e8fb16598c5/assets/sass/vendor/_mdc-layout-grid.scss#L5-L7

Instead I specifically apply padding for this widget area only.

techanvil commented 1 week ago

Thanks @benbowler. This seems fine for now, we can revisit it more generally if/when we need it in future.

IB :white_check_mark: