Skip to content

fix(PopoverContainer): Add all scrolling parents to closeOnViewportExit #3208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
codecademydev merged 6 commits into from
Nov 13, 2025
Merged

fix(PopoverContainer): Add all scrolling parents to closeOnViewportExit #3208

codecademydev merged 6 commits into from
Nov 13, 2025
+447 −102

Conversation

@dreamwasp
Copy link
Contributor

@dreamwasp dreamwasp commented Nov 10, 2025 •
edited
Loading

Overview

Fixed PopoverContainer not closing when targets scroll out of view by checking all scrollable parents (including document.documentElement for iframes), added tests, and refactored some code

PR Checklist

  • Head to the DataTable story
  • See that when you open a menu, it now closes when scrolling away
  • Check out the full page story
  • See that when you open a menu, it closes when scrolling away

Testing Instructions

  1. Go to DataTable Sorting and filtering story
  2. See the menus now close on scroll down

@dreamwasp dreamwasp marked this pull request as ready for review November 10, 2025 16:45
@dreamwasp dreamwasp requested a review from a team as a code owner November 10, 2025 16:45
@nx-cloud
Copy link

nx-cloud bot commented Nov 10, 2025 •
edited
Loading

View your CI Pipeline Execution ↗ for commit 5d7c608

☁️ Nx Cloud last updated this comment at 2025-11-12 16:00:44 UTC

@dreamwasp dreamwasp marked this pull request as draft November 11, 2025 18:11
@dreamwasp dreamwasp changed the title docs(DataTable): Add closeOnViewportExit fix(PopoverContainer): Add closeOnViewportExit Nov 11, 2025
@dreamwasp dreamwasp marked this pull request as ready for review November 11, 2025 20:57
@dreamwasp dreamwasp changed the title fix(PopoverContainer): Add closeOnViewportExit fix(PopoverContainer): Add all scrolling parents to closeOnViewportExit Nov 12, 2025
@dreamwasp dreamwasp marked this pull request as draft November 12, 2025 14:46
@codecademydev
Copy link
Collaborator

codecademydev commented Nov 12, 2025

📬 Published Alpha Packages:

@codecademy/[email protected]
@codecademy/[email protected]
@codecademy/[email protected]

@github-actions
Copy link
Contributor

github-actions bot commented Nov 12, 2025

🚀 Styleguide deploy preview ready!

Preview URL: https://6914aff94b671700b8efd2a5--gamut-preview.netlify.app
Deploy Logs: https://app.netlify.com/projects/gamut-preview/deploys/6914aff94b671700b8efd2a5

@dreamwasp dreamwasp marked this pull request as ready for review November 12, 2025 16:06
LinKCoding
LinKCoding approved these changes Nov 12, 2025
View reviewed changes
Copy link
Contributor

@LinKCoding LinKCoding left a comment

Choose a reason for hiding this comment

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

A lot of great clean up and tests!
🔥 🔥 🔥

@dreamwasp dreamwasp added the Ship It :shipit: Automerge this PR when possible label Nov 13, 2025
@codecademydev codecademydev merged commit f8d0ce5 into main Nov 13, 2025
19 of 20 checks passed
@codecademydev codecademydev removed the Ship It :shipit: Automerge this PR when possible label Nov 13, 2025
@codecademydev codecademydev deleted the cass-GMT-222-doc branch November 13, 2025 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LinKCoding LinKCoding LinKCoding approved these changes

@jakemhiller jakemhiller Awaiting requested review from jakemhiller jakemhiller is a code owner automatically assigned from Codecademy/gamut

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dreamwasp @codecademydev @LinKCoding