feat: add bottom sheet dialog#6551
Conversation
| const rootElement = document.getElementById('root')! | ||
| const root = createRoot(rootElement) | ||
|
|
||
| rootElement.setAttribute('vaul-drawer-wrapper', '') | ||
| rootElement.classList.add('app-height') |
There was a problem hiding this comment.
Do we have to set this here? Feels like a file we shouldn't touch if we can avoid
There was a problem hiding this comment.
Its supposed to be set on the top level div wrapping the app. But I can prob wrap the entire app in another div, but will have to do a bunch of testing again to see if it still works as expected.
There was a problem hiding this comment.
@gomesalexandre should I move this off the #root component? Or can we just leave as is, i'm worried if I move it to another div it might screw something up.
gomesalexandre
left a comment
gomesalexandre
left a comment
There was a problem hiding this comment.
First pass, LGTM for the most part as this mostly renames the Modal vernacular to Dialog - to be followed up by a regression test of all the diffed surface area
gomesalexandre
left a comment
gomesalexandre
left a comment
There was a problem hiding this comment.
Tested on desktop and couldn't find any regressions.
Asset search in asset page is still happy
Send asset search modal is still happy
Receive is still happy
Swapper asset search is still happy
Sends are still happy
Not a regression but noticed we're initially in an errored state @reallybeard, I'll take a look and fix this as a separate one-liner of sorts
QR code scanning is still happy
Not a regression from this PR as I could repro in develop but noted the modal re-renders/re-animates 3 times before being in a usable state.
gomesalexandre
left a comment
gomesalexandre
left a comment
There was a problem hiding this comment.
Tested on mobile app and couldn't find any regressions in sends (including QR codes), receives, swapper and asset search 🎉
Great work on this sexy UI rework ser!
0xApotheosis
left a comment
0xApotheosis
left a comment
There was a problem hiding this comment.
Code looks good.
Noticed one issue that we probably want to fix before we merge the back button is a dead click:
back-button-dead.mp4
fixed |
This reverts commit 56c2d84.
3 participants
Description
This adds the new bottom sheet dialog for mobile.
Right now this is only hooked up on send, receive, and asset select modal.
Pull Request Type
Issue (if applicable)
closes #6434
Risk
high risk, this touched the send modal. So this needs to be tested.
Testing
Engineering
Operations
You are able to use send, receive, and select assets in the mobile app as before with the normal modal.
Its important we test the send flow end to end, to make sure it works as normal.
Screenshots (if applicable)