-
Notifications
You must be signed in to change notification settings - Fork 5.4k
feat: show shield entry modal on settings #37606
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
base: main
Are you sure you want to change the base?
Conversation
|
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. |
| if (key === TRANSACTION_SHIELD_ROUTE && !hasShieldSubscription) { | ||
| this.setState({ showShieldEntryModal: true }); | ||
| return; | ||
| } |
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.
Bug: Shield Subscription Status Blocks Access Modal
The condition !hasShieldSubscription prevents the modal from showing for users with paused, cancelled, or past_due subscriptions. Since getHasShieldSubscription returns true for any subscription regardless of status, users with inactive subscriptions won't see the entry modal but also can't access the Transaction Shield page, leaving them unable to manage their subscription.
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.
User with existing shield subscription should redirect to Transaction Shield page. We have different UI inside Transaction Shield page to show those states
Builds ready [7a1fc95]
UI Startup Metrics (1223 ± 87 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
| } | ||
|
|
||
| export function getHasShieldSubscription(state: SubscriptionState): boolean { | ||
| const shieldSubscription = getShieldSubscription( |
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.
is this logic right @tuna1207 @lwin-kyaw
we don't wanna show the modal if the user has subscribed to shield atleast once
Description
If user has no shield subscription and click on Settings > Transaction Shield, show
Shield entry modalinstead of redirecting toShield planpage.On this case, Shield entry modal is just a normal modal and does not affect existing conditions on showing the entry modal.
Changelog
CHANGELOG entry: Add Shield entry modal to settings page
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
User without Shield subscription
settings-shield-modal.mov
Subscribed user
settings-shield-has-subs.mov
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Cursor Bugbot is generating a summary for commit 7a1fc95. Configure here.