Skip to content

Conversation

devongovett
Copy link
Member

@devongovett devongovett commented Oct 2, 2025

Found a bug that occurs in our CRA example app, where the body has height: 100%. If you open a modal, the underlay is not visible. We need to use the scrollHeight instead of the clientHeight / bounding client rect.

Test instructions:

  • Open CRA example app
  • Scroll down to the Overlay section and click the buttons
  • Verify that the modal underlay appears
  • Smoke test S1/S2/RAC storybook and docs for modal examples.

let scrollingElement = isScrollable(document.body) ? document.body : document.scrollingElement || document.documentElement;
// Prevent Firefox from adding scrollbars when the page has a fractional height.
let fractionalHeightDifference = scrollingElement.clientHeight - scrollingElement.getBoundingClientRect().height;
pageHeight = scrollingElement.scrollHeight - fractionalHeightDifference;
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think there's another way to get the scrollHeight but not rounded?

@rspbot
Copy link

rspbot commented Oct 2, 2025

@rspbot
Copy link

rspbot commented Oct 2, 2025

snowystinger
snowystinger previously approved these changes Oct 2, 2025
reidbarber
reidbarber previously approved these changes Oct 2, 2025
@LFDanLu LFDanLu dismissed stale reviews from reidbarber and snowystinger via 42cb451 October 2, 2025 15:47
@LFDanLu
Copy link
Member

LFDanLu commented Oct 2, 2025

grabbing a build for the apps with the last couple of changes just in case

@rspbot
Copy link

rspbot commented Oct 2, 2025

@LFDanLu
Copy link
Member

LFDanLu commented Oct 2, 2025

https://www.chromatic.com/build?appId=5f0dd5ad2b5fc10022a2e320&number=1051
Would be good if someone on iOS 26 could double check tray behavior

@dannify dannify added the release label Oct 2, 2025
@devongovett devongovett added this pull request to the merge queue Oct 2, 2025
Merged via the queue into main with commit b2f3515 Oct 2, 2025
32 checks passed
@devongovett devongovett deleted the fix-underlay-height branch October 2, 2025 17:10
@LFDanLu LFDanLu mentioned this pull request Oct 2, 2025
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants