site-kit-wp
site-kit-wp copied to clipboard
Issues with Most Popular Pages module responsiveness on 599px to 668px viewports
Bug Description
Analytics most popular pages widget is not properly appearing on viewport having width between 599px to 668px.
Steps to reproduce
- Set up site kit with Analytics.
- Go to main dashboard.
- Set viewport between 599px to 668px using dev tool.
- See issue.
Screenshots
Screenshots
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- For the Top content over 28 days table, on mobile viewports where the table headings start to overlap with each other, the table should be converted to a two column layout with tabs that as per this Figma mock.
Screenshot
Implementation Brief
Test Coverage
QA Brief
Changelog entry
@sigal-teller @aaemnnosttv This seems to be an issue only for a very short viewport range between 600px and 670px. I suggest simply wrapping the words, ideally with CSS hyphens. Does this look ok?
Hey @jimmymadon, it's worth noting we have a similar issue in the backlog, https://github.com/google/site-kit-wp/issues/5561, where we looked into hyphenation, among other things - see https://github.com/google/site-kit-wp/issues/5561#issuecomment-1191770816.
However as discussed there are issues with this approach; the direction from Seb toward the end of the issue was to simply change the font size.
@sigal-teller Could you please review the solutions proposed by @techanvil and @aaemnnosttv from this comment thread.
For me personally, the hyphens work nicely. But as Tom and Evan say, using icons are also a good option which will keep the height consistent too.
@sigal-teller You suggested adding horizontal scroll which we have introduced for the Audience Segmentation widget. In experimenting with the horizontal scroll, I realised we could further reduce the width of column 1 with minimal code change and effort.
Thank you @jimmymadon for these explorations. I think it doesn't look so great especially with long URLs and page names. Does using horizontal scrolling here adds a lot of complexity?
@sigal-teller Thank you - horizontal scrolling shouldn't be a big issue. I'll add that as the AC here, move this along and we can see the estimate that comes out from the IB.
Having the remaining columns be horizontally scrollable while the first column remains fixed in place is likely significantly more complicated/effort than simply scrolling the entire table, which is probably an easier solution. 🤔
I can kind of understand keeping the first column around for the context, but it will also make it so the user needs to horizontally scroll when using touch gestures ONLY after the first column. That's not intuitive or normal scroll behaviour, especially when there's no design here for a visual indicator that there's more content.
I think we should be scrolling the entire table/widget contents, not select columns. It presents significant accessibility/usability issues, especially on touchscreen devices, where these viewports are likely to be encountered.
FWIW, these originally were designed as horizontally scrollable in smaller viewports and this was later changed for some reason I can't recall.
On a related note, we should scroll the top navigation if it overflows (a separate issue) as this is a common pattern.
@tofumatt I agree that it's more complicated to scroll only some of the columns in several aspects. The question is if it makes sense to see the numbers in the table without the page names, and if the user will need to scroll back and forth again and again.
@jimmymadon I have another idea here that from a UX perspective is much better but might require more effort. Instead of having 1 table with scrolling we can have 4 tables that will include the 1st column and one more column, and we'll add tabs above the table to switch between them: "Pageviews", "Sessions" etc. This will be vwery similar to the pattern we introduced in the Audience segmentation UX for multiple groups in narrow viewports. I also found this pattern in SC (attaching a screenshot)
@aaemnnosttv As for the top navigation, we should definitely add a horizontal scroll instead of breaking into 2 lines. I suggested it a while ago when I worked on the header improvements which were paused, but I'm using it in all my designs since then. you can see it here for example.
@sigal-teller
This will be vwery similar to the pattern we introduced in the Audience segmentation UX for multiple groups in narrow viewports.
If @tofumatt or another AC reviewer (@eugene-manuilov?) can review this solution, it might be better to have a formal design of this in Figma which we can use for all multiple column tables with a wide first column.
@jimmymadon I added an example here for the Figma design I referred to in my previous comment. LMK if that can work.
That tabbed solution seems great, I think that's a more intuitive one than scrolling; it'd be more straightforward to implement and I think is flexible enough to even allow for more tabs than fit in the widget, by scrolling only the tabs.
Looks good to me, moving to IB 👍🏻
@benbowler, instead of making ReportTable breakpoint aware, we should read breakpoint data in a component that requires an alternative layout for the table based on the current breakpoint. In other words, we should update the report table to have an alternative layout and the component that renders that table to read the current breakpoint and to use an alternative layout if the breakpoint is tablet or mobile.
@eugene-manuilov sounds good, I've updated the IB.
Thanks, @benbowler. IB ✔️
QA Update: ⚠
@benbowler @techanvil I am confused by the solution we have gone with on this one. I was expecting to see my test site look like the figma designs with the tabbed view. I am on the develop branch and using data from oi.ie. While this is a new site, I did clear cache and refresh the develop branch a few times.
This is what I see at 599 px
This is what I see at 600 px
The overlapping continues throughout to 668 px.
Am I missing something here?
Thanks for flagging this @wpdarren, it turns out the PR was approved but not merged although the rest of the process was completed with the issue moved to QA. This was clearly an oversight so I've gone ahead and merged the PR.
QA Update: ✅
Verified:
Love the solution for this on mobile! Looks great.
- For the Top content over 28 days table, on mobile viewports where the table headings start to overlap with each other, the table is converted to a two column layout with tabs that as per this Figma mock.
- I checked the page title font is also bold when viewed in the desktop viewport, and in the WP dashboard "top content" widget as discussed in Slack.
https://github.com/user-attachments/assets/edffe7a6-db34-4c1c-8aef-8a3fbdff790a
Screenshots