Uh oh!
There was an error while loading. Please reload this page.
Dialog: Full screen & Bottom sheet support - #4048
Conversation
🦋 Changeset detectedLatest commit: db63923 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
mperrotti
left a comment
There was a problem hiding this comment.
I played around with this and I think I have a way forward (or at least a workaround) to resolve your Jest issues. Maybe we can pair? Ideally with another PRC engineer present 😅
Uh oh!
There was an error while loading. Please reload this page.
| ariaModal: boolean | ||
| } | ||
| export default React.forwardRef<HTMLDivElement, PropsWithChildren<DialogBottomSheetProps>>((props, forwardedRef) => { |
There was a problem hiding this comment.
| exportdefaultReact.forwardRef<HTMLDivElement,PropsWithChildren<DialogBottomSheetProps>>((props,forwardedRef)=>{ | |
| exportdefaultReact.forwardRef<HTMLDialogElement,PropsWithChildren<DialogBottomSheetProps>>((props,forwardedRef)=>{ |
There was a problem hiding this comment.
Actually I had to revert this again because the role is defined through the role prop.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…nto feature/full-screen-clean
…nto feature/full-screen-clean
mperrotti
left a comment
There was a problem hiding this comment.
It's looking good! Just one little thing we might to address: we don't delay calling onClose when clicking the "x" or keying Esc
Kapture.2024-01-23.at.19.07.57.mp4
If this is intentional, don't worry about it.
| // Arrow keys support in the testing library is currently not implemented and therefore can't be tested | ||
| // https://github.com/testing-library/user-event/issues/871 | ||
| it('expands/contracts when using the range slider', async () => { |
There was a problem hiding this comment.
If this merges before #4048, I mighjt update my PR to move these tests into the same test file.
maximedegreve
commented
Jan 24, 2024
Yeah that was intentionally because I thought you aren't swiping it away. |
mperrotti
left a comment
There was a problem hiding this comment.
Looking good. Since this is such a big PR, it might be nice to get a review from somebody who's "officially" and engineer before merging. Not required though 🙂
Closes
Note: I've chosen not to tackle the issues mentioned here to avoid causing any disruptive changes.
Changelog
New
1️⃣
action-sheetdialogWe've implemented a range slider to easily adjust the size of the dialog using only a keyboard, enhancing accessibility. We've used a similar approach for our SplitPageLayout resizer. I've also implemented reduced-motion support.
Screen.Recording.2023-12-12.at.17.14.30.mov
keyboard.mov
2️⃣
full-screendialogThis is quite simple, as it closely resembles the existing dialog behavior, with the only difference being that it's full-screen.
Screen.Recording.2023-12-12.at.17.15.47.mov
Changed
Removed
Rollout strategy
Testing & Reviewing
Merge checklist