-
Notifications
You must be signed in to change notification settings - Fork 101
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
Remove GenericCallLinkCoordinator, merging it into CallScreen. #3181
Conversation
This will allow for picture in picture on call links when available.
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.
Note to self: The navigation bar and bottom safe area now show up in light mode even though EC links are always in dark mode.
Generated by 🚫 Danger Swift against 56d7982 |
.overlay { | ||
Group { | ||
if let coordinator = rootCoordinator.overlayModule?.coordinator { | ||
coordinator.toPresentable() | ||
.transition(.opacity) | ||
} | ||
} | ||
.animation(.elementDefault, value: rootCoordinator.overlayModule) | ||
} |
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 the navigation triggered by notifications? I imagine this won't dismiss the overlay but just navigate behind it right?
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.
Hmmm, so it doesn't matter in this instance and it is only used when signed out (and we're not supporting PiP there even if there was somewhere to navigate to). But you're right, I need to think about this in the UserSessionFlowCoordinator
🤔
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.
Will follow this up in another PR.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3181 +/- ##
===========================================
- Coverage 77.85% 77.67% -0.19%
===========================================
Files 716 718 +2
Lines 56461 56601 +140
===========================================
+ Hits 43959 43966 +7
- Misses 12502 12635 +133
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View 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.
LGTM, just a suggestion and question about push notifications
Quality Gate passedIssues Measures |
This PR removes the
GenericCallLinkCoordinator
so we can share logic for the 2 types of call. This will allow for the use of PiP when available and so generic calls are now presented with the same overlay.