Inconsistent layout between similar tiles in zero states

[bethanylang]

Feature Description

As reported by @aaemnnosttv in Bug Bash (Asana task):

Ideally the areas of the tile would be aligned across tiles regardless of the length of the titles or other elements

Also

Text should be aligned to bottom to address.

---

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

Acceptance criteria

• The titles & content for key metric tiles in all their states (i.e. loading, error, zero, gathering, and ready) should be aligned consistently so they start in the same vertical position.
• The height of the error state for key metric tiles should consistently be the same, similar to other states.

Implementation Brief

• In assets/sass/components/key-metrics/_googlesitekit-km-widget-tile.scss:
  • Set the height of .googlesitekit-km-widget-tile--error to 100%.

The suggested solution is to use CSS subgrid (supported by all major browsers. https://caniuse.com/?search=subgrid). However it is not so straightforward since the elements we need to align are deeply nested relative to the parent container for all the key metric widgets which is 4 levels down. So we need to have nested subgrids. The code should do the following:

• Define the grid rows for the .googlesitekit-widget-area--mainDashboardKeyMetricsPrimary .googlesitekit-widget-area-widgets .mdc-layout-grid__inner element.
  • The rows should be defined starting at the tablet breakpoint.
    • Since the cells are rendered in a 2x2 grid, we need to define 4 rows for the tablet viewport. (auto 1fr auto 1fr)
    • From desktop breakpoint, we then define 2 rows (auto 1fr).
    • 2 rows are defined (and 4 on tablet only) because the KM widget has 2 distinct part which we need to align:
      • .googlesitekit-km-widget-tile__title-container
      • .googlesitekit-km-widget-tile__body
    • 4 rows are defined for tablets because each widget will span across 2 rows (see above).
• Create subgrid for all the children within the selector above (.mdc-layout-grid__inner) as follows:
  • grid-template-columns: subgrid;
  • grid-template-rows: subgrid;
• Make necessary adjustments to the CSS so that the content within the subgrids take the appropriate columns and rows. For instance since we are inheriting the columns and rows from the main grid, the children all have 12 columns. So we need to specify to take all the available space:
  • grid-column: 1 / -1;
  • grid-row: 1 / -1;

SASS POC lines to be added to assets/sass/components/key-metrics/_googlesitekit-km-widget-tile.scss:
https://gist.github.com/asvinb/c9334396216a4b0063ca277b912c2853

Bumped the estimate to polish the POC add enough time to QA all the possible scenarios across breakpoints.

Test Coverage

• Update VRT references.

QA Brief

• Test the Key Metrics widget area at all breakpoints in the success state, ensuring that the metrics are always aligned regardless of the length of the metric title.
• Test the Key Metrics widget area at all breakpoints in the error state, ensuring that the error titles are always aligned regardless of the length of the metric title and the error action.
• Test that the widgets are always the same height regardless of contents/breakpoint.

Changelog entry

• Improve alignment of Key Metric Widget tiles, including when errors are encountered.

[aaemnnosttv]

AC ✅

[10upsimon]

Passing this over to IBR, I did not realise that this already had an IBR suggestion and was looking more into determining which widgets were present for a more consistent alignment match on all elements within the error tiles, but the existing IBR covers enough to make this AC satisfied:

[techanvil]

IB ✅

[techanvil]

Hi @10upsimon, apologies as I should have looked more closely at this one while reviewing the IB.

Having picked it up for execution, I've realised there's actually quite a bit more to the issue. It appears the IB as specced covers the second AC point, but not the first. What we're trying to achieve here, by way of that first point, is to align the content so it starts at a consistent height regardless of the tile state.

This is illustrated in the "Narrow viewport" designs in Figma and there is related discussion on Slack.

Narrow viewport design on Figma:

Here's how things can look at the moment (artificially expanded to two rows for the sake of illustration):

As mentioned on Slack, we need to keep in mind that the title can potentially wrap onto three (or maybe even more) rows, and the idea is for the content to align to the height of the longest (tallest) title.

Tracing back through the Slack conversations, we can see that @asvinb and @nfmohit have synced about this, and one potential angle that has been flagged is to try using CSS subgrid.

It's clear this issue will be significantly more complicated than the current spec and estimate, so we should move this back to IB and remove it from the current sprint.

Would you like to pick this up again to continue working on the IB? I realise you're busy with a design doc so may not have capacity in which case someone else can pick it up. I'm quite interested myself, although it could be a bit of a complex one so I also need to be careful not to overcommit with Audience Segmentation design work underway.

Cc @bethanylang

[benbowler]

@asvinb can I grab this IB update?

[techanvil]

Hi @asvinb, nice work so far on the IB, it's an interesting solution, although there are a few unhandled cases which I've noticed.

Ideally, it would be nice to be able to modify the markup to avoid the nested subgrids, but realistically I don't see that this is particularly achievable - I think it would be much more complex than the nested solution you've proposed.

There are however some scenarios where the POC is not holding up, having applied it locally I've noticed that the CTA banners and error state layouts are now misaligned:

It would be worth exploring these further to ensure these cases can be managed with this solution.

There are also a couple of grammatical oddities in the way the IB is written, probably just a translation hiccup - I'm talking about the text that I have made bold here:

• The rows should be defined as from the tablet breakpoint (+1px to match MDC breakpoints for layouts).
  • Since the cells are rendered in a 2x2 grid, we need to define 4 rows to as from tablet. (auto 1fr auto 1fr)
  • As from desktop, we then define 2 rows (auto 1fr).

I think this is what you mean, please correct me if I am wrong:

• The rows should be defined starting at the tablet breakpoint (+1px to match MDC breakpoints for layouts).
  • Since the cells are rendered in a 2x2 grid, we need to define 4 rows for the tablet viewport. (auto 1fr auto 1fr)
  • For desktop viewports, we then define 2 rows (auto 1fr).

[asvinb]

Thanks @techanvil . 👍 for the tweaks to the IB. Agree with the different edge cases. I'll try to replicate and update the IB.

[asvinb]

@techanvil Gist updated. Please note that the gist is mostly showing that it should be possible to align the items using subgrid and the approach to take. There is also a note that the estimate is quite big even though we have a "working" POC because we need to cater for all the different scenarios. I agree with you, there are things which can be fixed during implementation now that the approach looks good.

[techanvil]

Great, thanks @asvinb. All makes sense. IB LGTM, great work! ✅

[mohitwp]

QA Update ⚠️

@benbowler

• Tested on dev environment.
• Verified success and error state using different combinations of widget on different breakpoints.

Question >
I noticed that in the dev environment, KM widgets in an error state have a lot of space between the text, which looks odd. I understand that we made changes to ensure the height of all tiles is the same, but in tablet and desktop viewports, there is a significant amount of empty space between the title and description. Is it possible to reduce this space?

Latest

Dev:

Pass cases:

Recording.795.mp4

[benbowler]

Hey @mohitwp, I saw this and mentioned it in the PR with some possible ways to make this gap smaller but this was beyond the original IB. I'll discuss with the team today as to whether this new gap is acceptable or if we want to do additional refactoring to reduce the gap and update here with the outcome of that discussion.

[mohitwp]

QA Update ✅

• Tested on dev environment.
• Verified that space is now reduce between widget conetnt appearing under error state.
• Verified success and error state using different combinations of widget on different breakpoints.
• Verified The titles & content for key metric tiles in all their states (i.e. loading, error, zero, gathering, and ready) aligned consistently and start in the same vertical position.
• Verified the height of the error state for key metric tiles remain consistently be the same, similar to other states.
• Note : In error states in mobile devices CTA positions looks odd but this issue is also exist on latest environment so I will create separate ticket for this.

Recording.795.mp4