[WIP DNR] Allow wrapping screens to guarantee a described view controller #159
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kyleve
force-pushed
the
kve/container-screen
branch
from
0fe89cc
to
e93a6ca
Compare
kyleve
force-pushed
the
kve/container-screen
branch
from
e93a6ca
to
10cec95
Compare
* origin/main: Changed expectPublisher/expectTask to be expect Updatd development podspec Added Concurrency Testing
kyleve
commented
Mar 25, 2023
| /// if isLoading { | ||
| /// return LoadingScreen(with: ...) | ||
| /// } else if isEmpty { | ||
| /// return ContentScreen(with ...) |
There was a problem hiding this comment.
returns are backwards
kyleve
commented
Mar 25, 2023
| let content = content() | ||
|
|
||
| if let content = content as? Self { | ||
| self = content |
There was a problem hiding this comment.
Oops, this drops the transition
kyleve
commented
Oct 13, 2023
|
|
||
| guard newPreferredContentSize != preferredContentSize else { return } | ||
|
|
||
| preferredContentSize = newPreferredContentSize | ||
| } | ||
|
|
||
| private func currentViewControllerChanged() { |
There was a problem hiding this comment.
- I think this needs update for iOS 16 and 17
kyleve
commented
Oct 13, 2023
|
|
||
| #if canImport(UIKit) | ||
|
|
||
| /// Used by `AnyContentScreen` and `DescribedViewController` to the backing view |
There was a problem hiding this comment.
- Some words missing here
kyleve
commented
Oct 13, 2023
There was a problem hiding this comment.
- Add: Tests for animated and non-animated transitions
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces a wrapper screen that transitions its content when the underlying VC type changes; which was lost as a "standard" thing when we removed
DescribedViewControllerusage from our apps – since it effectively doubled the VC hierarchy depth. Some folks still want this behavior, so giving them a more concrete way to achieve it.AnyContentScreen, which is a screen-based wrapper aroundDescribedViewController. If the containing screen's VC changes, it'll transition per the added transition type...DescribedViewControllerwhen it replaces its content. This in general probably always should have been a thing; since without an animation, things look pretty sudden / abrupt.Checklist