Skip to content

Fix breakpoint detection on mobile with content overflow - #1450

Open
qroll wants to merge 2 commits into
masterfrom
fix-breakpoint-detection
Open

Fix breakpoint detection on mobile with content overflow#1450
qroll wants to merge 2 commits into
masterfrom
fix-breakpoint-detection

Conversation

@qroll

@qroll qroll commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Type of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing apis or functionality to change)
  • Documentation (change to documentation, comments or API descriptions)
  • Tests (improvements to unit tests or E2E tests)
  • Other (technical improvements, refactoring, or changes that don't fall into the above categories)

Description of changes

  • Use the visual viewport to calculate the actual screen width instead of window.innerWidth, which could grow beyond the visible viewport when there is overflow content (without overflow hidden)
  • Original issue reported:
    • a responsive element was rendered: on md, it's a flex row, on sm, it's a flex column
    • on page load, it renders as the flex row as expected
    • but when resizing the window to sm, it remains as flex row instead of switching to flex column. on inspection, the breakpoint class remains on fds-breakpoint-md
    • this was demonstrated with Chrome's Responsive Viewport Mode

Checklist

  • Changes follow the project guidelines in CONTRIBUTING.md and CONVENTIONS.md
  • Looks good on mobile and tablet
  • Updated documentation
  • Added/updated unit tests - can't be verified in unit tests
  • Added/updated E2E tests

@qroll
qroll force-pushed the fix-breakpoint-detection branch from 9ef6fc2 to c1bcbb3 Compare September 2, 2026 03:28
@qroll
qroll force-pushed the fix-breakpoint-detection branch from c1bcbb3 to 64d819a Compare September 2, 2026 03:33
@qroll qroll added the type: bug Something isn't working label Sep 2, 2026
@qroll
qroll marked this pull request as ready for review September 2, 2026 05:03
@qroll qroll changed the title Get screen width from visual viewport in breakpoint class calculation Fix breakpoint detection on mobile with content overflow Sep 2, 2026
@qroll
qroll requested a review from ghazwan-gt September 2, 2026 05:06
const width = window.innerWidth;

const width = window.visualViewport
? window.visualViewport.width * window.visualViewport.scale

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we do Math.round instead here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean to round the final value of width? or to round window.visualViewport?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean for the calculation result

Math.round(window.visualViewport.width * window.visualViewport.scale)

Just in case the value has fractional result e.g. x.999, could cause a 1px mismatch

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, let me look into the rounding behaviour of media queries to remain consistent with our javascript checks

@qroll qroll Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm yeah, there seems to be some rounding involved

Chrome
599
600

Safari
Screenshot 2026-09-02 at 11 40 08 PM

Safari iOS has some bizarre behaviour with the media query range syntax, but that could have been fixed in the latest v26 patch:

Screenshot 2026-09-02 at 11 50 11 PM

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps switching over to use window.matchMedia would give us more consistent results

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants