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.

Sign up for GitHub

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

Jump to bottom

feat: snaps dynamic UI #12429

Draft
wants to merge 47 commits into
base: main
Choose a base branch
from
Draft

feat: snaps dynamic UI #12429

wants to merge 47 commits into from
+2,718 −46

Conversation

Daniel-Cross
Copy link
Contributor

@Daniel-Cross Daniel-Cross commented Nov 26, 2024 •
edited
Loading

Description

This PR integrates the Snaps Dynamic UI controller and components to allow different dialog prompts to show on the screen.

Related issues

Fixes:

Manual testing steps

  1. Make a Flask build
  2. Go to: https://metamask.github.io/snaps/test-snaps/latest/ in the in app browser
  3. Connect the Snaps Dialog snap
  4. Try to launch the different snaps prompts.

Screenshots/Recordings

Before

After

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.

@github-actions GitHub Actions
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.

@Daniel-Cross Daniel-Cross self-assigned this Dec 16, 2024
@Daniel-Cross Daniel-Cross added team-accounts team-snaps-platform Snaps Platform team Run Smoke E2E Triggers smoke e2e on Bitrise labels Dec 16, 2024
@Daniel-Cross Daniel-Cross changed the title feature: snaps dynamic UI feat: snaps dynamic UI Dec 16, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 16, 2024 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 04876a5
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/d474e6c3-86f1-4359-84e2-7c0517478204

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

FrederikBolding
FrederikBolding reviewed Dec 19, 2024
View reviewed changes
app/components/UI/FormTextField/index.tsx
import { FormTextFieldProps, FormTextFieldSize } from './form-text-field.types';
import Text from '../../../component-library/components/Texts/Text';

export const FormTextField = ({
Copy link
Member

Choose a reason for hiding this comment

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

Did this not exist on mobile? Is there an alternative we should be using instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't really handle 'forms' on mobile. It's just basic text inputs that need to be wrapped in divs so they can be styled.

FrederikBolding
FrederikBolding reviewed Dec 19, 2024
View reviewed changes
app/components/Approvals/Snaps/SnapUIRenderer/SnapUIRenderer.tsx
}

// Component that maps Snaps UI JSON format to MetaMask Template Renderer format
const SnapUIRendererComponent = ({
Copy link
Member

Choose a reason for hiding this comment

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

We should make sure that all of the CSS rules from the extension are applied to the renderer content: https://github.com/MetaMask/metamask-extension/blob/main/ui/components/app/snaps/snap-ui-renderer/index.scss

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really sure how to implement this but we can discuss it.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 15.75092% with 230 lines in your changes missing coverage. Please review.

Project coverage is 58.78%. Comparing base (9d6908b) to head (4fecdd1).
Report is 35 commits behind head on main.

Files with missing lines Patch % Lines
...vals/Snaps/SnapUIRenderer/SnapInterfaceContext.tsx 4.08% 47 Missing ⚠️
...s/Snaps/SnapDialogApproval/SnapsDialogApproval.tsx 2.85% 34 Missing ⚠️
...components/Approvals/Snaps/SnapUIRenderer/utils.ts 12.12% 29 Missing ⚠️
app/components/UI/Snaps/SnapUIInput/Index.tsx 5.88% 16 Missing ⚠️
app/components/UI/FormTextField/index.tsx 13.33% 13 Missing ⚠️
.../Approvals/Snaps/SnapUIRenderer/SnapUIRenderer.tsx 20.00% 12 Missing ⚠️
...ovals/Snaps/SnapUIRenderer/components/container.ts 9.09% 10 Missing ⚠️
app/components/UI/Box/index.tsx 23.07% 10 Missing ⚠️
...s/Approvals/Snaps/SnapUIRenderer/components/box.ts 10.00% 9 Missing ⚠️
...pprovals/Snaps/SnapUIRenderer/components/footer.ts 11.11% 8 Missing ⚠️
... and 14 more
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #12429      +/-   ##
==========================================
- Coverage   58.85%   58.78%   -0.07%     
==========================================
  Files        1851     1879      +28     
  Lines       41739    42089     +350     
  Branches     5397     5485      +88     
==========================================
+ Hits        24566    24743     +177     
- Misses      15502    15672     +170     
- Partials     1671     1674       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Daniel-Cross Daniel-Cross added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Jan 1, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 1, 2025 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 125e2da
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/2ec35c5c-18c6-42b5-88e5-b859e24e5328

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@socket-security Socket Security
Copy link

socket-security bot commented Jan 7, 2025 •
edited
Loading

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/[email protected] None 0 287 kB mdevils

🚮 Removed packages: npm/[email protected]

View full report↗︎

@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Jan 9, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
10.2% Coverage on New Code (required ≥ 60%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FrederikBolding FrederikBolding FrederikBolding left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@Daniel-Cross Daniel-Cross

Labels
Run Smoke E2E Triggers smoke e2e on Bitrise team-accounts team-mobile-platform team-snaps-platform Snaps Platform team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Daniel-Cross @codecov-commenter @FrederikBolding @metamaskbot @owencraston