Skip to content

Commit 6f70c0f

Browse files
authored
feat: update shield entry modal status only when user has interacted (#37925)
<!-- 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? --> This PR updates shield entry modal status only when user has interacted. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/37925?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: update shield entry modal status only when user has interacted ## **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] --> ## **Pre-merge author checklist** - [x] 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). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] 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** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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] > Defers Shield Entry modal background/state updates until after user interaction, adding an interaction flag and updating actions, selectors, provider flow, and tests. > > - **Shield Entry Modal UX/state**: > - Add `hasUserInteractedWithModal` to `appState.shieldEntryModal` and update `getShowShieldEntryModal` to show only when `show` is true and no interaction has occurred. > - Modal close/get-started now dispatch `setShowShieldEntryModalOnce({ show: false, hasUserInteractedWithModal: true })`. > - Tests updated to assert new payload shape and navigation behavior. > - **Actions/Reducer/Constants**: > - Replace `SET_SHOW_SHIELD_ENTRY_MODAL_ONCE` with `SET_SHIELD_ENTRY_MODAL_STATUS` and update reducer case. > - Refactor `setShowShieldEntryModalOnce` to accept an options object `{ show, shouldSubmitEvents, triggeringCohort, modalType, hasUserInteractedWithModal, shouldUpdateBackgroundState }`; gate background call on `shouldUpdateBackgroundState`. > - **Shield subscription flow** (`ui/contexts/shield/shield-subscription.tsx`): > - When active subscription: set `{ show: false }`. > - When eligible: show modal via `setShowShieldEntryModalOnce({ show: true, shouldSubmitEvents: true, triggeringCohort, modalType, shouldUpdateBackgroundState: false })` to postpone background state update until interaction. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit c07dbdb. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 1ca6538 commit 6f70c0f

File tree

7 files changed

+70
-29
lines changed

7 files changed

+70
-29
lines changed

ui/components/app/shield-entry-modal/shield-entry-modal.test.tsx

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,10 @@ describe('Shield Entry Modal', () => {
6767

6868
const closeButton = getByTestId('shield-entry-modal-close-button');
6969
fireEvent.click(closeButton);
70-
expect(setShowShieldEntryModalOnceSpy).toHaveBeenCalledWith(false);
70+
expect(setShowShieldEntryModalOnceSpy).toHaveBeenCalledWith({
71+
show: false,
72+
hasUserInteractedWithModal: true,
73+
});
7174
});
7275

7376
it('should call onGetStarted when the get started button is clicked', async () => {
@@ -78,7 +81,10 @@ describe('Shield Entry Modal', () => {
7881
);
7982

8083
fireEvent.click(getStartedButton);
81-
expect(setShowShieldEntryModalOnceSpy).toHaveBeenCalledWith(false);
84+
expect(setShowShieldEntryModalOnceSpy).toHaveBeenCalledWith({
85+
show: false,
86+
hasUserInteractedWithModal: true,
87+
});
8288
await waitFor(() => {
8389
expect(mockUseNavigate).toHaveBeenCalledWith({
8490
pathname: SHIELD_PLAN_ROUTE,
@@ -110,7 +116,10 @@ describe('Shield Entry Modal', () => {
110116
event: SubscriptionUserEvent.ShieldEntryModalViewed,
111117
cohort: mockState.appState.shieldEntryModal.triggeringCohort,
112118
});
113-
expect(setShowShieldEntryModalOnceSpy).toHaveBeenCalledWith(false);
119+
expect(setShowShieldEntryModalOnceSpy).toHaveBeenCalledWith({
120+
show: false,
121+
hasUserInteractedWithModal: true,
122+
});
114123
});
115124
});
116125
});

ui/components/app/shield-entry-modal/shield-entry-modal.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,12 @@ const ShieldEntryModal = ({
131131
);
132132
}
133133

134-
dispatch(setShowShieldEntryModalOnce(false));
134+
dispatch(
135+
setShowShieldEntryModalOnce({
136+
show: false,
137+
hasUserInteractedWithModal: true,
138+
}),
139+
);
135140
};
136141

137142
const handleOnGetStarted = () => {

ui/contexts/shield/shield-subscription.tsx

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,11 @@ export const ShieldSubscriptionProvider: React.FC = ({ children }) => {
150150
}
151151

152152
if (isShieldSubscriptionActive) {
153-
dispatch(setShowShieldEntryModalOnce(false));
153+
dispatch(
154+
setShowShieldEntryModalOnce({
155+
show: false,
156+
}),
157+
);
154158
return;
155159
}
156160

@@ -213,14 +217,16 @@ export const ShieldSubscriptionProvider: React.FC = ({ children }) => {
213217
return;
214218
}
215219

216-
const shouldSubmitUserEvents = true; // submits `shield_entry_modal_viewed` event
217220
dispatch(
218-
setShowShieldEntryModalOnce(
219-
true,
220-
shouldSubmitUserEvents,
221-
entrypointCohort,
221+
setShowShieldEntryModalOnce({
222+
show: true,
223+
shouldSubmitEvents: true, // submits `shield_entry_modal_viewed` event
224+
triggeringCohort: entrypointCohort,
222225
modalType,
223-
),
226+
// we will show the modal but we won't update the background state yet,
227+
// we will only update after the user has interacted with the modal
228+
shouldUpdateBackgroundState: false,
229+
}),
224230
);
225231
return;
226232
}
@@ -236,14 +242,16 @@ export const ShieldSubscriptionProvider: React.FC = ({ children }) => {
236242
modalType,
237243
);
238244
if (selectedCohort?.cohort === COHORT_NAMES.WALLET_HOME) {
239-
const shouldSubmitUserEvents = true; // submits `shield_entry_modal_viewed` event to subscription backend
240245
dispatch(
241-
setShowShieldEntryModalOnce(
242-
true,
243-
shouldSubmitUserEvents,
244-
selectedCohort.cohort,
246+
setShowShieldEntryModalOnce({
247+
show: true,
248+
shouldSubmitEvents: true, // submits `shield_entry_modal_viewed` event to subscription backend
249+
triggeringCohort: selectedCohort.cohort,
245250
modalType,
246-
),
251+
// we will show the modal but we won't update the background state yet,
252+
// we will only update after the user has interacted with the modal
253+
shouldUpdateBackgroundState: false,
254+
}),
247255
);
248256
}
249257
}

ui/ducks/app/app.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,10 @@ type AppState = {
139139
shouldSubmitEvents: boolean;
140140
modalType?: ModalType;
141141
triggeringCohort?: string;
142+
/**
143+
* Whether the user has interacted with the modal.
144+
*/
145+
hasUserInteractedWithModal?: boolean;
142146
};
143147
};
144148

@@ -797,7 +801,7 @@ export default function reduceApp(
797801
showSupportDataConsentModal: action.payload,
798802
};
799803

800-
case actionConstants.SET_SHOW_SHIELD_ENTRY_MODAL_ONCE:
804+
case actionConstants.SET_SHIELD_ENTRY_MODAL_STATUS:
801805
return {
802806
...appState,
803807
shieldEntryModal: {

ui/selectors/selectors.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,10 @@ export function getKeyringSnapRemovalResult(state) {
290290
export const getPendingTokens = (state) => state.appState.pendingTokens;
291291

292292
export function getShowShieldEntryModal(state) {
293-
return state.appState.shieldEntryModal?.show;
293+
const { show, hasUserInteractedWithModal } =
294+
state.appState.shieldEntryModal || {};
295+
// only show the modal if `show` is true and user has not interacted with the modal
296+
return Boolean(show) && !hasUserInteractedWithModal;
294297
}
295298

296299
export function getPendingShieldCohort(state) {

ui/store/actionConstants.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,5 +196,4 @@ export const SET_SHOW_CLAIM_SUBMIT_TOAST = 'SET_SHOW_CLAIM_SUBMIT_TOAST';
196196
export const SET_SHOW_SUPPORT_DATA_CONSENT_MODAL =
197197
'SET_SHOW_SUPPORT_DATA_CONSENT_MODAL';
198198

199-
export const SET_SHOW_SHIELD_ENTRY_MODAL_ONCE =
200-
'SET_SHOW_SHIELD_ENTRY_MODAL_ONCE';
199+
export const SET_SHIELD_ENTRY_MODAL_STATUS = 'SET_SHIELD_ENTRY_MODAL_STATUS';

ui/store/actions.ts

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -654,21 +654,33 @@ export function getSubscriptionBillingPortalUrl(): ThunkAction<
654654
};
655655
}
656656

657-
export function setShowShieldEntryModalOnce(
658-
show: boolean | null,
659-
shouldSubmitEvents: boolean = false,
660-
triggeringCohort?: string,
661-
modalType?: ModalType,
662-
): ThunkAction<void, MetaMaskReduxState, unknown, AnyAction> {
657+
export function setShowShieldEntryModalOnce({
658+
show,
659+
shouldSubmitEvents = false,
660+
triggeringCohort,
661+
modalType,
662+
hasUserInteractedWithModal = false,
663+
shouldUpdateBackgroundState = true,
664+
}: {
665+
show: boolean | null;
666+
shouldSubmitEvents?: boolean;
667+
triggeringCohort?: string;
668+
modalType?: ModalType;
669+
hasUserInteractedWithModal?: boolean;
670+
shouldUpdateBackgroundState?: boolean;
671+
}): ThunkAction<void, MetaMaskReduxState, unknown, AnyAction> {
663672
return async (dispatch: MetaMaskReduxDispatch) => {
664673
try {
665-
await submitRequestToBackground('setShowShieldEntryModalOnce', [show]);
674+
if (shouldUpdateBackgroundState) {
675+
await submitRequestToBackground('setShowShieldEntryModalOnce', [show]);
676+
}
666677
dispatch(
667678
setShowShieldEntryModalOnceAction({
668679
show: Boolean(show),
669680
shouldSubmitEvents,
670681
triggeringCohort,
671682
modalType,
683+
hasUserInteractedWithModal,
672684
}),
673685
);
674686
} catch (error) {
@@ -684,9 +696,10 @@ export function setShowShieldEntryModalOnceAction(payload: {
684696
shouldSubmitEvents: boolean;
685697
triggeringCohort?: string;
686698
modalType: ModalType;
699+
hasUserInteractedWithModal?: boolean;
687700
}): ThunkAction<void, MetaMaskReduxState, unknown, AnyAction> {
688701
return {
689-
type: actionConstants.SET_SHOW_SHIELD_ENTRY_MODAL_ONCE,
702+
type: actionConstants.SET_SHIELD_ENTRY_MODAL_STATUS,
690703
payload,
691704
};
692705
}

0 commit comments

Comments
 (0)