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
chore: [IOPID-1829] Change button on ToS screen #5762
Conversation
Affected stories
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5762 +/- ##
==========================================
+ Coverage 48.42% 48.86% +0.43%
==========================================
Files 1488 1605 +117
Lines 31617 32245 +628
Branches 7669 7809 +140
==========================================
+ Hits 15311 15755 +444
- Misses 16238 16432 +194
+ Partials 68 58 -10
... and 232 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see a lot of "blank" space on the bottom of the screen. Did we wrap the footer with buttons inside a safe area? When we use footerWithButtons
we don't need the safe area bottom space handling. What do you think? Can we do a double check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about splitting the two "bugs", UI regression on OnboardingTosScreen
and Email trimming, in two separate PRs? What do you think? For the latter bug (Email trimming), while addressing it I would take the time to write some test on EmailInsertScreen
to be resilient on all the use cases it handles. What do you think?
I've made both changes, and add new screenshot into the description, let me know if everything is ok! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Short description
Change button on ToS screen (use FooterWithButtons)
E2E
Tip
List of changes proposed in this pull request
iOS
Android
How to test
run the app as first onboarding
NB: to simulate error on tos screen need to change this value