Make Styles inherit by default #82
Conversation
cceckman
force-pushed
the
inheritstyle
branch
from
f45bd23 to
101ca3f
Compare
|
Has been rebased after #80; ready for review. |
| // StyledBox is a Box with an overriden Draw method. | ||
| // Embedding a Widget within another allows overriding of some behaviors. | ||
| type StyledBox struct { | ||
| Style string |
There was a problem hiding this comment.
This should probably be exported as a method rather than a field to keep the API consistent. I don't believe there are any exported fields in the current API (to my knowledge).
There was a problem hiding this comment.
Sorry, I scanned it too fast (this isn't part of the API). Ignore my comment.
There was a problem hiding this comment.
Yep, this is just internal for the example.
|
You're on fire! 🥇 Only had one comment, otherwise I'm ready to merge! |
|
Regarding the comment on |
|
Re: Moving |
|
(Also as FYI, re: "on fire" - I'm on vacation for a couple of weeks, and therefore have lots of weekday time for personal projects. That includes the stuff I'm using |
WithStyle and Theme have an interesting behavior. Theme.Style will always return a value (the zero value of Style, essentially), even if the Style with the requested name doesn't have anything set in the Theme. However, as far as I can tell, this does't matter- WithStyle first checks if the Theme HasStyle, and only applies an additional style when one is set in the Theme. I'd like these to be the same (see marcusolsson#81)... but I'm not going to send this as a PR yet until that's agreed.
The previous Style behavior was for each WithStyle application to reset each property; application of "this color" and then "underline" would go back to ColorDefault. This allows each of Style's properties to be either explicitly specified, or to inherit from the existing Style. The default is to inherit from the next style up in the widget stack. Mostly this is accomplished by making the boolean properties into a tri-state (inherit/on/off) type.
cceckman
force-pushed
the
inheritstyle
branch
from
101ca3f to
45b2dfd
Compare
Overall, this fixes #81 - it changes the semantics of
Styleto be a set of overrides on top of whatever the preexisting style is. The zero value forStylemeans "inherit everything".Stylechanges in a backwards-incompatible way- makesReverse,Bold, andUnderlineinto tri-state values ("inherit", "on", and "off").The included example (examples/color/main.go) isn't as clean as I'd like because it requires embedding- there's not a way to apply a style to one
Boxbut not another, at the moment.Follows #80; I'll rebase from there once merged. (I'm also open to suggestions on how to present stacked pull requests in a more sensible way.) Tagging #36 because it's a similar behavior.