-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
chore: Update headers for Region Modals #25084
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…to header/regionmodals
…to header/regionmodals
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
app/components/UI/Ramp/Aggregator/components/RegionSelectorModal/RegionSelectorModal.tsx
Outdated
Show resolved
Hide resolved
app/components/UI/Ramp/Aggregator/components/RegionSelectorModal/RegionSelectorModal.tsx
Outdated
Show resolved
Hide resolved
…to header/regionmodals
|
Skipping sonar cloud. Added appropriate header-related tests. Any other failures is outside of this PR's scope |
…to header/regionmodals
…to header/regionmodals
…to header/regionmodals
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
| fireEvent.press(closeButton); | ||
|
|
||
| expect(mockGoBack).toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test missing AAA blank line separation between Arrange and Act
Low Severity · Bugbot Rules
The new test violates the MANDATORY AAA pattern rule. The blank line at line 193 splits the Arrange section (between renderWithProvider and getByTestId), but there's no blank line between Arrange (getByTestId) and Act (fireEvent.press). Per the unit testing guidelines: "EVERY test MUST follow the AAA pattern (Arrange, Act, Assert) with blank line separation". The Arrange statements (renderWithProvider and getByTestId) should be grouped together, followed by a blank line before the Act section.
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection: Performance Test Selection: |
|
|
Skipping sonar cloud since the duplication error does not relate to this PR |
|
Hey @brianacnguyen I see in the description you mention updating the "Ramp Aggregator RegionSelectorModal" as one of the modals that was updated, but I don't see that file (app/components/UI/Ramp/Aggregator/components/RegionSelectorModal/RegionSelectorModal.tsx) in the current diff. |
My bad that was already split out into a different PR. I have updated the description |




Description
Refactored region and state selector modals to use the new
HeaderCentercomponent fromcomponents-tempinstead of the legacyBottomSheetHeadercomponent. This change modernizes the header implementation by:Box,Textfrom@metamask/design-system-react-native) with Tailwind styling instead ofStyleSheettitleprop directly instead of wrapping text in<Text>childrenThe following modals were updated:
Changelog
CHANGELOG entry: chore: migrated region and state selector modal headers to use HeaderCenter component
Related issues
Fixes: https://consensyssoftware.atlassian.net/jira/software/c/projects/MDP/boards/2972?assignee=62afb43d33a882e2be47c36f&quickFilter=3325&selectedIssue=MDP-279
Manual testing steps
Screenshots/Recordings
Before
After
Simulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2026-01-22.at.20.43.31.mov
Simulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2026-01-22.at.20.54.14.mov
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Low Risk
Low risk UI refactor replacing
BottomSheetHeaderwithHeaderCenterand updating tests/snapshots; main behavioral change is wiring explicit close buttontestIDs and verifying close interactions.Overview
Migrates the Card onboarding
RegionSelectorModaland DepositStateSelectorModalheaders from legacyBottomSheetHeaderto the newcomponents-tempHeaderCenter, passing titles via thetitleprop and adding explicit close buttontestIDs (region-selector-close-button,state-selector-close-button).Updates associated tests by removing now-unneeded header/design-system mocks, adjusting selectors to the new close button IDs, adding a close-button behavior test for
StateSelectorModal, and regenerating snapshots to reflect the new header layout/styling.Written by Cursor Bugbot for commit 098c8d5. This will update automatically on new commits. Configure here.