[RNMobile] Global styles provider #21637

dratwas · 2020-04-16T08:16:51Z

Description

This is a very first thing from wordpress-mobile/gutenberg-mobile#2147

This PR introduces a global style - block-level support mechanism to the mobile.

Terms:

wrapperStyles - styles that are applied on the block level

<!-- wp:paragraph {"style":{"color":{"text":"#4e4d0e"}}} -->
<p class="has-text-color" style="color:#4e4d0e">Color set in paragraph</p>
<!-- /wp:paragraph -->

globalStyles - styles that are passed down through editor and are overridden by block-level ones

Recently a lot of changes have been made in terms of custom colors and style in general.
The color and the background color is added to the wrapper on the web. (other styles are added or will be added soon):

In this PR I use getEditWrapperProps to get wrapper styles and I pass them to the block component. Thanks to that we can get background or color(and other props like line-height) of the block (if set) from wrapperProps like here - https://github.com/WordPress/gutenberg/pull/21637/files#diff-b27dffd07183b62521c86ec1d84cad1dR226

In addition, I added GlobalStylesContext to pass styles down to the nested blocks and override needed values on the block level. So, we can get correct styles wherever we use the GlobalStylesContext.Consumer.
We should pass flatten Theme (global) > User (edited global styles) > Specific styles per post as the initial value and then the style will be updated on the block level by concatenating global styles and wrapper style. On the web, these values are set as inline styles and are inherited by blocks out of the box so it's enough to set them there.

I'm not sure where it should be done on mobile. In the current implementation, I do that in block.native.js https://github.com/WordPress/gutenberg/pull/21637/files#diff-42b230fc8f23c8ce7322e2d6ff6d5df6R63-R66
That means the styles are updated on the block level for all blocks with no exceptions and in the edit file of the block we have correct values.

The second place where it could be done is an edit file of each block that has inner blocks. Thanks to that we could decide if that block should override global styles for its inner block. It is a different behavior than the web so I decided to go with the first option. This approach would also require external developers of 3rd party block to think about that if they want to override styles or not.

In both approaches, we have to keep in mind that sometimes we shouldn't apply the global styles as a fallback.
Example:
Let's say we have a group block where we set the background color to red. The red background color shouldn't be applied in a button block that is inside that group. So, in that case, we should set the background only when is set in wrapperProps of that button block.

How has this been tested?

Create a nested structure with groups, paragraphs, media-text in the web editor (local)
Set custom text/background colors
Copy HTML and use it on mobile or use this one below:

<!-- wp:group {"style":{"color":{"text":"#a54b01"}}} -->
<div class="wp-block-group has-text-color" style="color:#a54b01"><div class="wp-block-group__inner-container"><!-- wp:paragraph -->
<p>Hello :)</p>
<!-- /wp:paragraph -->

<!-- wp:group {"style":{"color":{"text":"#a9d994"}}} -->
<div class="wp-block-group has-text-color" style="color:#a9d994"><div class="wp-block-group__inner-container"><!-- wp:paragraph -->
<p>From colorful paragraphs :)</p>
<!-- /wp:paragraph -->

<!-- wp:group {"style":{"color":{"text":"#140c52"}}} -->
<div class="wp-block-group has-text-color" style="color:#140c52"><div class="wp-block-group__inner-container"><!-- wp:paragraph -->
<p>Colors set in groups</p>
<!-- /wp:paragraph -->

<!-- wp:group -->
<div class="wp-block-group"><div class="wp-block-group__inner-container"><!-- wp:paragraph {"style":{"color":{"text":"#4e4d0e"}}} -->
<p class="has-text-color" style="color:#4e4d0e">Color set in paragraph</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:group --></div></div>
<!-- /wp:group --></div></div>
<!-- /wp:group -->

<!-- wp:media-text {"style":{"color":{"background":"#4b9cbf","text":"#e4dc7c"}}} -->
<div class="wp-block-media-text alignwide is-stacked-on-mobile has-text-color has-background" style="background-color:#4b9cbf;color:#e4dc7c"><figure class="wp-block-media-text__media"></figure><div class="wp-block-media-text__content"><!-- wp:paragraph {"placeholder":"Content…","fontSize":"large"} -->
<p class="has-large-font-size">media-text works as well</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:media-text --></div></div>
<!-- /wp:group -->

Colors should be applied to the paragraphs/media-text (the group doesn't support colors yet)

Screenshots

Web

Mobile

Types of changes

Introduction of the mechanism that allows passing styles to the inner blocks and use getEditWrapperProps to get block-level styles. Global > Block > Block > ... (Global > Group > Group > Paragraph)

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR.

github-actions · 2020-04-16T08:23:17Z

Size Change: -793 B (0%)

Total Size: 816 kB

Filename	Size	Change
`build/block-editor/index.js`	106 kB	+76 B (0%)
`build/block-library/editor-rtl.css`	7.04 kB	-14 B (0%)
`build/block-library/editor.css`	7.04 kB	-14 B (0%)
`build/block-library/index.js`	114 kB	+1.28 kB (1%)
`build/blocks/index.js`	48.1 kB	-1 B
`build/components/index.js`	179 kB	-13 B (0%)
`build/components/style-rtl.css`	16.9 kB	-8 B (0%)
`build/components/style.css`	16.9 kB	-8 B (0%)
`build/compose/index.js`	6.66 kB	+2 B (0%)
`build/data/index.js`	8.44 kB	+14 B (0%)
`build/edit-post/index.js`	27.6 kB	-188 B (0%)
`build/edit-post/style-rtl.css`	12.2 kB	-121 B (0%)
`build/edit-post/style.css`	12.2 kB	-122 B (1%)
`build/edit-site/index.js`	10.9 kB	-127 B (1%)
`build/edit-site/style-rtl.css`	5.11 kB	-151 B (2%)
`build/edit-site/style.css`	5.11 kB	-147 B (2%)
`build/edit-widgets/index.js`	7.5 kB	-831 B (11%)	👏
`build/edit-widgets/style-rtl.css`	4.67 kB	-336 B (7%)	✅
`build/edit-widgets/style.css`	4.66 kB	-338 B (7%)	✅
`build/editor/index.js`	43.6 kB	+240 B (0%)
`build/format-library/index.js`	7.63 kB	+1 B
`build/keyboard-shortcuts/index.js`	2.51 kB	-2 B (0%)
`build/list-reusable-blocks/index.js`	3.12 kB	+1 B
`build/media-utils/index.js`	5.29 kB	+1 B
`build/plugins/index.js`	2.67 kB	-1 B
`build/redux-routine/index.js`	2.85 kB	+10 B (0%)
`build/rich-text/index.js`	14.8 kB	+1 B

ℹ️ View Unchanged

Filename	Size	Change
`build/a11y/index.js`	1.02 kB	0 B
`build/annotations/index.js`	3.62 kB	0 B
`build/api-fetch/index.js`	4.08 kB	0 B
`build/autop/index.js`	2.82 kB	0 B
`build/blob/index.js`	620 B	0 B
`build/block-directory/index.js`	6.23 kB	0 B
`build/block-directory/style-rtl.css`	760 B	0 B
`build/block-directory/style.css`	761 B	0 B
`build/block-editor/style-rtl.css`	10.2 kB	0 B
`build/block-editor/style.css`	10.2 kB	0 B
`build/block-library/style-rtl.css`	7.14 kB	0 B
`build/block-library/style.css`	7.14 kB	0 B
`build/block-library/theme-rtl.css`	683 B	0 B
`build/block-library/theme.css`	685 B	0 B
`build/block-serialization-default-parser/index.js`	1.88 kB	0 B
`build/block-serialization-spec-parser/index.js`	3.1 kB	0 B
`build/core-data/index.js`	11.4 kB	0 B
`build/data-controls/index.js`	1.29 kB	0 B
`build/date/index.js`	5.47 kB	0 B
`build/deprecated/index.js`	772 B	0 B
`build/dom-ready/index.js`	568 B	0 B
`build/dom/index.js`	3.1 kB	0 B
`build/edit-navigation/index.js`	3.54 kB	0 B
`build/edit-navigation/style-rtl.css`	485 B	0 B
`build/edit-navigation/style.css`	485 B	0 B
`build/editor/editor-styles-rtl.css`	428 B	0 B
`build/editor/editor-styles.css`	431 B	0 B
`build/editor/style-rtl.css`	3.27 kB	0 B
`build/editor/style.css`	3.27 kB	0 B
`build/element/index.js`	4.65 kB	0 B
`build/escape-html/index.js`	733 B	0 B
`build/format-library/style-rtl.css`	502 B	0 B
`build/format-library/style.css`	502 B	0 B
`build/hooks/index.js`	2.13 kB	0 B
`build/html-entities/index.js`	622 B	0 B
`build/i18n/index.js`	3.56 kB	0 B
`build/is-shallow-equal/index.js`	710 B	0 B
`build/keycodes/index.js`	1.94 kB	0 B
`build/list-reusable-blocks/style-rtl.css`	226 B	0 B
`build/list-reusable-blocks/style.css`	226 B	0 B
`build/notices/index.js`	1.79 kB	0 B
`build/nux/index.js`	3.4 kB	0 B
`build/nux/style-rtl.css`	616 B	0 B
`build/nux/style.css`	613 B	0 B
`build/primitives/index.js`	1.5 kB	0 B
`build/priority-queue/index.js`	789 B	0 B
`build/server-side-render/index.js`	2.68 kB	0 B
`build/shortcode/index.js`	1.7 kB	0 B
`build/token-list/index.js`	1.28 kB	0 B
`build/url/index.js`	4.02 kB	0 B
`build/viewport/index.js`	1.84 kB	0 B
`build/warning/index.js`	1.14 kB	0 B
`build/wordcount/index.js`	1.18 kB	0 B

_{compressed-size-action}

SergioEstevao · 2020-04-22T10:44:24Z

packages/block-library/src/paragraph/edit.native.js

- ...oldStyle,
- color: style && style.color && style.color.text,
+ ...style,
+ color: wrapperProps?.style?.color || globalStyle?.color,


Looking at this line I'm assuming that at the moment color is the only global style and wrapper prop that we have available in mobile?

ATM we support color, backgroundColor, and gradient. Other props will be added when they land on the web because we use the same logic

@SergioEstevao
I have checked that and we have 3 kinds of styles in wrapper props at this moment.

lineHeight - __experimentalLineHeight

fontSize - __experimentalFontSize

color/backgroundColor/gradient - __experimentalColor

Unfortunately, it seems like the lineHeight doesn't work in the rich-text at all. The fontSize works only for a paragraph block but not for heading block - It's related to thetagName that is passed to the rich-text.

So, we have these props already in the wrapperProps but our edit implementations don't use them yet.

BackgroundColor does not work in the Paragraph block?

Yeah, the backgroundColor works. I should mention that in my prev comment that I refactored it a bit :)

packages/block-editor/src/components/block-list/block.native.js

SergioEstevao

Looking and working great! Thanks for this!

pinarol · 2020-04-24T17:15:17Z

Thank you @dratwas for the PR! This is a tough one but I think you are in the right direction.

First of all I think we should adopt the terminology in the main issue and probably not use the term "globalStyle", because it seems like what we have here is only the container level style, or merged style? Maybe the context name is fine to include "global" but the prop name can benefit from changing.

Other than that, I have some general questions:

backgroundColor is something every block can have, I was wondering instead of separately setting it for every block, can we set it in the block container level?
If we can't set the background color in the block container level, then, what other thing can we do to make it super practical for every block?
Can we pass only 1 style object to every block which is already merged beforehand? In other words, why do we need to pass multiple style objects to every block?
Can you add an example for Button block and demonstrate how it'll look when you put that into a Group with a background color? I am curious because it already has a color, but it can also have a background color which is set in the container level.

dratwas · 2020-04-27T13:40:13Z

@pinarol thanks for your review :)

backgroundColor is something every block can have, I was wondering instead of separately setting it for every block, can we set it in the block container level?

I tried something like this just to check how it would look like but had issues with block borders. They are cut by an inner block background. I tried to change zIndex of borders but then blocks are not clickable because borders are on top of them.

The effect is different when I moved the background color a bit deeper (View that is always inside borders since they are positioned absolutely relative to this view). Borders are not cut but it looks a bit weird.

There is also an issue with a button component because the `backgroundColor` should be applied to the button, not its wrapper.

If we can't set the background color in the block container level, then, what other thing can we do to make it super practical for every block?

I think that it could be added on the block level - this is the best option - but the bordering needs to be fixed if we want the full-width backgroundColor

Can we pass only 1 style object to every block which is already merged beforehand? In other words, why do we need to pass multiple style objects to every block?

By multiple styles, you mean wrapperProps and globalStyle/mergedStyle? We need them for a button block or any other block that doesn't want to use globalStyles/mergedStyle.

Can you add an example for Button block and demonstrate how it'll look when you put that into a Group with a background color? I am curious because it already has a color, but it can also have a background color which is set in the container level.

It is exactly the thing why we need wrapperProps and globalStyle/mergedStyle in a block. If we don't want to use globalStyle/mergedStyle then we use wrapperProps to be sure that these styles are set directly to this block (They are not set in the container)

I used wrapperProps in button here - https://github.com/WordPress/gutenberg/pull/21637/files#diff-b448757d9f342ae78608e510bb8cc087R178

On the screen below i set background color in group but it's not applied to the button (it's expected result)

pinarol · 2020-04-27T17:32:03Z

I tried to change zIndex of borders but then blocks are not clickable because borders are on top of them.

Thanks for trying! I wonder if it is possible to cut a hole inside the border view so the rest becomes clickable? Just asking, no need to dive deep into this at the moment.

The effect is different when I moved the background color a bit deeper (View that is always inside borders since they are positioned absolutely relative to this view). Borders are not cut but it looks a bit weird.

Does it look different than what we already have on media-text?

And I was wondering how would that look if we applied the background color on Group block with the method you use for Media-Text?

It might look a bit weird but if you think we don't have any indentation inside inner blocks it's kind of a natural result. I checked how this works on web and I am a bit surprised about the UX there, because when you add a background color it adds indentation as well. I don't think we can do the same thing in the narrow screen.

👆 Here are 2 Paragraph blocks that are both on top level. It simply adds padding when you add background color, and the interesting thing is, the web output result also has padding. I am noting this to ask in the editor chat, curious if this is a long term approach.

There is also an issue with a button component because the backgroundColor should be applied to the button, not its wrapper.

OK I think this is about the current architecture of global styles. I find it a bit confusing to use the universal Background color palette for button color as well. Imho it should be represented by a different color section in the settings. Because the button color is simply not a background color, and one should be able to set the Button colors on a global level as well. But since the GSS is still work in progress it is early to make a judgement. I think what we have is enough for now.

pinarol · 2020-04-27T17:36:22Z

packages/block-library/src/button/edit.native.js


 return (
- ( style && style.color && style.color.background ) ||
+ wrapperProps?.style?.backgroundColor ||
 backgroundColor.color ||


Do we still need backgroundColor.color ? Is it for backward compatibility ? If so, it could be good to drop an inline comment to explain why we still need that.

Yes, we need that for supporting colors that are not custom ones and are set from the color palette. Let me check if i could merge them in the block.native.js and we could remove it from here. Additional pros of this is that will be applied for all blocks (like media-text etc)

Edit: I think we should leave it as it is because i would need to refactor with-colors to make it work.

I added an inline comment

packages/block-editor/src/components/block-list/block.native.js

packages/block-library/src/media-text/edit.native.js

dratwas · 2020-04-28T12:30:44Z

Thanks for trying! I wonder if it is possible to cut a hole inside the border view so the rest becomes clickable? Just asking, no need to dive deep into this at the moment.

Unfortunately, I'm not sure if there is an easy way to achieve that.

Does it look different than what we already have on media-text?

And I was wondering how would that look if we applied the background color on Group block with the method you use for Media-Text?

It looks the same.

I checked how this works on web and I am a bit surprised about the UX there, because when you add a background color it adds indentation as well. I don't think we can do the same thing in the narrow screen.

Yeah, i observed the same and was wondering if we should follow that rule but i remember that at some point, we decided to remove that indentation.

I am noting this to ask in the editor chat, curious if this is a long term approach.

🙏

pinarol · 2020-04-28T14:59:16Z

Yeah, i observed the same and was wondering if we should follow that rule but i remember that at some point, we decided to remove that indentation.

OK, I'll open a separate issue to track this, we can consider adding some padding as well.

pinarol

Thank you for the updates, this looks good to me! 🎉

dratwas added 2 commits April 14, 2020 15:37

use edit wrapper props

16e02a4

use context to pass wrapper styles

cdf9c39

dratwas added 5 commits April 16, 2020 15:48

Merge branch 'master' into rnmobile/feat/wrapperProps

d2c4711

split to globalStyles and wrapperProps

3e2e078

fix import name

ea89fb0

Merge branch 'master' into rnmobile/feat/wrapperProps

a8104ec

fix paragraph block

ce8009a

dratwas marked this pull request as ready for review April 20, 2020 16:23

dratwas requested review from SergioEstevao, pinarol and Tug April 21, 2020 08:34

SergioEstevao reviewed Apr 22, 2020

View reviewed changes

packages/block-editor/src/components/block-list/block.native.js Outdated Show resolved Hide resolved

dratwas added 3 commits April 22, 2020 15:29

add default value to the wrapperProps in compose

f6b82e4

Merge branch 'master' into rnmobile/feat/wrapperProps

889aa5c

change heading and paragraph to support other wrapperprops

14475a1

dratwas requested review from ajitbohra, jorgefilipecosta, Soean and talldan as code owners April 23, 2020 12:16

dratwas removed request for talldan, Soean, ajitbohra and jorgefilipecosta April 23, 2020 14:58

geriux mentioned this pull request Apr 24, 2020

[RNMobile] Color settings #21326

Merged

6 tasks

SergioEstevao approved these changes Apr 24, 2020

View reviewed changes

dratwas added 2 commits April 27, 2020 11:51

rename globalStyle to mergedStyle and merge styles before passing

2654cdc

use wrapperProps in button edit

6b1677b

Merge branch 'master' into rnmobile/feat/wrapperProps

4b82178

pinarol reviewed Apr 27, 2020

View reviewed changes

add some inline comments

a6e9b88

pinarol approved these changes Apr 28, 2020

View reviewed changes

pinarol mentioned this pull request Apr 28, 2020

Apply background/foreground colors to blocks wordpress-mobile/gutenberg-mobile#2195

Closed

dratwas merged commit 60ad217 into master Apr 28, 2020

dratwas deleted the rnmobile/feat/wrapperProps branch April 28, 2020 17:44

github-actions bot added this to the Gutenberg 8.1 milestone Apr 28, 2020

dratwas mentioned this pull request Apr 28, 2020

Global styles provider wordpress-mobile/gutenberg-mobile#2196

Merged

2 tasks

This was referenced May 1, 2020

Do we need to pass className or wrapperClassName? wordpress-mobile/gutenberg-mobile#102

Closed

Discuss: Have a styling solution that is close to gutenberg web's wordpress-mobile/gutenberg-mobile#323

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RNMobile] Global styles provider #21637

[RNMobile] Global styles provider #21637

dratwas commented Apr 16, 2020 •

edited

Loading

github-actions bot commented Apr 16, 2020 •

edited

Loading

SergioEstevao Apr 22, 2020

dratwas Apr 22, 2020 •

edited

Loading

dratwas Apr 23, 2020

SergioEstevao Apr 23, 2020

dratwas Apr 24, 2020

SergioEstevao left a comment

pinarol commented Apr 24, 2020 •

edited

Loading

dratwas commented Apr 27, 2020 •

edited

Loading

pinarol commented Apr 27, 2020 •

edited

Loading

pinarol Apr 27, 2020

dratwas Apr 28, 2020 •

edited

Loading

dratwas commented Apr 28, 2020

pinarol commented Apr 28, 2020

pinarol left a comment

[RNMobile] Global styles provider #21637

[RNMobile] Global styles provider #21637

Conversation

dratwas commented Apr 16, 2020 • edited Loading

Description

Terms:

How has this been tested?

Screenshots

Web

Mobile

Types of changes

Checklist:

github-actions bot commented Apr 16, 2020 • edited Loading

SergioEstevao Apr 22, 2020

Choose a reason for hiding this comment

dratwas Apr 22, 2020 • edited Loading

Choose a reason for hiding this comment

dratwas Apr 23, 2020

Choose a reason for hiding this comment

SergioEstevao Apr 23, 2020

Choose a reason for hiding this comment

dratwas Apr 24, 2020

Choose a reason for hiding this comment

SergioEstevao left a comment

Choose a reason for hiding this comment

pinarol commented Apr 24, 2020 • edited Loading

dratwas commented Apr 27, 2020 • edited Loading

pinarol commented Apr 27, 2020 • edited Loading

pinarol Apr 27, 2020

Choose a reason for hiding this comment

dratwas Apr 28, 2020 • edited Loading

Choose a reason for hiding this comment

dratwas commented Apr 28, 2020

pinarol commented Apr 28, 2020

pinarol left a comment

Choose a reason for hiding this comment

dratwas commented Apr 16, 2020 •

edited

Loading

github-actions bot commented Apr 16, 2020 •

edited

Loading

dratwas Apr 22, 2020 •

edited

Loading

pinarol commented Apr 24, 2020 •

edited

Loading

dratwas commented Apr 27, 2020 •

edited

Loading

pinarol commented Apr 27, 2020 •

edited

Loading

dratwas Apr 28, 2020 •

edited

Loading