AnchoredOverlay + useAnchoredPosition: close overlay when menu goes out of view
Changes
- Adds an optional argument to
useAnchoredPositionto observe changes to anchor element:observe - AnchoredPosition now uses useAnchoredPosition with
observewhich means overlay re-positions based on changes to anchor's position (scroll, resize, etc) - AnchoredOverlay closes when anchor goes out of view (includes ActionMenu and SelectPanel, but not Autocomplete)
- Fixes https://github.com/primer/react/issues/2184
see video
https://user-images.githubusercontent.com/1863771/187722377-b98399fd-61ce-4065-b061-207e0674e5cf.mov
Notes for reviewer:
- Have kept the change in primer/react and not upstreamed to primer/behaviours because I'm not sure about the long term tradeoffs yet.
- Using reach/observe-rect which calls getBoundingRect on every requestAnimationFrame. Added 2 performance optimisations: 2.1 Observer is only initiated when overlay is open 2.2 Callback is debounced (16ms) to make sure we're not updating state too fast
- Considered alternate approach of adding scroll listeners on every parent up the tree, but decided against it because it attaches events on application elements
Checklist:
- [x] Check if new dependency works across build systems (library has support for cjs, esm and umd)
- [ ] Is this an okay way to load lodash?
- [ ] Check if there is a performance regression because of observer (using debounce but need to test with memex)
- [ ] Check if this approach is accessible (a11y discussion on slack for github staff)
- [ ] Check if the type changes make sense for OnCloseGesture (maybe remove them from this PR)
- [ ] Check if Autocomplete should also add observe with useAnchoredPosition
๐ฆ Changeset detected
Latest commit: 3505214d355fc129c3f98ca1107b7e3bab738279
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 1 package
| Name | Type |
|---|---|
| @primer/react | Minor |
Not sure what this means? Click here to learn what changesets are.
Click here if you're a maintainer who wants to add another changeset to this PR
size-limit report ๐ฆ
| Path | Size |
|---|---|
| dist/browser.esm.js | 101.69 KB (+33.25% ๐บ) |
| dist/browser.umd.js | 102.1 KB (+32.72% ๐บ) |
๐ Hi @siddharthkp - Would you cc @primer/accessibility-reviewers when this is out of Draft? ๐
cc @primer/accessibility-reviewers for review :)
Context: In the tree view in memex, when you interact with a table cell, it opens a select panel and we allow users to scroll the table while select panel is open.
While scrolling, when the table cell that opened the select panel goes out of view, i'd like to close the dialog automatically, but I'm not sure what are the accessibility concerns of this approach
if you'd like to see a running example in memex, it's live here: https://github-9b5cf0a362-12535706.drafts.github.io/orgs/app/projects/1
cc @primer/accessibility-reviewers for review ๐
@siddharthkp Iโm not sure this is an โaccessibilityโ issue per se, but I noticed some unexpected behavior related to scroll-bouncing while I was testing this, so I made a screen recording. Iโm using Safari on macOS. When I overscroll the list of rows, I briefly see several empty rows, then scroll position snaps/bounces back. Sometimes, this means a cell can briefly go offscreen but then quickly return on-screen. When that cell has an open menu, the menu is closed, even if the cell is again visible after the scrolling+bouncing is complete.
Caption: Screen recording of an open menu closing after its associated cell bounces back into view post-scrolling:
https://user-images.githubusercontent.com/3104489/189673468-5aec1f77-cec5-40a9-9cee-e4d9466040c0.mov
Other menus on github.com remain open as the page is scrolled, even if they are scrolled out of view. Here is the issue comment menu, for example (screen recording below). Iโd expect all menus to behave consistently.
Caption: Screen recording showing that the issue comment three-dot menu remains open even after it scrolls out-of-view:
https://user-images.githubusercontent.com/3104489/189692010-67f98bba-4f1f-473c-9099-8ae086eb3aaa.mov
Other menus on github.com remain open as the page is scrolled, even if they are scrolled out of view. Here is the issue comment menu, for example (screen recording below). Iโd expect all menus to behave consistently.
Yes! that was my concern as well. The suggestion to close it came after my first iteration where it stays open:
Description: There is a sticky header above the table so the select panel awkwardly floats on when the table cell is "tucked under" the header.
https://user-images.githubusercontent.com/1863771/189927290-f14caf6b-46dc-4983-8501-ddd7a888f28d.mov
(in the video, ignore the select panel jumping around, that's not the recommendation)
One solution would be to "tuck in" the select panel under sticky header as well. From an implementation point of view, that might be tricky or finicky because the select panel needs to render in a portal "above" the table but below the header.
๐๐ป @siddharthkp just doing some triage on the PRC board and this one has been in "ready for review" for a while. Is this still active?
Is this still active?
We still need it ๐ but I haven't touched this in few weeks since floating it for a11y feedback. Will have to schedule this to a different time, going to convert to draft for now
Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.