TCA Navigation #185
Merged
TCA Navigation #185
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
stephencelis
commented
Aug 17, 2023
| @@ -1339,7 +1339,7 @@ | |||
| GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE; | |||
| GCC_WARN_UNUSED_FUNCTION = YES; | |||
| GCC_WARN_UNUSED_VARIABLE = YES; | |||
| IPHONEOS_DEPLOYMENT_TARGET = 15.0; | |||
| IPHONEOS_DEPLOYMENT_TARGET = 16.0; | |||
There was a problem hiding this comment.
To use navigationDestination I've bumped the deployment target. Up for alternate ideas, though.
There was a problem hiding this comment.
nah iOS 16 sounds good to me!
stephencelis
commented
Aug 17, 2023
| group.addTask { | ||
| for await event in self.userNotifications.delegate() { | ||
| await send(.userNotifications(event)) | ||
| public var body: some ReducerOf<Self> { |
There was a problem hiding this comment.
While some of these reduce(into:) needed to migrate to the body style to compose with ifLet, I've migrated all of them for consistency.
stephencelis
commented
Aug 17, 2023
| @@ -5,20 +5,17 @@ public struct BottomMenuState<Action> { | |||
| public var buttons: [Button] | |||
| public var footerButton: Button? | |||
| public var message: TextState? | |||
| public var onDismiss: MenuAction? | |||
There was a problem hiding this comment.
We can now leverage PresentationAction.dismiss for the bottom menu instead of threading in an explicit action.
stephencelis
commented
Aug 18, 2023
Sources/GameCore/GameCore.swift
Outdated
Comment on lines
20
to
52
| public struct Destination: Reducer { | ||
| public enum State: Equatable { | ||
| case alert(AlertState<Action.Alert>) | ||
| case bottomMenu(BottomMenuState<Action.BottomMenu>) | ||
| case gameOver(GameOver.State) | ||
| case upgradeInterstitial(UpgradeInterstitial.State = .init()) | ||
| } | ||
| public enum Action: Equatable { | ||
| case alert(Alert) | ||
| case bottomMenu(BottomMenu) | ||
| case gameOver(GameOver.Action) | ||
| case upgradeInterstitial(UpgradeInterstitial.Action) | ||
|
|
||
| public enum Alert: Equatable { | ||
| case forfeitButtonTapped | ||
| } | ||
| public enum BottomMenu: Equatable { | ||
| case confirmRemoveCube(LatticePoint) | ||
| case endGameButtonTapped | ||
| case exitButtonTapped | ||
| case forfeitGameButtonTapped | ||
| case settingsButtonTapped | ||
| } | ||
| } | ||
| public var body: some ReducerOf<Self> { | ||
| Scope(state: /State.gameOver, action: /Action.gameOver) { | ||
| GameOver() | ||
| } | ||
| Scope(state: /State.upgradeInterstitial, action: /Action.upgradeInterstitial) { | ||
| UpgradeInterstitial() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Game and Home have some of the more interesting navigation refactors. Here we're able to house 4 destinations.
* Beginnings of the UserSettingsClient. * extracted UserSettingsClient to its own library * Decoupled haptics settings from some features. * Stopped threading isAnimationReduced state through multple features and fixed tests. * Stopped threading isAnimationReduced to leaderboard. * Moved daily challenge settings into UserSettings. * Moved enableNotifications to UserSettings. * Moved developer settings to app storage. * Game feature has its own settings, haven't deleted home settings yet. * Removed GameFeature * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * wip * Fixed some runtime warnings. * wip * Flatten nag banner domains, remove old settings from home * wip * wip --------- Co-authored-by: Brandon Williams <[email protected]>
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 upgrades many of our domains to use TCA's navigation APIs. It's not an exhaustive list, since some contenders are not yet easy to transition due to our shared settings state. Once we refactor settings to go through a dependency it should be easier to migrate those.
I've also included a few other modernizations, like using TCA's built-in
onChangeoperator over the one we previously built from scratch for this app.