Skip to content

fix: prevent background scroll when popup is open#854

Open
Sachinxmpl wants to merge 4 commits intoasyncapi:masterfrom
Sachinxmpl:popup-overflow-fix
Open

fix: prevent background scroll when popup is open#854
Sachinxmpl wants to merge 4 commits intoasyncapi:masterfrom
Sachinxmpl:popup-overflow-fix

Conversation

@Sachinxmpl
Copy link
Copy Markdown

Description

  • Added css class popup-open in globals.css
  • body.popup-open {
    overflow: hidden;
    position: fixed;
    width: 100%;
    }
  • When pop is open css class popup-open is added to body. This ensures background doesn't overflow.
  • Works in desktop, mobile
image

Related issue(s)

Fixes #852

@netlify
Copy link
Copy Markdown

netlify bot commented Dec 4, 2025

Deploy Preview for peaceful-ramanujan-288045 ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit f12052e
🔍 Latest deploy log https://app.netlify.com/projects/peaceful-ramanujan-288045/deploys/695ff7bc69c13b00083bf6df
😎 Deploy Preview https://deploy-preview-854--peaceful-ramanujan-288045.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@Sachinxmpl Sachinxmpl changed the title Popup overflow fix fix: Prevent Background Scroll when popup is open Dec 4, 2025
@Sachinxmpl Sachinxmpl changed the title fix: Prevent Background Scroll when popup is open fix: prevent background scroll when popup is open Dec 4, 2025
Copy link
Copy Markdown
Member

@princerajpoot20 princerajpoot20 left a comment

Choose a reason for hiding this comment

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

LGTM! Good work.
Thanks for the contribution

@princerajpoot20
Copy link
Copy Markdown
Member

@AceTheCreator @thulieblack PTAL

Copy link
Copy Markdown
Member

@AceTheCreator AceTheCreator left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@AceTheCreator
Copy link
Copy Markdown
Member

@Sachinxmpl i think this changes is making the test fail, kindly look into it :)

@AceTheCreator AceTheCreator self-requested a review December 11, 2025 15:00
@Sachinxmpl
Copy link
Copy Markdown
Author

@AceTheCreator My current implementation uses position: fixed on body.
7 cypress tests (that contains .should('be.visible')) are failing as it prevents Cypress from scrolling to test elements.
I think during cypress test we should close the popup like we do in some tests.

@AceTheCreator
Copy link
Copy Markdown
Member

@AceTheCreator My current implementation uses position: fixed on body. 7 cypress tests (that contains .should('be.visible')) are failing as it prevents Cypress from scrolling to test elements. I think during cypress test we should close the popup like we do in some tests.

Yea, i agree we close the popup during test

@Sachinxmpl
Copy link
Copy Markdown
Author

@AceTheCreator Can i create a issue regarding closing popup during tests. And work on it?

@princerajpoot20
Copy link
Copy Markdown
Member

@Sachinxmpl You should do this work as part of this PR only. The reason is that it is not a separate task—it is just updating the test cases to align with your current changes. Hence, it must go in the same PR

Another reason is how we can merge this PR if the pipeline is failing due to test failures introduced by your changes.

@AceTheCreator
Copy link
Copy Markdown
Member

@thulieblack, do we still need the pop-up in any instance moving forward, except for promoting Developer Week?

@TenzDelek
Copy link
Copy Markdown
Member

gentle ping @thulieblack

@thulieblack
Copy link
Copy Markdown
Member

No we no longer need it as for now, we can hide it for future instances

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Background Page Scrolls When Popup Modal Is Open

5 participants