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

Fix for freezes in MenuBarExtra and NavigationStack #158

Merged
merged 5 commits into from
Jan 18, 2024

Conversation

Wouter01
Copy link
Contributor

@Wouter01 Wouter01 commented Jan 18, 2024

This PR changes the internal code of the @Default property wrapper, so it uses StateObject instead of ObservableObject. This fixes #144

As StateObject is not supported in SwiftUI 1.0, an @available has been added. This could be a breaking change for projects with the minimum target set to iOS 13.0 etc.

Instead of creating a new model when the key changes (which is what happened previously), the key will now be changed in the model itself. An extra check is done to make sure that the observers are not restarted too often.

Sources/Defaults/Defaults.swift Outdated Show resolved Hide resolved
Sources/Defaults/SwiftUI.swift Outdated Show resolved Hide resolved

var value: Value {
get { Defaults[key] }
set {
objectWillChange.send()
Copy link
Owner

Choose a reason for hiding this comment

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

I don't see any harm in keeping this? Unless I'm missing something. See: #144 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If that is kept, the menubar will still freeze for some reason. Removing it fixes that, and doesn't seem to affect anything

Copy link
Owner

Choose a reason for hiding this comment

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

@Wouter01 Removing objectWillChange.send() broke animations when using withAnimation, so I will have to add it back unless you can think of a better solution.

Reproduction: Fixture.zip

Copy link
Collaborator

@hank121314 hank121314 left a comment

Choose a reason for hiding this comment

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

Using @StateObject looks good in this case, thanks for the fix!

@sindresorhus sindresorhus merged commit 2dad0e4 into sindresorhus:main Jan 18, 2024
2 checks passed
@sindresorhus
Copy link
Owner

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.

MenuBarExtra freezes in a state loop when using Defaults
3 participants