Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

theme.json: change text colors depending on background #24166

Closed
oandregal opened this issue Jul 23, 2020 · 11 comments
Closed

theme.json: change text colors depending on background #24166

oandregal opened this issue Jul 23, 2020 · 11 comments
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json

Comments

@oandregal
Copy link
Member

I want to highlight an interaction that some themes do at the moment in PHP land, for which we don't have an alternative in the theme.json spec.

Note how the text color changes depending on the background color (dark/light combination):

text-color-change

@oandregal oandregal added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label Jul 23, 2020
@mtias mtias mentioned this issue Jul 23, 2020
82 tasks
@kjellr kjellr added the [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. label Jul 24, 2020
@aristath
Copy link
Member

aristath commented Aug 4, 2020

Ideally this could be accomplished by adding a class to elements that have a background-color.
So if a block has the has-background class, we could add an extra class to it: has-background-light or has-background-dark depending on the luminosity of the background color.
If we do that, then themes will be able to just add CSS like this:

.has-background-light:not(.has-text-color) {
  color: #000;
}

.has-background-dark:not(.has-text-color) {
  color: #fff;
}

See related PR on #24374

@oandregal
Copy link
Member Author

Hi Ari, thanks for sharing. I think we need to consider how this would play in the context of other things we're doing that may affect the color's lightness: letting the user change colors globally, using CSS variables, using a managed CSS approach so the theme doesn't enqueue any CSS, etc. Those things may change the color lightness in contexts different than editing the post, so we'd be unable to rewrite the block's markup to change the classes accordingly.

I wanted to share another approach we've tried in the pullquote block so we have it logged to look at this later, see #9763

@aristath
Copy link
Member

I wanted to share another approach we've tried in the pullquote block so we have it logged to look at this later, see #9763

If this approach was applied to ALL blocks, then we'd have something awesome in our hands 👍

@jorgefilipecosta
Copy link
Member

Maybe these interactions should be something intentional e.g: when a background color attributes changes to X, if there is not text color attribute also change color attribute to Y.
If we have a filter that allows programming this kind of logic when a theme wants this type of behaviors the theme would use the filter to program it.
We could create code samples for the most common use cases that exist now, so themes could easily do it.

@aristath
Copy link
Member

Yes! That would make life easier for many authors ❤️

@pinarol
Copy link
Contributor

pinarol commented Aug 31, 2020

We'd like to support this functionality on native mobile app as well. To be able to do that we need a common language to declare background/foreground color configurations in the theme.json, a way to say that if background is X (or darker than X?) then the foreground should be Y. If the theme does this programmatically it won't work on native mobile so I was wondering is it possible to try a more declarative & cross-platform way and put this config to theme.json?

@jorgefilipecosta
Copy link
Member

background/foreground color configurations in the theme.json, a way to say that if the background is X (or darker than X?) then the foreground should be Y. If the theme does this programmatically it won't work on native mobile so I was wondering is it possible to try a more declarative & cross-platform way and put this config to theme.json?

Hi @pinarol,
I guess there will always exist cases that are not covered by the declarative approach. E.g: if color is close to red, if attribute x and attribute y set attribute z.

Just to confirm running third-party JavaScript code on mobile is totally something we don't want to do for now. Even if we clearly know the URL of the file whose JavaScript we want to execute. The file just specifies a function whose name we know and that function receives an object (attributes) and returns another object (changed attributes), we can not execute that code on mobile?

Currently, the use cases we have are mostly for a background color whose value is (selected using a CSS class) set text color to x. And if running code is totally out of question I guess we can start with a simple declarative approach that covers this use-case.
We also have the case of expanding the padding of paragraphs on they have a background, I guess it may also be nice to try cover that in a declarative approach.

@pinarol
Copy link
Contributor

pinarol commented Sep 8, 2020

hi @jorgefilipecosta

The file just specifies a function whose name we know and that function receives an object (attributes) and returns another object (changed attributes), we can not execute that code on mobile?

It depends on what that JavaScript is doing. On mobile there's no DOM or CSS, so if the JS code is doing things related with DOM/CSS then it wouldn't work on native mobile.

On the other hand if that's a formula with basic JS operators then we should be able to evaluate it on mobile. So I don't think we are tied to fully declarative approach but we need to be clear about what inputs/outputs that formula could have.

The basic principle behind the cross platform approach is just being able to decouple ourselves from DOM/CSS.

Currently, the use cases we have are mostly for a background color whose value is (selected using a CSS class) set text color to x.

This one for example, we can't select the background color using a css class. But we should be able to evaluate some piece of code to calculate textColor:

if ( ${backgroundColor} == 'someColor' ) {
return 'yellow';
} else {
return 'white'
}

As soon as we know what we should inject to that formula from outside(in this case it is the backgroundColor) it should be evaluatable on mobile. Since there's no CSS, we'll apply the textColor returned by this formula programmatically.

(I wrote the ${backgroundColor} notation arbitrarily, please ignore the syntax for now)

As of this week @Tug is starting to actively work on Global Styles on native mobile and we are happy to try different options. Just point us to the JS code you want us to try.

(One last detail, it would be more efficient to have these kinds of JS codes in theme.json, that way we don't need to make extra network calls to get these formulas but we are open to explorations.)

@jorgefilipecosta
Copy link
Member

Thinking about the issue a little bit more I think the solutions we discussed don't match what happens in current themes and don't match the pull-quote.
For example, we have a system that:
Sets the text color to white when the background is set to black and text color is not set.
Sets the text color to black when the background is set to white and text color is not set.

When we select the background to black text color will be white, if right after we set the background to white text color will continue to be white because now we have a text color set. On current themes and pullquote this does not happen and text color also changes on the second time.

I really wonder if we should remove the color classes. If we keep the color classes the current mechanisms still work.

cc: @nosolosw

@aristath
Copy link
Member

aristath commented Sep 9, 2020

Just noting here that this should be possible server-side too via PHP... I haven't tested but something like this should work:

add_filter( 'render_block_data', function( $parsed_block ) {
    if ( isset( $parsed_block['attrs']['backgroundColor'] ) && ! isset( $parsed_block['attrs']['textColor'] ) ) {
        $parsed_block['attrs']['textColor'] = get_readable_color( $parsed_block['attrs']['backgroundColor'] );
    }
    return $parsed_block;
} );

Of course that's for the frontend and not the editor side.

Another option would be to have a global setting "Auto-Calc readable text colors from background".
If that option is enabled, it would add a listener to the background-color control. When the background-color changes, it can trigger a calc and change the value of text-colors to white/black depending on the background defined. Of course users can then go and tweak the text-color, but next time they change the background it will be auto-calc'd again (which is a good thing). The main difference with the current implementation is that this would be a permanent listener, regardless of whether the block has a text-color already or not.

@jorgefilipecosta
Copy link
Member

I'm closing this issue for now. It seems color classes will be kept, so the current mechanisms themes are using to achieve this goal can still be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json
Projects
None yet
Development

No branches or pull requests

5 participants