[Feat]: Replace <Tooltip> instances with useTooltipModal and BottomModal with BottomSheet #12656
Labels
team-confirmations
Push issues to confirmations team
Comments
|
I tried to use BottomSheet im place of BottomModal, but |
This was referenced Jan 30, 2025
|
Created a separate ticket to handle Confirm page BottomModal → BottomSheet separately |
github-merge-queue bot
pushed a commit
that referenced
this issue
Feb 28, 2025
…13268) <!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Updates: - Replaces BottomModal with BottomSheet in redesign Confirm page - Remove min height to prevent redesign Confirm page from showing excess space - Allow BottomSheet styles (rounded corners, footer bottom styles, and device padding detection padding) rather than custom BottomModal styles - Update BlockaidBanner to remove surrounding margin to allow for better reusability - Replace BlockaidBanner instances margin overrides with surrounding container margins - Update BottomSheet to pass style to BottomSheetDialog sheet cc: @brianacnguyen - Note: I think we don't need the additional background colors for header and footer since the parent, BottomSheet animated view sets the background color. Once I removed them, it triggered many snapshot tests and code owner reviews from 5 additional teams. Instead of making this change, we will continue to update the bottomsheet, footer, and header when applicable Fixes: - Fix TransactionReview BlockaidBanner negative padding → turn to positive padding to allow space between next element - Fix Confirm.test console.errors ## **Related issues** Fixes: #13267 Todo follow-up - Related Issue: #12656 ## **Manual testing steps** Test BottomSheets and BlockaidBanner alerts work as expected. Test redesign confirmation page works as expected ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### Before using BottomModal and no scroll behavior https://github.com/user-attachments/assets/0d4c5453-5824-47fd-8aa5-7405cf19de53 ### After using BottomSheet with scroll behavior <img width="320" src="https://github.com/user-attachments/assets/a908f81f-1a28-4e8c-966a-a390430dc7d9"> https://github.com/user-attachments/assets/8f8e2344-7706-49a4-b773-a5226b7826d4 ### Before 70% min height <img width="320" src="https://github.com/user-attachments/assets/94509d26-5e20-414d-942f-56938620f90a"> ### After remove 70% min height <img width="320" src="https://github.com/user-attachments/assets/641022c5-6f23-47a5-9f00-4e9358a96d3b"> ### **Before Transaction Review BlockaidBanner negative padding** <img width="320" src="https://github.com/user-attachments/assets/c057c4e7-c7a7-42f2-8251-b4f2c075a802"> ### **After Transaction Review BlockaidBanner positive padding ** <img width="320" src="https://github.com/user-attachments/assets/de1ecadb-c7ef-49e1-8ae2-6ce1b5d16997"> ### Before Confirm.test console errors   ### After Confirm.test no console.errors  ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: Brian Nguyen <brianacnguyen@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What is this about?
BottomModal uses react-native-modal modal. This prevents us from making use of BottomSheet.
useTooltipModal renders a BottomSheet.
We want to replace instances with the useTooltipModal hook and with
Per the design request, we want to use useTooltipModal to render BottomSheets instead of new custom tooltips. See:
app/components/Views/confirmations/components/UI/InfoRow/InfoRow.tsx→Scenario
No response
Design
No response
Technical Details
No response
Threat Modeling Framework
No response
Acceptance Criteria
No response
Stakeholder review needed before the work gets merged
References
No response
The text was updated successfully, but these errors were encountered: