Skip to content

@media rule thresholds do not account for browser font size setting #9390

Open
Open
@media rule thresholds do not account for browser font size setting#9390
Labels
bugAn error in the Docusaurus core causing instability or issues with its executiondomain: a11yRelated to accessibility concerns of the default theme

Description

@KosRud
KosRud
opened on Oct 10, 2023

Have you read the Contributing Guidelines on issues?

Prerequisites

  • I'm using the latest version of Docusaurus.
  • I have tried the npm run clear or yarn clear command.
  • I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
  • I have tried creating a repro with https://new.docusaurus.io.
  • I have read the console error message carefully (if applicable).

Description

Most things in docusaurus scale with rem units, but the thresholds for @media rules are set in px. Which means setting a large font size in the browser effectively "zooms in" the page, but the @media rules do not trigger like they would with actual zoom.

Screenshots below are from the page https://docusaurus.io/docs/styling-layout with the same zoom level (100%) and viewport size (~1000px). The large font size breaks the layout.

normal font size large font size
image_2023-10-10_18-07-29 image_2023-10-10_18-07-29 (2)

Reproducible demo

https://docusaurus.io/docs/styling-layout

Steps to reproduce

  1. Go to https://docusaurus.io/docs/styling-layout
  2. Set viewport width to ~1000px, just before the @media rule triggers
  3. Set large font size in the browser
  4. Observe the broken layout

Expected behavior

IMHO it would make sense to use rem unit for @media thresholds as it would represent the "actual" scale of the content (accounting for both zoom and browser font size).

Actual behavior

TOC on the right was unreasonably squished and navbar at the top has overlapping text. Normally with the content being enlarged this much a @media rule would trigger, but it does not happen because the threshold is tied to px and is not aware of browser font size setting.

image_2023-10-10_18-07-29 (2)

Your environment

Link: https://docusaurus.io/docs/styling-layout
Browser: Brave 1.58.137
OS: Windows 11

Also tried locally with npx create-docusaurus@latest my-website classic (pulled version 2.4.3), same result.

Self-service

  • I'd be willing to fix this bug myself.

Activity

  1. KosRud
    added
    bugAn error in the Docusaurus core causing instability or issues with its execution
    status: needs triageThis issue has not been triaged by maintainers
    on Oct 10, 2023

  2. Piyush-onGIT commented on Oct 11, 2023

    @Piyush-onGIT
    Piyush-onGIT
    on Oct 11, 2023

    Please assign me this issue, I would like to work on this.

  3. OzakIOne commented on Oct 11, 2023

    @OzakIOne
    OzakIOne
    on Oct 11, 2023
    Contributor

    Please assign me this issue, I would like to work on this.

    Docusaurus maintainers don't assign issue to people, if you want to contribute, just send a PR

  4. slorber commented on Oct 12, 2023

    @slorber
    slorber
    on Oct 12, 2023
    Collaborator

    Isn't this a Brave-only issue?

    Because despite using px I see Chrome, Firefox and Safari being able to switch to the mobile layout on zoom.

    CleanShot.2023-10-12.at.11.14.40.mp4
  5. slorber
    removed
    status: needs triageThis issue has not been triaged by maintainers
    on Oct 12, 2023

  6. KosRud commented on Oct 12, 2023

    @KosRud
    KosRud
    on Oct 12, 2023
    Author

    Isn't this a Brave-only issue?

    Because despite using px I see Chrome, Firefox and Safari being able to switch to the mobile layout on zoom.

    CleanShot.2023-10-12.at.11.14.40.mp4

    Have you set large font size?

    image

  7. slorber commented on Oct 12, 2023

    @slorber
    slorber
    on Oct 12, 2023
    Collaborator

    I don't use Brave. Can you show me where to find this option on other browsers and tell me if you see the bug in other browsers too?

  8. KosRud commented on Oct 12, 2023

    @KosRud
    KosRud
    on Oct 12, 2023
    Author

    I don't use Brave. Can you show me where to find this option on other browsers and tell me if you see the bug in other browsers too?

    I just installed Chrome and it does appear there too. Below is screenshot of the bug, as well as how to reach the setting.

    image
    image image

  9. KosRud commented on Oct 12, 2023

    @KosRud
    KosRud
    on Oct 12, 2023
    Author

    The bug occurs because the size of elements on the page is tied to rem units, which are affected by font size setting in (any) browser. By default 1rem = 16px so these units are in a sense equivalent. However, setting large font size in the browser will change this ratio (e.g. 1rem = 32px).

    This effectively zooms in the page, because everything is tied to rem in docusaurus CSS (e.g. 1rem margin used to be 16px and now it is 32px). However, @media rules are tied to px units which are unaffected by font size and so they are "unaware" of the layout getting compressed.

  10. Josh-Cena commented on Oct 12, 2023

    @Josh-Cena
    Josh-Cena
    on Oct 12, 2023
    Collaborator

    I'm not sure what to do. Sites like GitHub and nextjs.org seem to have fixed font size so their sites don't rescale at all. Vuepress only scales the navbar but not the content, and its screen size breakpoint is also based on pixels. Do you have a site showing ideal behavior?

  11. Josh-Cena
    added
    domain: a11yRelated to accessibility concerns of the default theme
    status: needs more informationThere is not enough information to take action on the issue.
    on Oct 12, 2023

  12. KosRud commented on Oct 12, 2023

    @KosRud
    KosRud
    on Oct 12, 2023
    Author

    I'm not sure what to do. Sites like GitHub and nextjs.org seem to have fixed font size so their sites don't rescale at all. Vuepress only scales the navbar but not the content, and its screen size breakpoint is also based on pixels. Do you have a site showing ideal behavior?

    I will try to find an example (or make one myself). But by simply experimenting with developer tools in browser, replacing

    @media (max-width: 996px) {

    with

    @media (max-width: 62.25rem) {

    seems to do the trick. As long as the default setting 1rem = 16px remains in place, these rules are equivalent. If the font size (ratio rem to px) is increased, the rule triggers earlier (which is desired to prevent the layout from breaking).

    The only issue is that I can't be sure this change won't unexpectedly create problems elsewhere (theoretically I don't think it should).

    Unless I'm missing something, simply replacing all media thresholds defined in px with 1/16 of that value in rem should provide ideal behavior, and look exactly the same as before with default font setting.

  13. KosRud commented on Oct 12, 2023

    @KosRud
    KosRud
    on Oct 12, 2023
    Author

    Speaking of which, searching for @media rules in the repo I found thresholds of 996px and 997px being used seemingly interchangeably. Not sure if this is intentional. Either way this is unlikely to cause serious issues but IMHO is confusing and could lead to weird situations in edge-case scenarios where the viewport is exactly 996px and only half the page is in compact mode.

    https://github.com/facebook/docusaurus/blob/ae3191654cc8022da39c5c425bc3dca85bf0314f/packages/docusaurus-theme-classic/src/theme/TOC/styles.module.css#L15

    https://github.com/facebook/docusaurus/blob/ae3191654cc8022da39c5c425bc3dca85bf0314f/packages/docusaurus-theme-classic/src/theme/DocRoot/Layout/Sidebar/styles.module.css#L17

    https://github.com/facebook/docusaurus/blob/ae3191654cc8022da39c5c425bc3dca85bf0314f/packages/docusaurus-theme-classic/src/theme/BlogSidebar/Desktop/styles.module.css#L41

    https://github.com/facebook/docusaurus/blob/ae3191654cc8022da39c5c425bc3dca85bf0314f/packages/docusaurus-theme-classic/src/theme/DocSidebar/Desktop/Content/styles.module.css#L8

  14. Josh-Cena commented on Oct 12, 2023

    @Josh-Cena
    Josh-Cena
    on Oct 12, 2023
    Collaborator

    @KosRud max-width: 996px is equivalent to min-width: 997px, because the breakpoint is inclusive :)

  15. KosRud commented on Oct 12, 2023

    @KosRud
    KosRud
    on Oct 12, 2023
    Author

    @Josh-Cena

    @KosRud max-width: 996px is equivalent to min-width: 997px, because the breakpoint is inclusive :)

    My bad, you are right. I missed that one is max and another is min.

    Regarding the potential fix, does replacing the px thresholds with 1/16 in rem sound reasonable?

  16. Josh-Cena commented on Oct 12, 2023

    @Josh-Cena
    Josh-Cena
    on Oct 12, 2023
    Collaborator

    It makes sense to me, but I'm not sure if it will cause problems elsewhere. For example, we also have JavaScript code that use the 996 boundary (soon to be configurable, but pixel-based nonetheless):

    https://github.com/facebook/docusaurus/blob/ae3191654cc8022da39c5c425bc3dca85bf0314f/packages/docusaurus-theme-common/src/hooks/useWindowSize.ts#L20-L29

    Therefore, I'd still be curious to know what the best practice is and examples of such.

  17. KosRud commented on Oct 12, 2023

    @KosRud
    KosRud
    on Oct 12, 2023
    Author

    @Josh-Cena

    I found a few examples of pages that use em or rem for @media rules (as well as for most sizes/margins, like docusaurus does). It works as expected, the @media rules being aware of extra scaling introduced by altered font size setting in the browser.

    https://www.ada.gov/resources/web-guidance/
    image

    https://www.goodemailcode.com/email-accessibility/rem-and-em.html
    image

    https://www.w3.org/WAI/fundamentals/accessibility-intro/
    image

    https://drafts.csswg.org/mediaqueries/

    This page also uses em for @media thresholds, but it also appears to use some javascript in conjunction with @media rules.

  18. Josh-Cena commented on Oct 13, 2023

    @Josh-Cena
    Josh-Cena
    on Oct 13, 2023
    Collaborator

    Thanks! I think the CSSWG website is convincing enough that we should do something similar.

  19. Josh-Cena
    removed
    status: needs more informationThere is not enough information to take action on the issue.
    on Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugAn error in the Docusaurus core causing instability or issues with its executiondomain: a11yRelated to accessibility concerns of the default theme

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions