cogentcore
Proposal: Editable instead of ReadOnly
Describe the feature
Note: this proposal would result in breaking changes that affect end-user code. We would appreciate feedback on this.
1. Positive properties
Most of our boolean widget properties are positive (ex: Hovered makes more sense than Unhovered, display makes more sense than hide). Our struct tags are all positive (ex: display, edit, set, save, etc). This reduces confusion and inconsistency, particularly surrounding double negatives. However, we fail to follow this rule for three widget states: Disabled, Invisible, and ReadOnly. Therefore, I propose that these be changed to Enabled, Visible, and Editable, respectively. I also propose that this positive property rule be codified in the principles.
2. States and abilities
states.States are transient states induced by user interaction that typically don't last for long (ex: Hovered, LongPressed, etc). abilities.Abilities are properties that specify in what ways a user is able to interact with a widget. We currently follow this de facto distinction in most cases, except for Enabled, Visible, and Editable. All three of them should clearly be abilities, not states: Enabled specifies whether you are able to interact with a widget at all, Visible specifies whether you are able to see a widget, and Editable specifies whether you are able to edit the contents of a widget. Therefore, I propose that these three states be converted to abilities. I also propose that this distinction be better clarified in the documentation for states.States and abilities.Abilities.
3. Inheritance
Enabled, Visible, and Editable are currently not inherited from parent widgets, but I propose that they be. If you have a disabled widget, it is exceedingly unlikely that you will want its child to be enabled. If you have an invisible widget, it is literally impossible for its child to be visible. If you have an uneditable widget, you would not want to be able to edit its child. This inheritance proposal has the benefit of simplifying cases like #1141 / #1143.
Implications
The primary practical public-facing developer implication of this proposal would be that something like this:
core.NewTextField(b).SetReadOnly(true)
Would turn into something like this:
core.NewTextField(b).Styler(func(s *styles.Style) {
s.SetAbilities(false, abilities.Editable)
})
This increase in verbosity is unideal, albeit more consistent with other styling properties. I am open to considering possible ways to reduce this verbosity.
Relevant code
No response
Overall this seems much more "cogent" and worth the updating cost.
It is possible to define single-effect styler functions for high-frequency use-cases, so we could even define SetReadOnly to implement that one-off styler, requiring no code updates?
The Visible case is more problematic however, and is not really a "trait" ability-like property -- it really is a transient, dynamic state that depends on scrolling and tab switching etc. What would be the point of having a widget with a stable "invisible" trait?
For example, a long time ago I used to have an inherited visibility property, and it was problematic to keep it updated properly based on dynamic changes in visibility at higher levels. It isn't great to have to go down through all the children to make everybody visible again, and you also have to then properly account for all the intervening levels of logic about whether something should be visible based on Stack layout, scrolling, etc. i.e., it requires a full Layout pass.
The great thing about the current Invisible logic is that rendering just automatically bails when it hits the first invisible guy, and when that is updated, everything below automatically becomes visible (until you hit the next visibility barrier). The default zero = Visible is kinda nice there too, but presumably it wouldn't be hard to have everyone start with a Visible default and just flip the logic, to avoid the double-negative.
By contrast, Editable and Enabled are sensibly trait like properties, and we definitely DO need to propagate them down the tree because it actively affects how they are rendered and behave. So it makes sense do make those abilities.
OK, we can keep Visible as an uninherited state. Single-effect helper styler functions are a very slippery slope.
In the context of implementing this, we will also further develop the abilities and states docs with more explanations and examples for common abilities and states. We will also consider making some ability settings more binding (such as focusable off for text fields and clickable/activatable off for double/triple clickable elements) (those ability configurations currently have minimal impact).
We also might want to consider whether it really makes sense to have an Indeterminate state given that it is only relevant for switch widgets. I guess it fits with the Checked state, although it might make sense to at least add helper functions for the indeterminate state to switch and switches; we could probably add a field to SwitchItem for Indeterminate to make setting that easier.
A summary of the current naming thinking:
states.Displayableinstead ofstates.Invisible, withWidgetBase.IsDisplayableandWidgetBase.IsVisiblehelper functions (IsVisible checks the bounding box for actual visibility)abilities.Enabledinstead ofstates.Disabledabilities.Editableinstead ofstates.ReadOnly