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

Settings Dependency + Navigation Finesse #186

Merged
merged 35 commits into from
Aug 22, 2023
Merged

Conversation

stephencelis
Copy link
Member

This PR builds off #185. It's a big one!

Settings Dependency

First, we did a refactor we've been wanting to do for a long time. isowords' domain has held onto non-optional Settings.State and has shared this state through various computed properties. This meant introducing a bunch of awkward domains and transformation to make settings available to various features. This pattern is an extension of the "shared state" case study we originally shipped alongside the Composable Architecture. While the pattern is interesting, it introduces a bunch of boilerplate that is difficult to maintain over time and as a project grows.

Since first exploring this pattern of shared/computed state we have come to the conclusion that shared state should come from a dependency instead, and then features that need access to this state can simply ask the dependency for it. And in this case it means pushing UserSettings into a UserSettingsClient that can be queried, updated, and subscribed to. Moving this state into a dependency means all settings screens can be simple, optional presentations rather than contortions of sharing the same non-optional state.

Navigation Finesse

With settings as a dependency, we were able to remove some extra domains in the app, most notably the GameFeature, which was simply a computed part of state that made settings available to the game feature. The game can now present settings directly, home can present the game directly, and features can more reliably request dismissal rather than rely on delegates to do the job manually.

@stephencelis stephencelis merged commit 5a4546a into modernize Aug 22, 2023
2 checks passed
@stephencelis stephencelis deleted the settings-dependency branch August 22, 2023 15:37
stephencelis added a commit that referenced this pull request Aug 23, 2023
* Use built-in Reducer.onChange

* wip

* wip

* update stats

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* fix

* wip

* Settings Dependency + Navigation Finesse (#186)

* 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 <mbrandonw@hey.com>

---------

Co-authored-by: Brandon Williams <mbrandonw@hey.com>
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.

2 participants