Notices and NoticeNotifications should follow mobile styles in tablet viewport
Feature Description
Noticed this during RRM but we can review this across the board for subtle notifications. On tablet, the banners just look weird in my opinion. Refer to the picture attached.
- There is a very dense block of text and the buttons sections have a lot of space.
- This feels unbalanced.
- I'm wondering if it might be best to move the buttons below and let the text sit along the width of the banner.
UPDATE: Changing the layout of notices simply based on the size of viewport doesn't work for us when certain notices are rendered in narrow components, e.g. in #11459 (Figma). So we should determine the layout of a notice based on the width of the notice itself and not the viewport width.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- All Site Kit "Notices" (i.e.
<Notice>and<NoticeNotification>components) should be styled as per their corresponding Figma mocks based on the width of the notices themselves. - If the width of a notice is <= 800px, use the Mobile and Tablet designs.
- If the width of a notice is > 800px, use the Narrow Desktop and Wider Desktop designs.
Implementation Brief
** The following IB is implemented in this https://github.com/google/site-kit-wp/pull/11954 PR as a PoC. Tests will need to be fixed. **
- Within
assets/js/components/Notice/index.js:- [x] Wrap the entire component being returned with another div with a className of
googlesitekit-notice-container. - [x] Move the
refto this parent container now.
- [x] Wrap the entire component being returned with another div with a className of
- Within
assets/sass/components/notice/_googlesitekit-notice.scss:- [x] For the
googlesitekit-notice-containerselector, add acontainer-type: inline-sizeproperty. - [x] Change all media queries for
bp-tabletto be@container (width > 800px) - [x] Remove unnecessary 10px bottom margin for the
ptag within.googlesitekit-notice__contentwhich was incorrectly added previously.
- [x] For the
- [ ] Update all VRTs and fix JSUnit snapshots.
Test Coverage
- No new tests required.
QA Brief
Changelog entry
@sigal-teller
For some reason, we designed all "Notice" style notifications to follow the desktop layout even for tablet viewports where the buttons are next to the text.
As shown in the Feature description screenshot, this looks ugly when there is a long title, description and multiple long CTAs in the case of the RRM notice. And it would look better when it is fixed as per the Figma mock (the fixed version is below):
However, this does mean that several other notices, which do not have long titles, descriptions or many CTAs, do look a bit empty and funny in this layout as shown below:
So what would be your guidance here? Should we keep things as they are? Or should we update all notices at tablet viewport to follow the "mobile/table" multi-row design as per the Figma mocks?
I wanted to confirm this before we proceed here as even though the code change is small, it will be a considerable effort to test all affected notices in the plugin!
Hey @jimmymadon For the tablet view we definitely need to follow the Figma design which places the CTA buttons below the title and text, and not to the right. Longer texts create an unbalanced view which doesn't look good (like the screenshot in the description in this issue). As for the screenshots you've added, I'm not sure I understand what you meant. It looks like the width specified is 959px, which is above the breakpoint for tablet, which mean it should use the "Narrow desktop viewport" (buttons to the right of the text and not below). Perhaps I'm missing something here?
@sigal-teller My apologies, the screenshots I've shared are for Site Kit's tablet view breakpoint which ends at 960px and not 900px (as mentioned in the Figma mocks). We had discussed this and could just update the notes in Figma to ensure we don't have this confusion in the future. We don't seem to have a breakpoint between 600px and 960px. So the screenshots are at the highest possible tablet viewport which shows the extra whitespace when we use the tablet layout.
Thanks @jimmymadon for clarifying. In this case I will update the notes in Figma to match the actual breakpoints. As for this issue, we've created a system, which includes several notices like success, errors, warnings etc. It's true that success notices are potentially shorter then error or warning which includes longer texts, but we need to take into account those long texts as well. I did try both options at the time as we know that some notice variants won't look as good as planned. I find having some blank space in corner cases is better then creating a unbalanced view with narrow paragraph that breaks into too many lines.
AC + IB ✔️
I'm changing the priority of this as it was highlighted by multiple testers in the PUE v0 bug bash so it is being noticed actively specifically in this epic so it should be addressed alongside the bug bash issues for this epic.
AC and IB ✔️