Closed cfilipov closed 6 years ago
Hi Cristian. Thank you for the feedback and your interest in this project.
You outlined two quite independent points (view controller support and and "marker override"). Could you split one of those off into a separate ticket so we can track them independently?
Edited this issue to focus only on overriding markers. Moved the discussion on adding view controller support over to #7.
Currently my fork has all three changes I suggested mixed together. I'd be happy to split those out into separate pull requests but I'm trying to gauge your interest on the topics first before spending time splitting up into separate commits.
You're trying to style a multistate UI component for the current state only. It appears to be the only solution, because you assume that applying a state-specific style is a destructive operation, it destroys the style applied for the previous state.
Trying to handle a multistate UI component styling this way will raise more questions then it will solve. How do you ensure that when you're applying a style for the current state there will be left no traces of the style applied for the previous state? Do you assume that all the state-specific styles should operate on the same set of properties? This cannot be statically enforced [in a way that you would like], unfortunately. Do you introduce a style de-application operation? But to which state should we unroll the style then? Should we maintain a stack of appearance-related properties snapshots? It's not going to be cheap, but also it's not very extensible.
Instead, make the style static, make it cover all the states. We can see some examples of a similar approach in UIKit: for a button you define its appearance for different states in one go, without dynamic amendments. If you create a new component with custom states, just allow upfront styling for all the states. It's going to make your code more linear and reusable, without all the hassle of dynamic appearance amending. If it means you have to introduce a custom subclass where previously you used a standard component, this is the right thing to do.
I hope it makes sense to you and you can apply this approach in your projects.
How do you ensure that when you're applying a style for the current state there will be left no traces of the style applied for the previous state?
It isn't the goal to remove all traces of the previous style. The default style has already been applied and now we want to override the parts that the new style defines. This is similar to CSS classes are used on the web. When applying .styled(SelectedStyle.self)
to a view SelectedStyle
might perhaps only set the background color. If we then apply .styled(DisabledStyle.self)
I don't expect that to undo SelectedStyle
.
As for undoing the override, you could just call .styled()
without an override which would apply the style based on the stylesheet rules without the override.
It isn't the goal to remove all traces of the previous style.
Then we'll end up with unpredictable final style. This is different from CSS.
The default style has already been applied and now we want to override the parts that the new style defines.
When we apply a few styles consequently, we have no idea what set of properties has been set multiple times (overridden) and what set of properties has been set once (used by a single style), so the final combination is more or less unpredictable.
This is similar to CSS classes are used on the web.
CSS always allows you to remove a style, thus rolling back to the appearance defined exclusively by the still active styles or the default values. We don't have anything of this.
When applying
.styled(SelectedStyle.self)
to a viewSelectedStyle
might perhaps only set the background color. If we then apply.styled(DisabledStyle.self)
I don't expect that to undoSelectedStyle
.
Sure, it sounds fine in your example. But let's focus on the selection. Imagine you have applied SelectedStyle
and now you're transiting back to unselected state. What do you do? Removing SelectedStyle
is not supported, so you have to apply UnselectedStyle
. Now imagine it was only about the border color, but now you're adding a specific background color as well. But only to SelectedStyle
, because you forgot and compiler and/or framework doesn't help tracking this. And here you can say "it's a programming error", but I think there could be more sophisticated combinations requiring just not practical attention to style compatibility.
As for undoing the override, you could just call .styled() without an override which would apply the style based on the stylesheet rules without the override.
And still the final style will be unpredictable, because re-applying the default style will only override the properties defined in the default style; it will not reset the properties set by other styles to their default values.
I don't think such unpredictable behavior could be beneficial. Also, if you adopt the approach of the static styling for multistate UI components, you don't need anything like this at all, thus eliminating the very context where the problem arose.
I see your point, and yes this is a limitation that you cannot unapply a style in the way CSS would do. Perhaps using the style override to apply styles dynamically was a bad example. One more use is to apply styles to an instance without having to associate all instances of that class with a style.
protocol RedBackground {}
...
let stylesheet = StyleSheet(styles: [
Style<RedBackground, UIView> { $0.backgroundColor = .red }
])
...
// This is a normal UIView
let view1 = UIView()
// Only this instance is styled RedBackground
let view2 = UIView(styled: RedBackground.self)
Alternately you could also just define a new subclass just for that instance that conforms to RedBackground
, but if this class is only ever used in one place I tend to prefer the former. The styled()
variant is useful for views which you did not create and don't own or when you can't rely on the view being completely setup from init(frame:)
.
Perhaps using the style override to apply styles dynamically was a bad example.
Indeed, an attempt to use this framework in a dynamic manner raises more questions and concerns then it can solve (at least when a combination of the automatic application and a manual application is considered). The last time I've seen such an attempt similar issues were discovered. And it kills the "just works" experience.
One more use is to apply styles to an instance without having to associate all instances of that class with a style. Alternately you could also just define a new subclass just for that instance that conforms to
RedBackground
This would take one line of code and create zero uncertainties. This is how it works best.
The
styled()
variant is useful for views which you did not create and don't own ...
This is fully supported by the current design:
extension XYThirdPartyLabel: BodyFontStyle, MultilineLabelStyle {}
... or when you can't rely on the view being completely setup from
init(frame:)
.
You can only override a constructor in a subclass, but then a standard approach is going to be much more time saving.
Thank you for the proposition, it triggered an interesting discussion and showed a few action points. After all, I don't see how this feature can land in the framework without making its use cases less clear and straightforward.
Action points:
Would you be interested in a pull request that adds the following: ability to override style after it has already been applied by specifying an override marker. See this commit for example implementation. As an example, perhaps you want to change the style of a particular view when it's in the selected state. Instead of manually updating the properties you can use a marker to override.
In the above code, even if the view conforms to some other marker, this method explicitly uses the passed in marker for styling.