site-kit-wp
site-kit-wp copied to clipboard
Improve the layout of the graphic on the Consent Mode Setup CTA Banner at narrow desktop viewports.
Feature Description
The layout of the graphic on the Consent Mode Setup CTA Banner at narrow desktop viewports could be improved. As can be seen here at 961px the white and grey window in the middle of the graphic is slightly clipped:
It would be good to avoid this clipping and retain some of the green background around it as can be seen here at a wider viewport:
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- The Consent Mode Setup CTA Banner's layout should be vertically stacked, with the text and CTAs on one row and the graphic displayed on a row beneath it, for the following viewports, as per the linked Figma designs:
- For the "wide" desktop viewport (width >= 1281px), the layout should flow horizontally, with text/CTAs and the graphic on a single row, with the graphic to the right, as per the Figma design.
Please refer to the actual Figma designs as the source of truth, but for the sake of illustration here are screenshots of the layouts from Figma:
Mobile:
Tablet:
Narrow desktop:
Wide desktop:
Implementation Brief
- [ ] Update
assets/js/components/consent-mode/ConsentModeSetupCTAWidget.js:- [ ] Refactor the component to use divs with classes rather than
Grid,RowandCellcomponents, because these components don't support the wide desktop breakpoint we need.
- [ ] Refactor the component to use divs with classes rather than
- [ ] Update the layout of the CTA based on the AC in
assets/sass/components/ads/_googlesitekit-ads-setup-banner.scss, use a mobile first approach and update the layout using flexbox, switching the flex direction to position the svg beside or below the widget text depending on the breakpoint.
Test Coverage
- Update VRTs based on the new UI changes.
QA Brief
Changelog entry
I'll create a design for narrow/tablet viewport
@techanvil I added the tablet/narrow viewport design for the banner. LMK if there's anything else required.
Thanks @sigal-teller, that's great for the tablet view. However, what's still not entirely clear is how the graphic should display at the narrower desktop viewports.
Currently the desktop layout has the graphic on the right hand side but it crops at narrower desktop widths as seen in the images above. I wouldn't imagine we want to have the graphic displaying at the bottom for some of the desktop viewports, would we?
What would be great is if you could provide a design at the narrow desktop viewport of 901px to help clarify this further...
@techanvil Well actually in the narrow desktop viewport I would use the tablet design. There's too much content in this banner to fit in the same layout as wider viewports. The texts breaks into too many lines and the banner looks unbalanced. I think that in this case in narrow viewports we should adapt the design to look batter rather then match the wider viewports design. I added a design for it here.
Thanks @sigal-teller. In that case, would you like to specify the breakpoint at which we transition from the tablet/narrow desktop layout to the full width desktop layout? Or, we could simply specify that we should break at the point where the graphic isn't horizontally cropped in the full width layout (maybe also specifying some minimum padding at the horizontal edges too). WDYT?
@sigal-teller please could you respond to the above q? Just aware you will be on PTO next week so would like to keep this small one moving if possible. Thanks!
@techanvil If that's easier to implement then yes, we can define that we're using the wide desktop viewport as long as the graphics isn't cropped. If it's easier to set a fixed breakpoint that's we can do that as well, we might be able to use one of our existing breakpoints, depending if they are close enough.
Hey @sigal-teller, thanks for your reply.
I've been looking into this one a bit more, and, actually I am thinking we should reconsider the approach here. Sorry I didn't think of this earlier when asking my question about the breakpoints.
We can take the approach of breaking at a different breakpoint - but, it would be preferable not to. Bearing in mind we already have multiple banners with a similar design, as seen below (and no doubt more to come), it would be good to use a single generic component for all of these - this is a refactor we can make in the forthcoming Banner Notifications epic.
Ads:
Consent Mode:
RRM:
For the sake of UI consistency and code simplicity, it would be preferable if these all had the same responsive behaviour when it comes to their breakpoints.
What if we take another look at how the graphic is used within the breakpoint? I realise it arguably looks a bit off with it being cropped in the middle of the banner. However, we're already cropped at the top and bottom in desktop viewports. What if we adjusted the size and position of the graphic to avoid cropping on the left, but allow the right to be cropped, where it looks more natural as it aligns with the end of the banner?
Admittedly it's not perfect, but here's how it could look at 961px. As a compromise, for the sake of consistency and using standard breakpoints I think it could be OK.
Alternatively - personally, I don't actually think the left hand crop is so bad as long as the foreground graphic elements are not cropped and are well positioned within the banner. We could specify that the main "window" graphic should be centered within the right hand of the pane and only the green background can be cropped within this area, for example:
We can of course still take the approach of a different breakpoint if needs be - I don't want to suggest overly compromising the aesthetic integrity. But, as discussed above it would be nice to avoid it if we reasonably can.
What do you think?
Hey @techanvil I think you're right and we should strive for consistency in all setup banners, since they so use the same layout. i think there are 2 main issues here:
the 1st issue is to avoid cropping the visual, unless it's on the right edge of the banner. I think that this can be solved by creating a shape that can be displayed uncut when it's scaled down to fit the remaining space. I can do that for each green shape that I'm using, and then it will only get cut in wider viewports that will allow more space for the text.
The 2nd issue is switching to tablet layout in narrow viewports. The reason why I preferred this design in narrow viewports is to avoid an unbalanced layout, where the text is stacking on the left while increasing banner height significantly, and as a result we have a small illustration on the right side of the banner. It looks unbalanced. I think that maybe we should use this design for all banners in narrow viewports (I assume we can do that while refactoring all banner).
WDYT?
Thanks @sigal-teller. Adjusting the graphics as you've described SGTM.
With regard to 2nd issue - if we decide to use the vertically stacked layout for narrow desktop viewports as per your design, it does raise the question of what viewport width this layout should be applied up to. Our standard desktop breakpoints are 961px and 1281px. Here's a mockup of how it could look at the top end of this range, 1280px. It's OK, but the horizontal layout is probably better. Do you think this would be an acceptable layout at this width (presumably with a tweak to the graphics as per your updated design)?
It's also worth considering that if we are going to make changes to all of these banners, it might be better to undertake the work as part of the Banner Notification Refactoring epic that @jimmymadon has picked up the lead on... Cc @binnieshah
Thanks @techanvil this may make sense if we are going to make changes to the banners. @sigal-teller let us know what you think on what Tom has said above. I can then revise the sprint plan accordingly. Thanks
@techanvil I don't think it will be a good idea to use this design for all desktop viewports, including the wider viewports. Stacking the text above the illustration creates unbalanced layout and the banner height gets extended more then it needs.
We can use the breakpoints you mentioned, so from 1281px (net 1121 for the SK view without WP nav bar) we can use the wider desktop viewport design as it is here, meaning the text will be placed to the left of the illustration.
Tablet and narrow desktop viewports will have the vertical stacking like here.
Does that make sense?
@techanvil @binnieshah Just a quick note that as part of the first 2 phases of the Banner Notifications Refactoring epic, we don't intend to work on "inline" banners, i.e. banners which appear within the dashboard widget areas rather than within the Header component. We also don't intend to specifically consolidate the design or make any design changes unless it specifically makes the refactored code much simpler in a case here and there. But we will get to these eventually once the actual "notifications" are all refactored.
c.c. @aaemnnosttv
OK, so sounds like it would be good to get this one done within an upcoming sprint. @techanvil please could you review Sigal's note above and confirm this is OK? I can then assign it within the team. Thanks
Thanks @binnieshah.
@sigal-teller, that makes sense, and it's great you have created the designs for the different viewports for the Consent Mode banner.
I think, for the sake of moving this issue along we can address the CoMo banner in isolation, and I've added AC.
We can then follow up with a separate issue to apply the same responsive layout to the Ads and RRM banners. It would be great to flesh these out with per-viewport designs as well, to get a visual check on how they would look prior to committing to implementation, and in case the graphics might need a tweak. @sigal-teller, would you be able to provide designs for these?
IB ✅
QA Update ⚠️
@zutigrm Layout for the tablet is not matching with Figma design. As per AC Tablet (viewport width <= 600px): Figma Can you please check if AC is correct ?
Tablet viewport
Wide Dashboard
Narrow Dashboard
Mobile Dashboard
Thanks for raising it @mohitwp . It is most likely written by mistake/typo during AC, as we usually consider tablets starting from 600px not bellow, it is already mobile bellow that width. Let me confirm this with @techanvil since he wrote the AC
Thanks @mohitwp and @zutigrm - this was a mistake in the AC as Aleksej has pointed out. I have now corrected it, sorry for the inconvenience!
QA Update ✅
- Tested on dev environment.
- Verified Consent Mode setup CTA banner appearing at per Figma files.
Tablet view
Mobile View
Wide Dashboard
Narrow Dashboard