site-kit-wp
site-kit-wp copied to clipboard
Margin on top of text of the Change groups CTA should be 32px and not 24px.
Bug Description
Margin on top of text of the Change groups CTA should be 32px and not 24px.
Steps to reproduce
- Turn on audience segmentation featured flag
- Review the distance/margin above 'CTA Groups' CTA and the next item on the page
Screenshots
Details
Implementation : 24px
Figma: 32px
Additional Context
- PHP Version: 8.0
- Browser: Chrome
- Plugin Version: 1.129.0
- Device: MacOS Sonoma on MacbookPro
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- When a widget area has a subtitle but no title, the vertical gap between the widget area and the content above it should be as follows:
- Mobile viewports: 19px.
- Tablet and desktop viewports: 32px.
- Concretely this will apply to the Audiences Widget Area as the only widget area with a subtitle and no title at the time of writing.
Implementation Brief
- [x] Create a new file,
assets/sass/components/analytics-4/audience-segmentation/_googlesitekit-audience-widget-area.scss, adding the following styles:- [x]
.googlesitekit-widget-area--mainDashboardTrafficAudienceSegmentation,padding-top: 3pxon the mobile breakpoints. - [x]
.googlesitekit-widget-area--mainDashboardTrafficAudienceSegmentation,padding-top: 8pxon the desktop breakpoints.
- [x]
Test Coverage
- No additional test coverage required.
QA Brief
- Check the spacing between the widget areas matches the figma designs on mobile and desktop.
Changelog entry
- Update margins for the Audience Segmentation widget's title.
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.
Thanks @benbowler. This seems fine for now, we can revisit it more generally if/when we need it in future.
IB :white_check_mark:
QA Update ❌
- Tested on dev environment.
- @benbowler As per AC margin should be 32px for desktop and 19px for mobile. But currently, in desktop margin is 24px and in mobile 16px.
Desktop:
Mobile:
Hey @mohitwp, you need to take into account the padding on the top of the Audience Segmentation section as well as the padding on the bottom of the metrics component. If you add both of these padding values together you will see they add up to the correct amounts on each breakpoint.
QA Update ✅
Thank you @benbowler
- Tested on dev environment.
- Verified in desktop margin is 24px + 8px which is 32px.
- Verified in mobile margin is 16px + 3px which is 19px.
Desktop:
Mobile: