Repository navigation
fix: finish Sheet exit when the final observer report is missed - #10829
dajiaohuang wants to merge 1 commit into
Conversation
| * OF ANY KIND, either express or implied. See the License for the specific language | ||
| * governing permissions and limitations under the License. | ||
| */ | ||
| import {addEvent, getPropagationTargets} from '@react-aria/utils'; |
There was a problem hiding this comment.
These aren't exported at the package entry, so they should be imported through react-aria/private/utils/
| } | ||
| }; | ||
| element.addEventListener('react-aria-sheet-close', finish, {once: true}); | ||
| removeScrollEnd = addEvent(getPropagationTargets(scroller), 'scrollend', onScrollEnd); |
There was a problem hiding this comment.
I don't think this will work. getPropagationTargets returns the window, not the scroller itself. scrollend events on an element don’t bubble (https://www.w3.org/TR/cssom-view/#scrolling).
| expect(tree.queryByRole('dialog')).toBeNull(); | ||
| }); | ||
|
|
||
| it('finishes exiting when the final IntersectionObserver notification is missing', async () => { |
There was a problem hiding this comment.
This would pass before this PR. The test should mock Element.prototype.getAnimations and assert that the dialog remains mounted before dispatching the exit scrollend
|
Thanks for attempting this, but in the original issue @mhasanince offered to create a PR, so I'd rather see a fix from them since they have a working solution and have tested it in their CI environment. |
Closes #10813
Summary
SheetOverlaycurrently waits for the final zero-ratioIntersectionObservercallback before resolving its exit animation. If that notification is missed, the sheet remains mounted withdata-exitingand blocks the page. Resolve the exit when the scroller reaches the computed exit position as well, or immediately when it is already there. Keep the observer event as an independent completion path and remove the scroll listener once either path finishes.Added a synthetic regression test that completes the enter scroll, marks the sheet visible, closes it without sending the observer's final zero-ratio callback, and verifies the overlay unmounts after
scrollend.Validation
corepack yarn jest packages/react-aria-components/test/Sheet.test.js --runInBand— 64 passed.corepack yarn oxlint packages/react-aria-components/src/Sheet.tsx packages/react-aria-components/test/Sheet.test.js— passed.corepack yarn oxfmt --check packages/react-aria-components/src/Sheet.tsx packages/react-aria-components/test/Sheet.test.js packages/react-aria-components/package.json— passed.git diff --check— passed.AI disclosure
Prepared with AI assistance; I reviewed and verified the change and tests.
Pull Request Checklist
Test Instructions
Run
corepack yarn jest packages/react-aria-components/test/Sheet.test.js --runInBand. The new test omits the observer's final zero-ratio notification and dispatchesscrollendat the exit position; the sheet should unmount.