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

Authentication Layouts #994

Merged
merged 7 commits into from
May 31, 2023
Merged

Authentication Layouts #994

merged 7 commits into from
May 31, 2023

Conversation

pixlwave
Copy link
Contributor

This PR makes the following changes:

  • Remove a lot of old unused code from the onboarding screen as we don't have a carousel anymore.
  • Adds a FullscreenDialog container used for layouts of screens that have content with buttons at the bottom.
    • This layout no longer scrolls when the content fits on the screen.
    • When the content doesn't fit on the screen, the buttons now move out the way instead of being an overlay so the entire screen scrolls.
  • Uses FullscreenDialog for the analytics prompt and server confirmation screen.
  • Fix a bug in the analytics prompt where the 2 points with HTML weren't scaling due to having fixed fonts set by DTCoreText.
  • Bumps the minimum OS to 16.4 for the scroll fix.

The original plan was to also use the new layout in the onboarding screen however, it turns out it didn't really make sense as that one doesn't need to scroll with accessibility text sizes, instead the logo can simply get smaller.

Screen.Recording.2023-05-31.at.8.54.04.am.mov

@pixlwave pixlwave requested a review from a team as a code owner May 31, 2023 11:51
@pixlwave pixlwave requested review from Velin92 and removed request for a team May 31, 2023 11:51
@github-actions
Copy link

github-actions bot commented May 31, 2023

Warnings
⚠️ Some of the commits are missing ticket numbers. Please consider squashing all commits that don't have a tracking number.
⚠️ You seem to have made changes to views. Please consider adding screenshots.

Generated by 🚫 Danger Swift against a6c18ae

@codecov
Copy link

codecov bot commented May 31, 2023

Codecov Report

Patch coverage: 5.06% and project coverage change: -0.35 ⚠️

Comparison is base (87075ee) 46.83% compared to head (018c46c) 46.49%.

❗ Current head 018c46c differs from pull request most recent head a6c18ae. Consider uploading reports for the commit a6c18ae to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #994      +/-   ##
===========================================
- Coverage    46.83%   46.49%   -0.35%     
===========================================
  Files          349      349              
  Lines        22649    22634      -15     
  Branches     12332    12272      -60     
===========================================
- Hits         10608    10523      -85     
- Misses       11740    11816      +76     
+ Partials       301      295       -6     
Flag Coverage Δ
unittests 23.81% <5.06%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ources/Other/SwiftUI/Layout/FullscreenDialog.swift 0.00% <0.00%> (ø)
...ticsPromptScreen/AnalyticsPromptScreenModels.swift 84.61% <0.00%> (ø)
...yticsPromptScreen/View/AnalyticsPromptScreen.swift 82.08% <0.00%> (-12.17%) ⬇️
...irmationScreen/View/ServerConfirmationScreen.swift 68.33% <0.00%> (-12.27%) ⬇️
...reens/OnboardingScreen/View/OnboardingScreen.swift 26.58% <0.00%> (-17.64%) ⬇️
...tX/Sources/Other/Extensions/AttributedString.swift 85.71% <88.88%> (+1.09%) ⬆️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@Velin92 Velin92 left a comment

Choose a reason for hiding this comment

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

LGTM, just one small thing, and maybe worth asking to the design team for the scrolling behaviour of the view, because I feel the overlapping with the bottom indicator is a bit too evident for buttons maybe?

@sonarcloud
Copy link

sonarcloud bot commented May 31, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@pixlwave pixlwave merged commit dcc4ea5 into develop May 31, 2023
6 checks passed
@pixlwave pixlwave deleted the doug/auth-layouts branch May 31, 2023 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants