feat: Add new privacy policy toast by jonybur · Pull Request #23838 · MetaMask/metamask-extension

jonybur · 2024-04-03T13:44:41Z

Description

Changes:
Adds a new privacy policy banner that will show up after June 4, 2024.
If the user closes it manually, the privacy policy banner will not show up again.
If the user does not close the toast manually, the privacy policy banner will cease to show up after a day of being shown for the first time.
Changes the Survey banner so that we can display multiple banners at the same time without overlapping each other.

Considerations:

Make sure this doesn't show up during onboarding
This toast will show up even after years of the privacy policy being changed. We should set up a deadline for that or eventually revert this PR.
Check if copy is correct.

Related issues

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

Manual testing steps

Go to the home page
Check if there's a new privacy banner there (date should be edited in the code so that the privacy policy shows up earlier than May 6, 2024)
When closing it, it shouldn't show up again
If not closing it manually, the toast should not show up again after 1 day of being shown for the first time. This data is saved in localStorage so editing the timestamp there should be enough to test this feature out.

Screenshots/Recordings

Before

After

Pre-merge author checklist

I’ve followed MetaMask Coding Standards.
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 format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). 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.

github-actions · 2024-04-03T13:44:53Z

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.

darkwing

Added some guidance here. Nice start!

ui/pages/home/home.component.js

… fix-2319

…o fix-2319

… fix-2319

…o fix-2319

darkwing · 2024-04-08T18:27:11Z

Kicked CI because test failures look intermittent. This looks awesome!

NidhiKJha

Really great start! Left some nits.

NidhiKJha · 2024-04-09T06:32:51Z

ui/pages/home/index.scss

-  &__survey-banner {
-    position: absolute;
+  &__overlay-banners {
+    z-index: 9999;


Why are we using this z-index?

I found that the current implementation was being shown under other elements in the DOM.

The toasts should always be on top of everything, so the best way to ensure this is with z-index, as there are currently other elements using z-index as well.

NidhiKJha · 2024-04-09T06:35:04Z

ui/pages/home/home.component.js

+          />
+        )}
+        {showPrivacyPolicyToast && isPrivacyToastRecent && (
+          <BannerBase


Question: Why are we using BannerBase and not Toast component here?

The implementation mirrors the one of the Survey banner, I haven't seen the Toast component before, but I think I can change the implementation of both banners to use Toast

ui/pages/home/home.component.js

NidhiKJha · 2024-04-09T06:38:41Z

To get CI tests to pass, you might need to update the error spec file and fixtures as you have added the state in the app-state directly. Use the command yarn build:test and once the build is generated. You need to run yarn test:e2e:single test/e2e/tests/errors.spec.js --browser chrome --update-snapshot

salimtb · 2024-05-06T19:34:32Z

ui/pages/routes/routes.component.js

+    const isPrivacyToastNotShown = !newPrivacyPolicyToastShownDate;
+
+    return (
+      <ToastContainer>


I suggest moving this function into its own component and then importing it. This approach offers several benefits: it makes the function easier to test on its own (unit testing), enables us to create a dedicated Storybook entry for it, and facilitates its reuse in future projects or components if needed, and reduce the size of the routes.component.js file

… fix-2319

metamaskbot · 2024-05-08T23:35:45Z

Builds ready [2b58957]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8
- content-script: 0
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1569 ± 657 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	69	135	94	19	9
		domContentLoaded	10	33	16	7	3
		load	56	3330	1569	1369	657
		domInteractive	10	33	16	7	3

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 450 Bytes (0.01%)
ui: 1.58 KiB (0.02%)
common: 1.25 KiB (0.02%)

metamaskbot · 2024-05-10T19:44:04Z

Builds ready [23a6b09]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8
- content-script: 0
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1262 ± 729 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	72	186	103	31	15
		domContentLoaded	9	54	19	13	6
		load	59	4739	1262	1518	729
		domInteractive	9	54	19	14	6

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 450 Bytes (0.01%)
ui: 1.58 KiB (0.02%)
common: 1.25 KiB (0.02%)

metamaskbot · 2024-05-10T20:26:19Z

Builds ready [977936a]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8
- content-script: 0
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (756 ± 576 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	55	149	90	23	11
		domContentLoaded	7	68	16	13	6
		load	44	3351	756	1201	576
		domInteractive	7	68	16	13	6

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 450 Bytes (0.01%)
ui: 1.58 KiB (0.02%)
common: 1.25 KiB (0.02%)

codecov · 2024-05-10T20:26:59Z

Codecov Report

Attention: Patch coverage is 50.60241% with 41 lines in your changes are missing coverage. Please review.

Project coverage is 67.42%. Comparing base (cfcbd1e) to head (977936a).

Files	Patch %	Lines
ui/pages/routes/routes.component.js	51.85%	26 Missing ⚠️
app/scripts/metamask-controller.js	0.00%	5 Missing ⚠️
ui/store/actions.ts	33.33%	4 Missing ⚠️
app/scripts/controllers/app-state.js	0.00%	3 Missing ⚠️
ui/pages/routes/routes.container.js	0.00%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #23838      +/-   ##
===========================================
- Coverage    67.48%   67.42%   -0.05%     
===========================================
  Files         1288     1289       +1     
  Lines        50153    50236      +83     
  Branches     13023    13023              
===========================================
+ Hits         33842    33871      +29     
- Misses       16311    16365      +54

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

darkwing · 2024-05-10T20:36:36Z

@jonybur I can confirm I no longer see the strange behavior of the toast not going away. 👍

metamaskbot · 2024-05-22T22:08:25Z

Missing release label release-11.16.0 on PR. Adding release label release-11.16.0 on PR and removing other release labels(release-11.18.0), as PR was cherry-picked in branch 11.16.0.

Jonathan Bursztyn and others added 2 commits April 3, 2024 14:42

Add multiple banners support, add privacy policy toast & logic

36e0b85

Merge branch 'develop' into fix-2319

73bc3d7

Merge branch 'develop' into fix-2319

3955353

darkwing suggested changes Apr 3, 2024

View reviewed changes

ui/pages/home/home.component.js Outdated Show resolved Hide resolved

ui/pages/home/home.component.js Outdated Show resolved Hide resolved

ui/pages/home/home.component.js Outdated Show resolved Hide resolved

ui/pages/home/home.component.js Outdated Show resolved Hide resolved

Jonathan Bursztyn added 8 commits April 4, 2024 14:29

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

dddd893

… fix-2319

Refactor localStorage

96652eb

Merge branch 'fix-2319' of github.com:MetaMask/metamask-extension int…

33eb6b3

…o fix-2319

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

962a15e

… fix-2319

Refactor to work like the survey toast

ad2e614

Style fix

329f082

Fix theme

78c0397

Fix privacy policy date

3f28370

jonybur added team-core-extension-ux Core Extension UX team needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. labels Apr 8, 2024

Jonathan Bursztyn and others added 6 commits April 8, 2024 13:41

Revert unrelated change, remove comment

cc88426

Add tests

d1eafe1

Merge branch 'develop' into fix-2319

23df13b

Fix lint, improve testing

c0d31ae

Merge branch 'fix-2319' of github.com:MetaMask/metamask-extension int…

05c3d62

…o fix-2319

Add JSDoc

e9fe4eb

jonybur marked this pull request as ready for review April 8, 2024 14:07

jonybur requested a review from a team as a code owner April 8, 2024 14:07

jonybur requested a review from darkwing April 8, 2024 14:08

Jonathan Bursztyn and others added 2 commits April 8, 2024 15:30

Update copy

8bf7abf

Merge branch 'develop' into fix-2319

0b01c7a

NidhiKJha requested changes Apr 9, 2024

View reviewed changes

Fix themes

c4895b7

jonybur requested a review from darkwing May 3, 2024 19:21

salimtb reviewed May 6, 2024

View reviewed changes

Jonathan Bursztyn added 2 commits May 7, 2024 14:49

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

8dc4b0a

… fix-2319

Remove unused var

33230ef

darkwing assigned jonybur May 7, 2024

Jony Bursztyn and others added 3 commits May 7, 2024 17:53

Merge branch 'develop' into fix-2319

edb0589

Merge branch 'develop' into fix-2319

d39693f

Merge branch 'develop' into fix-2319

b3086d6

darkwing approved these changes May 7, 2024

View reviewed changes

Jony Bursztyn and others added 2 commits May 8, 2024 14:51

Merge branch 'develop' into fix-2319

b211af1

Merge branch 'develop' into fix-2319

2b58957

Merge branch 'develop' into fix-2319

d6bd412

NidhiKJha approved these changes May 9, 2024

View reviewed changes

Jony Bursztyn and others added 4 commits May 9, 2024 14:54

Merge branch 'develop' into fix-2319

2e2abe4

Merge branch 'develop' into fix-2319

079f071

Merge branch 'develop' into fix-2319

23a6b09

Merge branch 'develop' into fix-2319

977936a

jonybur merged commit 99b9480 into develop May 14, 2024

jonybur deleted the fix-2319 branch May 14, 2024 14:37

github-actions bot locked and limited conversation to collaborators May 14, 2024

metamaskbot added the release-11.18.0 label May 14, 2024

hesterbruikman added the area-privacy label May 17, 2024

metamaskbot added release-11.16.0 Issue or pull request that will be included in release 11.16.0 and removed release-11.18.0 labels May 22, 2024

Conversation

jonybur commented Apr 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions bot commented Apr 3, 2024

Uh oh!

darkwing left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

darkwing commented Apr 8, 2024

Uh oh!

NidhiKJha left a comment

Choose a reason for hiding this comment

Uh oh!

NidhiKJha Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

jonybur Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

NidhiKJha Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

jonybur Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

NidhiKJha commented Apr 9, 2024

Uh oh!

salimtb May 6, 2024

Choose a reason for hiding this comment

Uh oh!

metamaskbot commented May 8, 2024

Uh oh!

metamaskbot commented May 10, 2024

Uh oh!

metamaskbot commented May 10, 2024

Uh oh!

codecov bot commented May 10, 2024

Codecov Report

Uh oh!

darkwing commented May 10, 2024

Uh oh!

metamaskbot commented May 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jonybur commented Apr 3, 2024 •

edited

Loading