feat: Add option to hide return modal#1350
Conversation
0xEdouardEth
force-pushed
the
hideReturnToApp
branch
from
57137f8 to
72edc58
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1350 +/- ##
==========================================
+ Coverage 74.93% 74.99% +0.05%
==========================================
Files 184 184
Lines 4513 4519 +6
Branches 1105 1108 +3
==========================================
+ Hits 3382 3389 +7
+ Misses 1131 1130 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| this.state.hideReturnToAppModal = | ||
| hideReturnToAppModal ?? this.state.hideReturnToAppModal; |
There was a problem hiding this comment.
Should the fallback be false? I guess I'm not clear if the goal here is to preserve a previous value on the same instance? Is that even possible? Won't this value clear once the stream is torn down?
There was a problem hiding this comment.
It’s to fall back to the default value set in the state at line 29.
public state: RemoteCommunicationPostMessageStreamState = {
_name: null,
remote: null,
deeplinkProtocol: false,
hideReturnToAppModal: false,
platformManager: null,
};
| connector?: RemoteCommunication; | ||
| qrcodeLink?: string; | ||
| useDeeplink?: boolean; | ||
| hideReturnToAppModal?: boolean; |
There was a problem hiding this comment.
[nit] since this isn't a modal anymore I suppose we should make it more generic: hideReturnToAppNotification?
There was a problem hiding this comment.
Totally agree :)
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
* feat: Add option to hide return modal * fix: review * fix: test * fix: lint * fix: test * fix: cursor comment MetaMask/metamask-sdk#1350 (comment)
2 participants
Explanation
References
Checklist
Note
Introduces a new hideReturnToAppNotification option (default false) and passes it through to deeplink/QR URLs as hr=0/1; refactors SDK option initialization and updates tests.
hideReturnToAppNotification(defaultfalse) toMetaMaskSDKOptionsanddefaultMetaMaskSDKOptions.defaultMetaMaskSDKOptions; auto-filldappMetadata.urlif missing in web env.performSDKInitializationand expected options in tests.hideReturnToAppNotificationtoRemoteConnectionState; initialize fromsdk.options.startConnection, appendhr=0/1tolinkParamsand ensure it’s included forconnectWithflows; QR link reflectshr.hideReturnToAppNotificationtoRemoteCommunicationPostMessageStreamstate and constructor.getPostMessageStreamforwards the flag fromremoteConnection.state.write, includehr=0/1in deeplink URL params based onstate.hideReturnToAppNotification.hr=0/1in generated links.Written by Cursor Bugbot for commit 7caf78f. This will update automatically on new commits. Configure here.