Page MenuHomePhabricator

Fix inconsistent media query usage to avoid 1px viewport bugs
Closed, ResolvedPublic3 Estimated Story Points

Description

Background

Vector employs both min-width and max-width media queries, both of which are inclusive of the pixel value. Due to this, the styles are prone to breaking on the exact pixel values of our breakpoints (i.e. https://wikimedia.slack.com/archives/G8QAPHCTT/p1654903525144409).

AC

  • No visual changes on non-breakpoint viewports
  • Layout and styling are consistent on breakpoint viewports

Developer notes

This can be solved by refactoring our CSS to consistently use a single type of media query, or updating our breakpoints to account for that 1px. The easiest solution would be to add new media query variables that take the 1px into account, so that the existing CSS can be fixed with search and replace.

@width-breakpoint-desktop-increment: @width-breakpoint-desktop - 1;
// Active when viewport is 1000px and over
@mediaquery-min-width-desktop: ~'min-width: @{width-breakpoint-desktop}';
// Active when viewport is 999px and under
@mediaquery-max-width-desktop: ~'max-width: @{width-breakpoint-desktop-increment}';

Then @media ( max-width: @width-breakpoint-desktop ) { } becomes @media ( @mediaquery-max-width-desktop ) { }

That said, I think it's a good idea to refactor media queries to use min-width when possible, as that's generally best suited for CSS that targets mobile by default and uses media queries to handle larger viewports. To avoid regressions and to limit the number of needed changes, clean up should ideally be done after the CSS grid is implemented and after our layout styles are simplified (T310010).

QA Steps

Vector uses breakpoints from core so the following viewport widths should be tested:

  1. 320px
  2. 720px
  3. 1000px
  4. 1200px
  1. Visit https://en.wikipedia.beta.wmflabs.org/wiki/Dog
  2. For each viewport width listed above, resize your browser's viewport to that exact width. Compare what you see at this viewport width with what you see when the viewport width has increased by 1 pixel and decreased by 1 pixel. You shouldn't see anything peculiar that only happens at the in-between viewport width. For example, before work was done on this ticket, the table of contents was losing its background color at exactly 1000px but this was only happening at exactly 1000px and wasn't happening at 999px or 1001px.

Screen Shot 2022-06-23 at 11.55.45 AM.png (1×1 px, 322 KB)

For exactly 720px:

  • Confirm the more button doesn't show in addition to the edit/watch tabs.

For exactly 1000px:

  • Confirm the sticky header displays
  • Confirm the table of contents toggle that appears next to the title doesn't show on scroll

Event Timeline

bwang updated the task description. (Show Details)

Great write-up, covering what we've also been identifying in the current breakpoints discussion T303522 on technical side for Wikimedia Design System.
I'd suggest just one change from task description to replace vars
@media ( @mediaquery-max-width-desktop ) with @media( max-width: @max-width-breakpoint-desktop )

Also the notes seem to have a logical error, this line
@mediaquery-max-width-desktop: ~'max-width: @{width-breakpoint-desktop-increment}';
would mean we're reducing the (minimum-width) breakpoint for desktop by 1px and make it max-width.
Instead
max-width-breakpoint-desktop: @min-width-breakpoint-desktop-wide - 1px is according.

We aim for those (min-/max-widths) in Codex with the upcoming breakpoint tokens.

Change 808080 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/skins/Vector@master] Introduce media query breakpoint variables and replace all media query usage with new variables

https://gerrit.wikimedia.org/r/808080

bwang removed bwang as the assignee of this task.Jun 27 2022, 7:10 PM
bwang moved this task from Doing to Code Review on the Web-Team-Backlog (Kanbanana-FY-2021-22) board.

Change 808080 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Introduce media query breakpoint variables and replace all media query usage with new variables

https://gerrit.wikimedia.org/r/808080

Change 808083 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/skins/Vector@master] Refactor some max-width media queries to use min width

https://gerrit.wikimedia.org/r/808083

Change 808083 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Refactor some max-width media queries to use min width

https://gerrit.wikimedia.org/r/808083

Jdlrobson subscribed.

I believe this can be resolved.