-
Notifications
You must be signed in to change notification settings - Fork 5k
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
feat: Integrate JSX into snap notifications #27407
Merged
+451
−216
Merged
Changes from 102 commits
Commits
Show all changes
109 commits
Select commit
Hold shift + click to select a range
2968d94
make adjustments to show notification call
hmalik88 39f7d1b
Bump Snaps packages
Mrtenz 652a6c3
Merge branch 'develop' into hm/integrate-jsx-notifications
hmalik88 bdd1935
Bump dependencies again
Mrtenz 0e30d3a
update notification controller, update raw snap notification type
hmalik88 4597ba1
Merge branch 'develop' into hm/integrate-jsx-notifications
hmalik88 5689fe5
Merge branch 'mrtenz/bump-snaps-packages' into hm/integrate-jsx-notif…
hmalik88 3d5a9c4
add snap notification modal
hmalik88 1dfa869
Merge branch 'develop' into mrtenz/bump-snaps-packages
hmalik88 306d898
feat(snaps): Connect `getCurrencyRate` hook to `multichainRateControl…
GuillaumeRx f375215
feat: Integrate MetaMask links into Snaps `Link` component (#27377)
hmalik88 a6b6016
Merge branch 'mrtenz/bump-snaps-packages' into hm/integrate-jsx-notif…
hmalik88 8377776
Merge branch 'mrtenz/bump-snaps-packages' into hm/integrate-jsx-notif…
hmalik88 aad36db
integrate snaps expanded view into notification system
hmalik88 bd96aef
update yarn lock
hmalik88 f1e5e52
update lavamoat policies
hmalik88 f26bbb4
lint fixes
hmalik88 e60733e
another lint fix
hmalik88 8e96b0e
more lint fixes
hmalik88 ea99b02
fix type issues
hmalik88 f7168be
fix export
hmalik88 be72bb6
possible fix
hmalik88 c74c866
use literal instead of enum
hmalik88 2f49777
update notification detail button test
hmalik88 48f2567
fix notification detail block explorer button test
hmalik88 391663e
update snap-utils
hmalik88 174295b
dedupe
hmalik88 e31f656
Merge branch 'develop' into mrtenz/bump-snaps-packages
hmalik88 e14230f
Merge branch 'mrtenz/bump-snaps-packages' into hm/integrate-jsx-notif…
hmalik88 6d03046
update yarn.lock
hmalik88 d50fdc2
Merge branch 'mrtenz/bump-snaps-packages' into hm/integrate-jsx-notif…
hmalik88 cbfc9e4
update yarn.lock
hmalik88 d16283f
dedupe
hmalik88 0fb7067
Merge branch 'mrtenz/bump-snaps-packages' into hm/integrate-jsx-notif…
hmalik88 6fc9fc7
regenerate lock file
hmalik88 6a0038a
update policy
hmalik88 43fc317
update policy
hmalik88 27f06b2
possible fix
hmalik88 7de27e9
revert changes
hmalik88 090d973
update policy final time?
hmalik88 4e994b4
Bump dependencies again again
Mrtenz 350fde4
Bump execution environment URL
Mrtenz 426140d
Merge branch 'mrtenz/bump-snaps-packages' into hm/integrate-jsx-notif…
hmalik88 dbe384e
changes per PR comments
hmalik88 a7b0b4c
regenerate yarn.lock
hmalik88 5b34018
make change per comment
hmalik88 ddef404
fix
hmalik88 29b116b
lint fix
hmalik88 ec9a418
possible import fix
hmalik88 4573c85
another possible fix
hmalik88 76ccce6
Fix circular dependency issues
FrederikBolding afe7809
Fix another circular dependency
FrederikBolding 7c18e47
Merge branch 'develop' into mrtenz/bump-snaps-packages
Mrtenz fbe1ca7
feat(snaps): Add `size` prop to `Heading` (#27721)
GuillaumeRx 7de9c14
lint fix
hmalik88 3b6141e
import fix
hmalik88 74aa414
another fix
hmalik88 bc47345
Merge branch 'mrtenz/bump-snaps-packages' into hm/integrate-jsx-notif…
hmalik88 0ff9ab2
add missing section
hmalik88 efed42e
lint fix
hmalik88 75174e2
lint fix
hmalik88 ea3cac7
fix undefined error
hmalik88 67c23d3
fix bug and make style changes
hmalik88 cc5a9e2
fix styling
hmalik88 c5a2982
Merge branch 'develop' into hm/integrate-jsx-notifications
hmalik88 6ca3c8a
regenerate yarn lock
hmalik88 f9dcbf2
Merge branch 'develop' into hm/integrate-jsx-notifications
hmalik88 ffc2c9e
Revert "Merge branch 'develop' into hm/integrate-jsx-notifications"
hmalik88 73f32b9
Revert "Revert "Merge branch 'develop' into hm/integrate-jsx-notifica…
hmalik88 05e8d0e
undo
hmalik88 a5dc289
fix
hmalik88 73e0ba5
Revert "undo"
hmalik88 14ce024
fix yarn lock
hmalik88 6290f92
Merge branch 'develop' into hm/integrate-jsx-notifications
hmalik88 14d6ab0
lint fix
hmalik88 4091c0a
fix read action in notification list item
hmalik88 374f5fd
remove extra hook
hmalik88 1b9b48e
fix notification-details
hmalik88 e844105
lint fix
hmalik88 f05912d
revert change to actions
hmalik88 fbf0de6
update components
hmalik88 bad7f15
Merge branch 'develop' into hm/integrate-jsx-notifications
hmalik88 fe6cb7e
lint fix
hmalik88 b55f393
lint fix
hmalik88 ecdf8ea
another lint fix
hmalik88 2194a13
fixes
hmalik88 566f4c3
Merge branch 'develop' into hm/integrate-jsx-notifications
hmalik88 9de4276
fix storybook
hmalik88 a650335
Merge branch 'develop' into hm/integrate-jsx-notifications
hmalik88 1365918
Merge branch 'develop' into hm/integrate-jsx-notifications
hmalik88 b7f667b
update logic to delete notification on detail view dismount
hmalik88 a81ea0f
Merge branch 'develop' into hm/integrate-jsx-notifications
hmalik88 36d3052
lint fix
hmalik88 0c90660
Merge branch 'main' into hm/integrate-jsx-notifications
hmalik88 e6c83ec
ensure snap notification timeout hook runs on dismount
hmalik88 38ad522
Merge branch 'main' into hm/integrate-jsx-notifications
hmalik88 fbd3ea3
revert footer change
hmalik88 90b20d3
use isExternal to determine if link is external in notification detai…
hmalik88 ad4be8f
remove unnecessary else case
hmalik88 b015142
return snap_notify response
hmalik88 4ec8491
address PR comments
hmalik88 73c82ee
Merge branch 'main' into hm/integrate-jsx-notifications
hmalik88 8dc2243
remove export
hmalik88 84bf09c
prevent modal rendering if not snap notification
hmalik88 caa3a17
Merge branch 'main' into hm/integrate-jsx-notifications
hmalik88 921c0e8
make sure footer link displays warning modal
hmalik88 7f2c175
Merge branch 'main' into hm/integrate-jsx-notifications
hmalik88 e365317
make sure modal and external link is not shown for metamask links in …
hmalik88 461939e
Merge branch 'main' into hm/integrate-jsx-notifications
hmalik88 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -1481,12 +1481,19 @@ export default class MetamaskController extends EventEmitter { | |||||
}, | ||||||
showInAppNotification: { | ||||||
method: (origin, args) => { | ||||||
const { message } = args; | ||||||
const { message, title, footerLink, interfaceId } = args; | ||||||
|
||||||
const detailedView = { | ||||||
title, | ||||||
...(footerLink ? { footerLink } : {}), | ||||||
interfaceId, | ||||||
}; | ||||||
|
||||||
const notification = { | ||||||
data: { | ||||||
message, | ||||||
origin, | ||||||
...(interfaceId ? { detailedView } : {}), | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Do we need to be strict about removing the property here or can we do something like this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above. |
||||||
}, | ||||||
type: TRIGGER_TYPES.SNAP, | ||||||
readDate: null, | ||||||
|
@@ -2923,14 +2930,22 @@ export default class MetamaskController extends EventEmitter { | |||||
origin, | ||||||
args.message, | ||||||
), | ||||||
showInAppNotification: (origin, args) => | ||||||
this.controllerMessenger.call( | ||||||
showInAppNotification: (origin, args) => { | ||||||
const { message, title, footerLink } = args; | ||||||
const notificationArgs = { | ||||||
interfaceId: args.content, | ||||||
message, | ||||||
title, | ||||||
footerLink, | ||||||
}; | ||||||
return this.controllerMessenger.call( | ||||||
'RateLimitController:call', | ||||||
origin, | ||||||
'showInAppNotification', | ||||||
origin, | ||||||
args, | ||||||
), | ||||||
notificationArgs, | ||||||
); | ||||||
}, | ||||||
updateSnapState: this.controllerMessenger.call.bind( | ||||||
this.controllerMessenger, | ||||||
'SnapController:updateSnapState', | ||||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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.
I'd like to keep the object clean without
undefined
. The performance difference of creating the intermediate object is neglible.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.
This is not a performance worry, but it adds additional complexity that makes it harder to reason about the code.
I'm not necessarily against it, but why do we need
undefined
to not be present here?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.
Technically it's not type correct if
footerLink
ordetailedView
is present on the notification object, also I have to do more type-fu to access thedetailedView
property instead of usinghasProperty
as I'm currently doing in some of the components.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.
I agree with Frederik that it would be better to pass
undefined
and do a stricter property check when trying to use itThere 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.
I'll consider this as a nit that we can fix after if we really want. This would just improve code readability :)