Skip to content
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
merged 109 commits into from
Dec 6, 2024
Merged

Conversation

hmalik88
Copy link
Contributor

@hmalik88 hmalik88 commented Sep 25, 2024

Description

Adds an expanded view for snaps notifications, using JSX content returned from the snap to populate the expanded view.

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? Allow for richer snaps notifications
  2. What is the improvement/solution? Add expanded view for snaps, allowing a snap to return jsx content in the expanded view.

Screenshots/Recordings

After

Screen.Recording.2024-11-29.at.4.45.06.AM.mov

Manual testing steps

  1. Pull down https://github.com/hmalik88/swiss-knife-snap and run yarn start to serve the snap and site.
  2. Build this branch using yarn start:flask
  3. Install the snap from the local host site.
  4. Trigger a notification with the "trigger a JSX notification" button and observe the results.

Pre-merge author checklist

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.

Copy link
Contributor

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.

@github-actions github-actions bot added the team-snaps-platform Snaps Platform team label Sep 25, 2024
@hmalik88 hmalik88 changed the base branch from mrtenz/bump-snaps-packages to develop September 25, 2024 19:42
Copy link

socket-security bot commented Sep 25, 2024

No dependency changes detected. Learn more about Socket for GitHub ↗︎

👍 No dependency changes detected in pull request

Mrtenz and others added 11 commits September 27, 2024 17:16
## **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? Previously, developers couldn't
navigate to parts of the extension with the existing functionality
provided to them through snaps UI
2. What is the improvement/solution? The Snaps `Link` component now
checks for if the url is a MetaMask url and navigates to the proper path
accordingly.


## **Screenshots/Recordings**

### **After**


## **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/develop/.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/develop/.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.
@hmalik88 hmalik88 changed the base branch from develop to mrtenz/bump-snaps-packages October 8, 2024 03:54
eriknson
eriknson previously approved these changes Dec 5, 2024
Copy link
Member

@eriknson eriknson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Design lgtm!

@metamaskbot
Copy link
Collaborator

Builds ready [b015142]
Page Load Metrics (1765 ± 58 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint15881978177512460
domContentLoaded15771939174511153
load15841966176512058
domInteractive236134116
backgroundConnect86423178
firstReactRender1492242211
getState753121446632
initialActions01000
loadScripts12041568136710550
setupStore681152110
uiStartup177526212105220106
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 215 Bytes (0.00%)
  • ui: 6.79 KiB (0.09%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [caa3a17]
Page Load Metrics (2027 ± 104 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint164323242025213102
domContentLoaded162923081998218105
load163823212027217104
domInteractive236838136
backgroundConnect867282210
firstReactRender149324178
getState943941487034
initialActions00000
loadScripts12441863156718287
setupStore67912157
uiStartup184631032369333160
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 215 Bytes (0.00%)
  • ui: 7.33 KiB (0.09%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [461939e]
Page Load Metrics (2045 ± 139 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint31428281752653313
domContentLoaded161828082022278133
load162928922045290139
domInteractive24108432411
backgroundConnect782292311
firstReactRender169024168
getState1092881423718
initialActions00000
loadScripts120423511577261125
setupStore612921
uiStartup184332392360347167
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 215 Bytes (0.00%)
  • ui: 7.64 KiB (0.10%)
  • common: 0 Bytes (0.00%)

@hmalik88 hmalik88 added this pull request to the merge queue Dec 6, 2024
Merged via the queue into main with commit 8d7f003 Dec 6, 2024
75 checks passed
@hmalik88 hmalik88 deleted the hm/integrate-jsx-notifications branch December 6, 2024 14:59
@github-actions github-actions bot locked and limited conversation to collaborators Dec 6, 2024
@metamaskbot metamaskbot added the release-12.10.0 Issue or pull request that will be included in release 12.10.0 label Dec 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
INVALID-PR-TEMPLATE PR's body doesn't match template release-12.10.0 Issue or pull request that will be included in release 12.10.0 team-snaps-platform Snaps Platform team
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

9 participants