textual
textual copied to clipboard
[colors] Add a "auto" color
When text has color: auto [percentage] it will be displayed with the contrasting colour of its background
As a result we can have widgets that have color: auto; in their CSS, and see their text colour automatically adapted to their background:

@willmcgugan the implementation is twice more verbose than the "sentinel 'auto' Color" one, and I'm not a big fan of the fact that we now offload a characteristic of a "color" property to another property of the stylesheet. The whole thing is more complicated now imho. But here we go... This new implementation works now, with the tests of the previous one still passing as is :slightly_smiling_face:
More complicated maybe, but I think this a better abstraction. If you consider that an object should have one reason to exist, a Color class should only ever abstract a color (and "auto" is not a color). "auto" to my mind is more of a way of enabling functionality on the Style object.
@willmcgugan Feedback (hopefully) addressed! :slightly_smiling_face: 28edacf
I have to say I'm really not a fan of the color_auto - it's so tightly coupled to color and having 2 properties interfering with each other like that is just confusing. If we have to guess what the behaviour is for something that is conceptually so simple, I think we're already going to the wrong route.
I liked the idea of a sentinel value but agree the previous approach broke the "color" abstraction.
IMO it'd be nice to have the color property value have a type Color | Auto (i.e. a union where Auto is some container or sentinel to mark the special case), rather than having a separate flag. The approach as it stands feels like "using a flag to simulate a union type".
So self.styles.color could return a Color or an Auto?
What kind of interface would the Auto support? Can I write self.styles.color.r or would I have to do is isinstance(self.styes.color) beforehand?
So
self.styles.colorcould return a Color or an Auto? What kind of interface would the Auto support? Can I writeself.styles.color.ror would I have to dois isinstance(self.styes.color)beforehand?
Maybe the problem is that we're trying to fit something that is not a colour (and that doesn't exists in "browser's CSS", so we cannot take inspiration from there) into a property named "color"? :thinking:
Shouldn't we rather follow the "principle of least surprise", and say that if it's definitely not a colour it shouldn't be set via a property that has this name?
Maybe it could go under something like a standalone color-auto property, which value could either be:
set(I guess it could be a synonym for the value95%, which is the default value for theget_contrast_textmethod?)unset(similarly toborder: unsetfor example in browsers' CSS, so one can cancel the property if it's been set somewhere else)- an alpha? (in either percentage or fractional units?)
I did not mean to do that!
While we're at it I'm putting this here - mostly for myself, for future reference :smile:
List of the "universal" values we can apply to any CSS property in browsers' CSS:
- initial: to set a property to its initial value.
- inherit: to make an element's property the same as its parent.
- unset: to set a property to its inherited value if it inherits or to its initial value if not - combination of the
initialandinheritkeywords - revert: to reset a property to the value established by the user-agent stylesheet (or by user styles, if any exist).
- revert-layer: to reset a property to the value established in a previous cascade layer.
I agree that styles.color should probably not be anything other than a Color. It would surprise me if that wasn't the case..
For the Python interface, I think we will need color_auto as a boolean, but also color_auto_alpha as a float to capture the alpha color. I think that would avoid the edge case I mention above.
@DrBenton I see where you are going with another CSS property to capture the auto color state, but find the declaration of "color: auto 90%" to be quite elegant. And another rule might confuse, especially if there is a "color" and a "color-auto".
For the Python interface, I think we will need
color_autoas a boolean, but alsocolor_auto_alphaas a float to capture the alpha color. I think that would avoid the edge case I mention above.
Alright, let's do this! *cracks his knuckles
Just for the record. After a 3 way call I think we've agreed on the following:
- The
Styles.colorproperty setter can accept "auto" with an optional alpha, e.g. "auto 90%" - The
Styles.colorproperty getter returns the calculated contrasting color. - We add a
Styles.color_autoboolean (getter only) that exposes if "auto" is in effect.
I think this alleviates any edge cases, and is reasonably easy to wrap ones head around.
Thumbs up if you're in favor...
As I was implementing this I realised that there is a slight issue :sweat_smile: :
- In order to make ColorProperty return a conytrasting colour on the fly when we request its value, like we said, we would need it to compute that value by traversing the Node's ancestors tree.
- The problem with this is that we already do exactly this in the DOMNode's
colorsgetter, which not only returns this colour but also thebase_background,base_colorandbackgroundones.
--> so we would do a calculation of the colour that needs to traverse all the Node's ancestors in this colors getter, but if there is a color: auto in the mix each ancestor would also calculate its colour the fly using its own ancestors in the ColorProperty __get__ method- which gives us a loop-over-ancestors wrapped in another loop-over-ancestors :grimacing:
However, I realised that we can solve the original issue (being able to do widget.styles.color_auto = False) pretty simply, by following this "private property / public getter" principle we were mentioning: if the implementation is still the same but _color_auto is now just a private property of the StylesBase class, the issue is actually solved! :heavy_check_mark: :smile:
--> 94ba806
@DrBenton I'm not sure I follow. Let's have a chat about it on Tuesday.
@willmcgugan here you go! 4439ca1