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: display full terms of service during onboarding #29392

Open
wants to merge 42 commits into
base: main
Choose a base branch
from
Open

Conversation

jonybur
Copy link
Contributor

@jonybur jonybur commented Dec 20, 2024

Description

During the onboarding process, the user is currently presented with a simple checkbox to agree to terms:

Screenshot 2024-08-12 at 14 15 34

Legal would prefer we show the full terms of service, requiring the user to scroll the entire terms and agree at the end.

Open in GitHub Codespaces

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3013

Manual testing steps

  1. Start onboarding
  2. See checkbox but without full terms.
  3. Verify full terms are displayed, requiring user to scroll and agree.

Screenshots/Recordings

Before

Screenshot 2024-08-12 at 14 15 34

After

demo.mp4

No response

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.

@jonybur jonybur requested a review from a team as a code owner December 20, 2024 15:28
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.

@jonybur jonybur changed the title feat: add feat: display full terms of service during onboarding Dec 20, 2024
@jonybur jonybur self-assigned this Jan 6, 2025
@metamaskbot
Copy link
Collaborator

Builds ready [5d2206e]
Page Load Metrics (1771 ± 70 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint24220891706361173
domContentLoaded14822076174014670
load14872092177114570
domInteractive26224624723
backgroundConnect1085362311
firstReactRender177630189
getState55917168
initialActions01000
loadScripts10631651131013263
setupStore85525209
uiStartup17162364203116077

@jonybur
Copy link
Contributor Author

jonybur commented Jan 9, 2025

For some reason, this is failing the playwright tests (they are not even being ran).
E2E tests are failing due to a flaky test. They are not failing on lastest commit 5d2206e, but they are on commit 24353cf; which only adds a couple of console logs for debugging purposes.

davibroc and others added 2 commits January 9, 2025 23:54
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

Modify the script to adjust to the new UI for the terms and conditions

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29616?quickstart=1)

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **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/main/.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/main/.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.
@metamaskbot
Copy link
Collaborator

Builds ready [426a732]
Page Load Metrics (1548 ± 48 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1406182215499746
domContentLoaded14001800152410048
load1405182415489948
domInteractive23137513015
backgroundConnect75622157
firstReactRender156530199
getState55214168
initialActions01000
loadScripts1025137911318842
setupStore65116188
uiStartup15852072175311756
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 2.63 KiB (0.03%)
  • common: -66 Bytes (-0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [8dbcf91]
Page Load Metrics (1683 ± 47 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1555204616819847
domContentLoaded15432031166010048
load1557204816839847
domInteractive25105462713
backgroundConnect75220147
firstReactRender1672392311
getState55013147
initialActions01000
loadScripts1129155012548641
setupStore56717199
uiStartup1762224519169747
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 2.63 KiB (0.03%)
  • common: -66 Bytes (-0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [e75aaa3]
Page Load Metrics (1682 ± 65 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14811937168013464
domContentLoaded14531919165513464
load14811946168213565
domInteractive25186453617
backgroundConnect673252010
firstReactRender1575332110
getState45215157
initialActions01000
loadScripts10751432123610952
setupStore65410105
uiStartup16812371192818388
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 2.63 KiB (0.03%)
  • common: -66 Bytes (-0.00%)

@jonybur jonybur requested a review from a team as a code owner January 14, 2025 16:29
@metamaskbot
Copy link
Collaborator

Builds ready [a73cefe]
Page Load Metrics (1682 ± 37 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1565186316878440
domContentLoaded1531183616558340
load1563187216827837
domInteractive25142553919
backgroundConnect772292110
firstReactRender1674352311
getState45514116
initialActions01000
loadScripts1093140412347134
setupStore65213136
uiStartup1745205418988340
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -103.46 KiB (-1.78%)
  • ui: 5.43 KiB (0.07%)
  • common: -33.55 KiB (-0.40%)

@metamaskbot
Copy link
Collaborator

Builds ready [2a4c065]
Page Load Metrics (1734 ± 87 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint51120921664315151
domContentLoaded14832078169717082
load14912097173418187
domInteractive23228564923
backgroundConnect5208374823
firstReactRender15105553416
getState484323216
initialActions01000
loadScripts10661581126314770
setupStore56813157
uiStartup169229012162353170
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 2.67 KiB (0.03%)
  • common: -66 Bytes (-0.00%)

@hesterbruikman hesterbruikman added the DO-NOT-MERGE Pull requests that should not be merged label Jan 28, 2025
@hesterbruikman
Copy link

Adding a Do not merge label because we want to make some improvements to onboarding before introducing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO-NOT-MERGE Pull requests that should not be merged team-wallet-ux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants