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

Apply the custom global styles CSS variables on the server #21420

Closed
wants to merge 1 commit into from

Conversation

youknowriad
Copy link
Contributor

This PR tries to move the CSS styles variables generation to the server to avoid persisting these in the markup.

Some thoughts so far:

  • I like the fact that the markup stays pure
  • There's some code duplication on the frontend to apply the styles on the editor
  • I don't like that the markup generated by wp.blocks.serialize is a bit less portable.

@youknowriad youknowriad added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label Apr 6, 2020
@youknowriad youknowriad requested a review from mtias April 6, 2020 09:37
@youknowriad youknowriad self-assigned this Apr 6, 2020
* @return string New block output.
*/
function gutenberg_apply_style_attribute( $block_content, $block ) {
if ( isset( $block['attrs'] ) && isset( $block['attrs']['style'] ) ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally this doesn't rely on the attributes presence but on the "support flag". We need server-side awareness of support flags. cc @gziolo did you work on that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet, the plan was to come up with something better than supports :)

Anyway, supports works on the server and can be exposed when registered there to the client. So we should be good in that context. You just need to say we want it and we can implement everything. @aduth has some custom code that shims classname or align flags on the server. We can add the same logic for everything else.

Let me know whether we should update https://github.com/WordPress/gutenberg/blob/master/docs/rfc/block-registration.md#backward-compatibility and the part that states that supports is client-side only and exists only for backward compatibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'm thinking more and more that supports is actually a good thing if done right. Don't know what you'll think.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably fine to keep it as is but make it more configurable. We still need to use data that can be serialized but we shouldn't limit ourselves to boolean.

@github-actions
Copy link

github-actions bot commented Apr 6, 2020

Size Change: -4 B (0%)

Total Size: 889 kB

Filename Size Change
build/block-editor/index.js 102 kB -10 B (0%)
build/blocks/index.js 57.5 kB +2 B (0%)
build/components/index.js 195 kB -1 B
build/edit-post/index.js 92.9 kB +2 B (0%)
build/edit-site/index.js 10.1 kB +1 B
build/editor/index.js 42.8 kB +1 B
build/plugins/index.js 2.54 kB +1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.4 kB 0 B
build/api-fetch/index.js 3.79 kB 0 B
build/autop/index.js 2.58 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.03 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 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/editor-rtl.css 7.22 kB 0 B
build/block-library/editor.css 7.22 kB 0 B
build/block-library/index.js 110 kB 0 B
build/block-library/style-rtl.css 7.53 kB 0 B
build/block-library/style.css 7.54 kB 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/components/style-rtl.css 16.6 kB 0 B
build/components/style.css 16.5 kB 0 B
build/compose/index.js 6.21 kB 0 B
build/core-data/index.js 10.7 kB 0 B
build/data-controls/index.js 1.04 kB 0 B
build/data/index.js 8.23 kB 0 B
build/date/index.js 5.36 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.05 kB 0 B
build/edit-navigation/index.js 2.71 kB 0 B
build/edit-navigation/style-rtl.css 239 B 0 B
build/edit-navigation/style.css 241 B 0 B
build/edit-post/style-rtl.css 12.3 kB 0 B
build/edit-post/style.css 12.3 kB 0 B
build/edit-site/style-rtl.css 5.02 kB 0 B
build/edit-site/style.css 5.02 kB 0 B
build/edit-widgets/index.js 7.18 kB 0 B
build/edit-widgets/style-rtl.css 3.74 kB 0 B
build/edit-widgets/style.css 3.73 kB 0 B
build/editor/editor-styles-rtl.css 400 B 0 B
build/editor/editor-styles.css 402 B 0 B
build/editor/style-rtl.css 3.49 kB 0 B
build/editor/style.css 3.49 kB 0 B
build/element/index.js 4.44 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 6.95 kB 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 1.93 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.3 kB 0 B
build/keycodes/index.js 1.7 kB 0 B
build/list-reusable-blocks/index.js 2.99 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/media-utils/index.js 4.84 kB 0 B
build/notices/index.js 1.57 kB 0 B
build/nux/index.js 3.01 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/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.5 kB 0 B
build/server-side-render/index.js 2.54 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.01 kB 0 B
build/viewport/index.js 1.61 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action


// Try replacing an existing style
$style_regex = '/((^\s*<[a-zA-Z]+\b[^><]*)style="([^"]*)")([^><]*>.*)/';
$updated_content = preg_replace_callback($style_regex, function($matches) use($extra_styles) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the preg_replace* calls use $limit = 1?

@jorgefilipecosta
Copy link
Member

Hi @youknowriad, is this approach something we still want to follow in the short term or it is not something we should give priority for now?

@youknowriad
Copy link
Contributor Author

I think it's something to keep at hand but not something to pursue immediately. Let's focus on adding more editor features support and global styles support to theme.json and blocks for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants