Skip to content

[IMPLEMENT] Use Terms Modal#5242

Merged
chrisleewilcox merged 57 commits intomainfrom
implement/466-modal-user-terms
Mar 17, 2023
Merged

[IMPLEMENT] Use Terms Modal#5242
chrisleewilcox merged 57 commits intomainfrom
implement/466-modal-user-terms

Conversation

@tommasini
Copy link
Copy Markdown
Contributor

@tommasini tommasini commented Nov 15, 2022 •
edited
Loading

Description
Implementation of the new User Terms logic.

  • The new user will not be able to create a new wallet or import an existing one while the new user while do not accept the Terms and Conditions
  • The existing user will not be able to do anything if do not accept the Terms and Conditions

Screenshots/Recordings
Behaviour with fresh install:
Importing SRP
https://recordit.co/wjQpTaIMds
Create new wallet
https://recordit.co/RmtOhH9gih

With existing users:
https://recordit.co/4h8bTKbUl5

Test Cases
Case 1:

  • Fresh install
  • accept user terms
  • close app
  • open app
  • expected not to appear the user terms again

Case2:

  • Already installed app
  • accept user terms
  • close app
  • open app
  • expected not to appear the user terms again

Case3:
Need to be tested the events on Mix panel
Key: 'Terms of Use'
value1: 'ToU Displayed'
value2: 'ToU Accepted'

Case4:

  • Small screen devices

Issue

Progresses #???

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Nov 15, 2022

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.

sethkfman
sethkfman reviewed Nov 15, 2022
View reviewed changes
@tommasini tommasini marked this pull request as ready for review January 4, 2023 11:39
@tommasini tommasini requested a review from a team as a code owner January 4, 2023 11:39
@tommasini tommasini added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Jan 4, 2023
@tommasini tommasini changed the title [DRAFT][IMPLEMENT] Use Terms Modal [IMPLEMENT] Use Terms Modal Jan 4, 2023
Cal-L
Cal-L reviewed Jan 6, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

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

Left some comments.

@Cal-L
Copy link
Copy Markdown
Contributor

Cal-L commented Jan 6, 2023

It also looks like the implemented UI is slightly different than what is presented in the Finalized section in Figma.

  • The checkbox isn't aligned relative to the agreement description
  • The circle with the down caret doesn't exist in the implementation

I left a few comments in Figma asking for clarification on some pieces. Will keep an eye out for a response. @tommasini Who should we reach out to as the designer for this UI?
Figma for reference - https://www.figma.com/file/j3T1H0Xiuke9NhFJ3yOxBU/Updated-Terms-of-Use?node-id=0%3A1&t=VX7fTz82hEXlbNiO-0

@chrisleewilcox
Copy link
Copy Markdown
Contributor

chrisleewilcox commented Mar 10, 2023

Hi all. Tomas had looped me in on this thread because I am testing the ToU and I have concerns about the accessibility icon at it's current position on the ToU content in mobile app. Currently the accessibility icon is too close to the agree to terms checkbox and if a user mistaps on the accessibility icon they can get confused with the different accessibility content being displayed and have a hard time agreeing to terms.
Can we move the accessibility icon? I'm thinking to the upper right of the ToU content. If so I think this would have to be done on the website side.

@SamuelSalas
E2e appium/5783 tou onboarding new app installs (#5951)
0cfb06b 
* Webdriverio and Detox test scripts for Term of use feature. Test scripts updated to have the term of use steps

* Small changes to fit browserstack

* Remove static-logos.js file changes
@chrisleewilcox
Copy link
Copy Markdown
Contributor

chrisleewilcox commented Mar 13, 2023

@tommasini I am unable to see the ToU events. I can see other events for mobile app but not ToU events. Can you take a look?
https://recordit.co/5fDI0enmj3

@chrisleewilcox chrisleewilcox added QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed and removed QA in Progress QA has started on the feature. labels Mar 13, 2023
@tommasini tommasini added team-mobile-client and removed release-6.2.0 Issue or pull request that will be included in release 6.2.0 labels Mar 14, 2023
@tommasini
Copy link
Copy Markdown
Contributor Author

tommasini commented Mar 14, 2023

@chrisleewilcox It's fixed, the ToU modal was moved to before importing the SRP or create a new wallet screens and right after the Metrics agreed screen, this way news users, if they agree to participate on the metrics, the mix panel events should appear

Cal-L
Cal-L approved these changes Mar 14, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@Cal-L Cal-L left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Code LGTM. @tommasini Could you verify Chris's question about the ToU events? LMK if you need support

@tommasini tommasini added needs-qa Any New Features that needs a full manual QA prior to being added to a release. and removed QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed labels Mar 15, 2023
@tommasini
Copy link
Copy Markdown
Contributor Author

tommasini commented Mar 15, 2023

They are addressed on this PR, the reason why they were not appearing it was that it was a new user and still did not accept to participate in the metrics

@tommasini tommasini added the release-6.3.0 Issue or pull request that will be included in release 6.3.0 label Mar 15, 2023
@SamuelSalas
Copy link
Copy Markdown
Contributor

SamuelSalas commented Mar 16, 2023

Webdriver.IO Browserstack test results:
/wdio/features/Accounts/CreatingWalletAccount.feature

https://app-automate.browserstack.com/builds/8921ccc14ac9135ca38531b8fc30448c28a5e573/sessions/521d7390f4c43a28130de11107f32ed6e3e0dbf3?auth_token=60b1b6a1021fde7802e00eb31d6661ce751543e08f10ccd54179e78d4b1e7f96

/wdio/features/Accounts/ImportingAccount.feature

https://app-automate.browserstack.com/builds/8921ccc14ac9135ca38531b8fc30448c28a5e573/sessions/0538249f59ed6e471203e5fe6dc6374cfc756a63?auth_token=08f7b5c143a11dac3e620766ad1a7b9ae8453ff098e0586fc4dbbeaf6416b5d7

/wdio/features/Onboarding/CreateNewWallet.feature

https://app-automate.browserstack.com/builds/8921ccc14ac9135ca38531b8fc30448c28a5e573/sessions/94d198ecd44e786cbebca558bdceb6fc664b1e8f?auth_token=79b49c5ed157b59418bc8552c2aa1ac70245791b02a65a3a16240c7c699b8178

/wdio/features/Onboarding/ImportWallet.feature

https://app-automate.browserstack.com/builds/8921ccc14ac9135ca38531b8fc30448c28a5e573/sessions/f11b33be46545b268bda4ef17fafdd9564a7fe97?auth_token=6225c8675703479047c579bc2247ec0538291aaa37e162030c51b27018497a1a

/wdio/features/Onboarding/ImportWalletRegression.feature

https://app-automate.browserstack.com/builds/8921ccc14ac9135ca38531b8fc30448c28a5e573/sessions/c505a6675f2ed1cee046e0a07d8b856b109f66b1?auth_token=074accdd04692188d52c7827ef2f8b24ab85f13edc2d22e81b44233dcf36f53b

/wdio/features/Onboarding/TermsOfUse.feature

https://app-automate.browserstack.com/builds/8921ccc14ac9135ca38531b8fc30448c28a5e573/sessions/983f14e98a39a833d93081dc78256c64caee0a56?auth_token=0db9e909a06dea0bb1f1ecb359120fbe24e8404570562a85fc9042ad734e22d7

/wdio/features/Performance/AppLaunchTime.feature

https://app-automate.browserstack.com/builds/8921ccc14ac9135ca38531b8fc30448c28a5e573/sessions/bd21a47bd0d37590b1d7a86dccf8272df9e284b3?auth_token=b56a04ef64aa0f4b9a5e9789ea63f49e59e64cb68cb085c11a2049d389d2d96f

/wdio/features/AddressFlow.feature -FAILED

https://app-automate.browserstack.com/builds/8921ccc14ac9135ca38531b8fc30448c28a5e573/sessions/dab2b9214e3a086d22693cb1d491cbabe6a7a9c4?auth_token=e307fc8ab0f4d18518404bcde4880cf8ec77a1c55b427302a1b07caf0ecf923a

/wdio/features/ExploringWizard.feature

https://app-automate.browserstack.com/builds/8921ccc14ac9135ca38531b8fc30448c28a5e573/sessions/82c6107fc9010e5edb2228ebe3c49c1521fa2bd9?auth_token=5d64907aeef8250126fe209b90b1ad858bbfde11ce21642b6f2326ca4eab091f

/wdio/features/LockResetWallet.feature

https://app-automate.browserstack.com/builds/8921ccc14ac9135ca38531b8fc30448c28a5e573/sessions/cad6bbb66e42f768de7de0e5b2153556d50b4f87?auth_token=c82d1299a0c53345edcf17bc24e5960b4e59b364d71699c66c08b40fae12502f

/wdio/features/ImportCustomToken.feature - FAILED

https://app-automate.browserstack.com/builds/8921ccc14ac9135ca38531b8fc30448c28a5e573/sessions/dff5cc6163cb422e5e29574749341db528e513c7?auth_token=b9d8253686a519b8a72cfaeec70f479854ea28ee82f8f3d90959984295d72de1

/wdio/features/NetworkFlow.feature

https://app-automate.browserstack.com/builds/8921ccc14ac9135ca38531b8fc30448c28a5e573/sessions/55375aa04ef4df19ce81b923559e40dfa27e3827?auth_token=ea4acf3f6b62566e9b139115b923a75958173eacb4b65cd5ced4364673cdd6cf

/wdio/features/RequestTokenFlow.feature - FAILED

https://app-automate.browserstack.com/builds/8921ccc14ac9135ca38531b8fc30448c28a5e573/sessions/f8a137cd9a02ba2193eda3b83979c4b2834244a4?auth_token=7b0959e392fc12b47c39c6712c646ca4ce88019771126bd3fa5355919333c88c

/wdio/features/SecurityPrivacyRememberMe.feature

https://app-automate.browserstack.com/builds/8921ccc14ac9135ca38531b8fc30448c28a5e573/sessions/0a3f6b8a0d732e514589a839cd89d4fa84c34414?auth_token=69da264a969521f146005f481532e08e63a2d5f2d7b3c5e4aa205692614116e2

/wdio/features/SendToken.feature

https://app-automate.browserstack.com/builds/8921ccc14ac9135ca38531b8fc30448c28a5e573/sessions/b40047a26355496f1faa64e0f3920eefeb80a5c2?auth_token=be1b837cc2d560066000120cd29c2ff50a91cfb2cc3fb42d42ed32152069ad5a

/wdio/features/Onboarding/OnboardingCarousel.feature

https://app-automate.browserstack.com/builds/8921ccc14ac9135ca38531b8fc30448c28a5e573/sessions/c1283403ff6c2c0ad81e7652c8b3f1292831311a?auth_token=5f5275f58e2601d72999b3e66a3f62e6837c32e70a1b5b0cb6c1a154cb807aa9

@SamuelSalas
Copy link
Copy Markdown
Contributor

SamuelSalas commented Mar 16, 2023

Detox test results:
Test Suites: 4 failed, 9 passed, 13 total
Tests: 16 failed, 120 passed, 136 total

Failed:
e2e/specs/confirmations/send-eth-flow.spec.js
e2e/specs/confirmations/advanced-gas-fees.spec.js
e2e/specs/wallet-tests.spec.js
e2e/specs/deeplinks.spec.js

@SamuelSalas
Solve lint issues
9efea64
@SamuelSalas
Merge branch 'main' into implement/466-modal-user-terms
8ddffc4 
 Please enter a commit message to explain why this merge is necessary,
chrisleewilcox
chrisleewilcox approved these changes Mar 17, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@chrisleewilcox chrisleewilcox left a comment

Choose a reason for hiding this comment

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

LGTM

@chrisleewilcox chrisleewilcox merged commit bb239f3 into main Mar 17, 2023
@chrisleewilcox chrisleewilcox deleted the implement/466-modal-user-terms branch March 17, 2023 20:27
@github-actions github-actions bot locked and limited conversation to collaborators Mar 17, 2023
@chrisleewilcox chrisleewilcox added QA Passed QA testing has been completed and passed and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Mar 20, 2023
@gauthierpetetin gauthierpetetin added the team-mobile-ux Mobile UX team label Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@chrisleewilcox chrisleewilcox chrisleewilcox approved these changes

@Cal-L Cal-L Cal-L approved these changes

@jpcloureiro jpcloureiro Awaiting requested review from jpcloureiro

+1 more reviewer

@sethkfman sethkfman sethkfman approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@chrisleewilcox chrisleewilcox

@SamuelSalas SamuelSalas

Labels

QA Passed QA testing has been completed and passed release-6.3.0 Issue or pull request that will be included in release 6.3.0 team-mobile-ux Mobile UX team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@tommasini @Cal-L @chrisleewilcox @SamuelSalas @sethkfman @gauthierpetetin @cortisiko