Skip to content

Conversation

@MiaKoring
Copy link
Contributor

Like the title says, I changed the protocol to be publicly available, allowing for more third party view update triggering property wrappers, i.e. @query or @AppStorage

Also I renamed it do ObservableProperty, decoupling it verbally from @State.

Doing this involved declaring the protocol-required properties and functions as public too and replacing StateProperty with ObservableProperty wherever it gets used.

0 sideeffects expected, as there were no logic changes, aside from making some functions/values more accessible.

Copy link
Owner

@stackotter stackotter left a comment

Choose a reason for hiding this comment

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

Just requested a few small changes

/// Declaring conformance to ObservableProperty is required for
/// SwiftCrossUI to automatically register an observer on the
/// conforming object's Publisher.
public protocol ObservableProperty {
Copy link
Owner

Choose a reason for hiding this comment

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

I think that ObservableProperty should inherit from DynamicProperty, because it doesn't make sense to implement an ObservableProperty whose state doesn't persist across view recalculations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, I didn’t think of it, just changed the name on what it used before

moved ObservableProperty to separate file, updated documentation comment
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