Skip to content

Commit 3c82407

Browse files
feat: Add non-navigable alert modals (#37425)
<!-- 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** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> Introduces a new optional alert property to hide navigation for this alert. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/37425?quickstart=1) ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> https://github.com/user-attachments/assets/249abb46-f295-4035-b838-5e1843658410 ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/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-extension/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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds `hideFromAlertNavigation` to alerts and updates hooks/modal to skip these in navigation, with tests and usage updates. > > - **Alert Model**: > - Add optional `hideFromAlertNavigation` to `Alert`. > - **Hooks (`useAlerts`)**: > - Return `navigableAlerts`, `navigableFieldAlerts`, `navigableGeneralAlerts`, and `getNavigableFieldAlerts`, excluding alerts with `hideFromAlertNavigation`. > - Ensure severity sorting is non-mutating. > - **UI – MultipleAlertModal**: > - Drive selection by `alertKey` with robust syncing/fallback logic. > - Navigate only through `navigable*` alerts; hide nav controls if current alert sets `hideFromAlertNavigation` or when skipping is requested. > - Acknowledge just triggers `onFinalAcknowledgeClick` (no cycling). > - **UI – ConfirmInfoAlertRow**: > - Derive `selectedAlert` once; pass `skipAlertNavigation` based on `hideFromAlertNavigation`. > - **Alerts – Shield Coverage**: > - Mark shield coverage alert with `hideFromAlertNavigation: true`. > - **Tests**: > - Update/extend tests for severity ordering, navigable alert filtering, modal navigation hiding/skipping, and selection behavior. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 22c7214. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent cc732fb commit 3c82407

File tree

7 files changed

+388
-86
lines changed

7 files changed

+388
-86
lines changed

ui/components/app/alert-system/multiple-alert-modal/multiple-alert-modal.test.tsx

Lines changed: 103 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ describe('MultipleAlertModal', () => {
2323
const FROM_ALERT_KEY_MOCK = 'from';
2424
const CONTRACT_ALERT_KEY_MOCK = 'contract';
2525
const DATA_ALERT_KEY_MOCK = 'data';
26+
const HIDDEN_ALERT_KEY_MOCK = 'hidden';
2627
const onAcknowledgeClickMock = jest.fn();
2728
const onCloseMock = jest.fn();
2829

@@ -158,15 +159,20 @@ describe('MultipleAlertModal', () => {
158159
expect(onAcknowledgeClickMock).toHaveBeenCalledTimes(1);
159160
});
160161

161-
it('renders the next alert when the "Got it" button is clicked', () => {
162-
const { getByTestId, getByText } = renderWithProvider(
162+
it('does not change the alert when the "Got it" button is clicked', () => {
163+
onAcknowledgeClickMock.mockReset();
164+
const { getByTestId, getByText, queryByText } = renderWithProvider(
163165
<MultipleAlertModal {...defaultProps} alertKey={DATA_ALERT_KEY_MOCK} />,
164166
mockStoreAcknowledgeAlerts,
165167
);
166168

169+
expect(getByText(alertsMock[1].message)).toBeInTheDocument();
170+
167171
fireEvent.click(getByTestId('alert-modal-button'));
168172

173+
expect(onAcknowledgeClickMock).toHaveBeenCalledTimes(1);
169174
expect(getByText(alertsMock[1].message)).toBeInTheDocument();
175+
expect(queryByText(alertsMock[2].message)).not.toBeInTheDocument();
170176
});
171177

172178
it('closes modal when the "Got it" button is clicked', () => {
@@ -185,20 +191,6 @@ describe('MultipleAlertModal', () => {
185191
expect(onAcknowledgeClickMock).toHaveBeenCalledTimes(1);
186192
});
187193

188-
it('resets to the first alert if there are unconfirmed alerts and the final alert is acknowledged', () => {
189-
const { getByTestId, getByText } = renderWithProvider(
190-
<MultipleAlertModal
191-
{...defaultProps}
192-
alertKey={CONTRACT_ALERT_KEY_MOCK}
193-
/>,
194-
mockStore,
195-
);
196-
197-
fireEvent.click(getByTestId('alert-modal-button'));
198-
199-
expect(getByText(alertsMock[0].message)).toBeInTheDocument();
200-
});
201-
202194
describe('FieldAlerts not present', () => {
203195
const alertsMockWithoutField = [
204196
{
@@ -279,7 +271,101 @@ describe('MultipleAlertModal', () => {
279271

280272
fireEvent.click(getByTestId('alert-modal-back-button'));
281273

282-
expect(getByText(alertsMock[1].message)).toBeInTheDocument();
274+
expect(getByText(alertsMock[0].message)).toBeInTheDocument();
275+
});
276+
});
277+
278+
describe('Alerts hidden from navigation', () => {
279+
const alertsWithHiddenNavigationMock = [
280+
{
281+
key: FROM_ALERT_KEY_MOCK,
282+
field: FROM_ALERT_KEY_MOCK,
283+
severity: Severity.Warning,
284+
message: 'Alert 1',
285+
reason: 'Reason 1',
286+
alertDetails: ['Detail 1', 'Detail 2'],
287+
},
288+
{
289+
key: HIDDEN_ALERT_KEY_MOCK,
290+
field: HIDDEN_ALERT_KEY_MOCK,
291+
severity: Severity.Danger,
292+
message: 'Hidden Alert',
293+
hideFromAlertNavigation: true,
294+
},
295+
{
296+
key: CONTRACT_ALERT_KEY_MOCK,
297+
field: CONTRACT_ALERT_KEY_MOCK,
298+
severity: Severity.Info,
299+
message: 'Alert 3',
300+
},
301+
];
302+
303+
const mockStoreWithHiddenNavigation = configureMockStore([])({
304+
...STATE_MOCK,
305+
confirmAlerts: {
306+
alerts: { [OWNER_ID_MOCK]: alertsWithHiddenNavigationMock },
307+
confirmed: {
308+
[OWNER_ID_MOCK]: {
309+
[FROM_ALERT_KEY_MOCK]: false,
310+
[HIDDEN_ALERT_KEY_MOCK]: false,
311+
[CONTRACT_ALERT_KEY_MOCK]: false,
312+
},
313+
},
314+
},
315+
});
316+
317+
const mockStoreWithHiddenNavigationConfirmed = configureMockStore([])({
318+
...STATE_MOCK,
319+
confirmAlerts: {
320+
alerts: { [OWNER_ID_MOCK]: alertsWithHiddenNavigationMock },
321+
confirmed: {
322+
[OWNER_ID_MOCK]: {
323+
[FROM_ALERT_KEY_MOCK]: true,
324+
[HIDDEN_ALERT_KEY_MOCK]: true,
325+
[CONTRACT_ALERT_KEY_MOCK]: true,
326+
},
327+
},
328+
},
329+
});
330+
331+
it('does not render navigation controls when the selected alert hides navigation', () => {
332+
const { queryByTestId } = renderWithProvider(
333+
<MultipleAlertModal
334+
{...defaultProps}
335+
alertKey={HIDDEN_ALERT_KEY_MOCK}
336+
/>,
337+
mockStoreWithHiddenNavigation,
338+
);
339+
340+
expect(queryByTestId('alert-modal-next-button')).toBeNull();
341+
expect(queryByTestId('alert-modal-back-button')).toBeNull();
342+
});
343+
344+
it('skips alerts hidden from navigation when cycling forward', () => {
345+
const { getByTestId, getByText, queryByText } = renderWithProvider(
346+
<MultipleAlertModal {...defaultProps} />,
347+
mockStoreWithHiddenNavigation,
348+
);
349+
350+
fireEvent.click(getByTestId('alert-modal-next-button'));
351+
352+
expect(queryByText('Hidden Alert')).not.toBeInTheDocument();
353+
expect(getByText('Alert 3')).toBeInTheDocument();
354+
});
355+
356+
it('acknowledges an alert that hides navigation without cycling', () => {
357+
onAcknowledgeClickMock.mockReset();
358+
const { getByTestId } = renderWithProvider(
359+
<MultipleAlertModal
360+
{...defaultProps}
361+
alertKey={HIDDEN_ALERT_KEY_MOCK}
362+
/>,
363+
mockStoreWithHiddenNavigationConfirmed,
364+
);
365+
366+
fireEvent.click(getByTestId('alert-modal-button'));
367+
368+
expect(onAcknowledgeClickMock).toHaveBeenCalledTimes(1);
283369
});
284370
});
285371
});

0 commit comments

Comments
 (0)