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

Jump to bottom

Add EditorStyles CSS to the widgets editor #25699

Merged
merged 2 commits into from Sep 30, 2020
Merged

Add EditorStyles CSS to the widgets editor #25699

merged 2 commits into from Sep 30, 2020

Conversation

adamziel
Copy link
Contributor

@adamziel adamziel commented Sep 29, 2020 •
edited

Description

Solves #17391 by using the same <EditorStyles /> as in the post editor.

How has this been tested?

  1. Go to widgets editor.
  2. Add a list.
  3. Confirm it's rendered in the same way as in the post editor (think margins, list marker).

Screenshots

Zrzut ekranu 2020-09-29 o 13 52 09

Types of changes

Bug fix (non-breaking change which fixes an issue)

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.

@adamziel adamziel self-assigned this Sep 29, 2020
@adamziel adamziel added [Block] Legacy Widget Affects the Legacy Widget Block - used for displaying Classic Widgets [Feature] Widgets Screen The block-based screen that replaced widgets.php. [Package] Edit Widgets /packages/edit-widgets labels Sep 29, 2020
@adamziel adamziel added this to PRs in progress in Block-based Widgets Editor via automation Sep 29, 2020
@github-actions
Copy link

github-actions bot commented Sep 29, 2020 •
edited

Size Change: +19 B (0%)

Total Size: 1.17 MB

Filename Size Change
build/block-editor/index.js 129 kB +62 B (0%)
build/components/index.js 168 kB -5 B (0%)
build/edit-site/index.js 20.4 kB -77 B (0%)
build/edit-site/style-rtl.css 3.76 kB +222 B (5%) 🔍
build/edit-site/style.css 3.77 kB +224 B (5%) 🔍
build/edit-widgets/index.js 18.2 kB +28 B (0%)
build/edit-widgets/style-rtl.css 2.82 kB +7 B (0%)
build/edit-widgets/style.css 2.82 kB +5 B (0%)
build/rich-text/index.js 13 kB -447 B (3%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.52 kB 0 B
build/api-fetch/index.js 3.35 kB 0 B
build/autop/index.js 2.72 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 8.61 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/style-rtl.css 11.1 kB 0 B
build/block-editor/style.css 11.1 kB 0 B
build/block-library/editor-rtl.css 8.6 kB 0 B
build/block-library/editor.css 8.6 kB 0 B
build/block-library/index.js 135 kB 0 B
build/block-library/style-rtl.css 7.61 kB 0 B
build/block-library/style.css 7.6 kB 0 B
build/block-library/theme-rtl.css 741 B 0 B
build/block-library/theme.css 741 B 0 B
build/block-serialization-default-parser/index.js 1.78 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 47.5 kB 0 B
build/components/style-rtl.css 15.4 kB 0 B
build/components/style.css 15.4 kB 0 B
build/compose/index.js 9.42 kB 0 B
build/core-data/index.js 12 kB 0 B
build/data-controls/index.js 1.27 kB 0 B
build/data/index.js 8.42 kB 0 B
build/date/index.js 31.9 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 4.42 kB 0 B
build/edit-navigation/index.js 10.7 kB 0 B
build/edit-navigation/style-rtl.css 868 B 0 B
build/edit-navigation/style.css 871 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.25 kB 0 B
build/edit-post/style.css 6.24 kB 0 B
build/editor/editor-styles-rtl.css 492 B 0 B
build/editor/editor-styles.css 493 B 0 B
build/editor/index.js 45.4 kB 0 B
build/editor/style-rtl.css 3.83 kB 0 B
build/editor/style.css 3.82 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 7.49 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 1.74 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.55 kB 0 B
build/is-shallow-equal/index.js 709 B 0 B
build/keyboard-shortcuts/index.js 2.39 kB 0 B
build/keycodes/index.js 1.85 kB 0 B
build/list-reusable-blocks/index.js 3.02 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.12 kB 0 B
build/notices/index.js 1.69 kB 0 B
build/nux/index.js 3.27 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.44 kB 0 B
build/primitives/index.js 1.34 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/server-side-render/index.js 2.6 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.24 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.74 kB 0 B
build/warning/index.js 1.13 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@youknowriad
Copy link
Contributor

This is something we wanted to avoid because the editor styles are optimized for the post editor and site editor but potentially the widget area is different.

For instance a sidebar might use a different background and font for than the regular post content.

@youknowriad
Copy link
Contributor

A better approach here might be to load the default editor styles.

draganescu
draganescu approved these changes Sep 29, 2020
View reviewed changes
Copy link
Contributor

@draganescu draganescu left a comment

Choose a reason for hiding this comment

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

This looks and works all right in my testing and review.
Considering we will use the "fake post" technique for new editors ahead we may want to update the docs and specify the post for the block_editor_settings filter can be null.

youknowriad
youknowriad reviewed Sep 29, 2020
View reviewed changes
lib/widgets-page.php Outdated
@@ -77,7 +77,7 @@ function gutenberg_widgets_init( $hook ) {
gutenberg_get_legacy_widget_settings()
);

$settings = gutenberg_experimental_global_styles_settings( $settings );
$settings = apply_filters( 'block_editor_settings', $settings, null );
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should do that tbh, this will bring a ton of settings and features from the post editor that we're uncertain whether we want or not; Maybe it's good but I'd rather do it setting per setting.

As mentionned, the styles is a good one where I think they are different. The widgets should just use the default editor styles.

@adamziel
Copy link
Contributor Author

@youknowriad all good notes - thank you for this extra context. I added a comment so that it's explained right in the code. I wasn't sure what you meant exactly by "default editor styles" so I used gutenberg_extend_block_editor_styles - did I guess correctly? If not, would you mind expanding?

@youknowriad
Copy link
Contributor

By default editor styles, I mean the ones that are provided by the plugin itself. This file basically gutenberg_dir_path() . 'build/editor/editor-styles.css'; and not the ones provided by the theme. I believe the gutenberg_extend_block_editor_styles function will bring the ones provided by theme if existing.

@adamziel
Copy link
Contributor Author

adamziel commented Sep 30, 2020 •
edited

@youknowriad I may be missing something, but gutenberg_extend_block_editor_styles doesn't seem to bring over the styles provided by the theme:

gutenberg/lib/client-assets.php

Lines 570 to 620 in 06d1740

function gutenberg_extend_block_editor_styles( $settings ) {
$editor_styles_file = gutenberg_dir_path() . 'build/editor/editor-styles.css';
/*
* If, for whatever reason, the built editor styles do not exist, avoid
* override and fall back to the default.
*/
if ( ! file_exists( $editor_styles_file ) ) {
return $settings;
}
if ( empty( $settings['styles'] ) ) {
$settings['styles'] = array();
} else {
/*
* The styles setting is an array of CSS strings, so there is no direct
* way to find the default styles. To maximize stability, load (again)
* the default styles from disk and find its place in the array.
*
* See: https://github.com/WordPress/wordpress-develop/blob/5.0.3/src/wp-admin/edit-form-blocks.php#L168-L175
*/
$default_styles = file_get_contents(
ABSPATH . WPINC . '/css/dist/editor/editor-styles.css'
);
/*
* Iterate backwards from the end of the array since the preferred
* insertion point in case not found is prepended as first entry.
*/
for ( $i = count( $settings['styles'] ) - 1; $i >= 0; $i-- ) {
if ( isset( $settings['styles'][ $i ]['css'] ) &&
$default_styles === $settings['styles'][ $i ]['css'] ) {
break;
}
}
}
$editor_styles = array(
'css' => file_get_contents( $editor_styles_file ),
);
// Substitute default styles if found. Otherwise, prepend to setting array.
if ( isset( $i ) && $i >= 0 ) {
$settings['styles'][ $i ] = $editor_styles;
} else {
array_unshift( $settings['styles'], $editor_styles );
}
return $settings;
}

It sets $editor_styles_file = gutenberg_dir_path() . 'build/editor/editor-styles.css'; once and then ensures it's part of $settings['styles']. In which case it would be the right function to use?

@youknowriad
Copy link
Contributor

Oh! you're right :) ignore me :P

@adamziel adamziel merged commit 2acb853 into master Sep 30, 2020
Block-based Widgets Editor automation moved this from PRs in progress to Done Sep 30, 2020
@adamziel adamziel deleted the fix/widget-editor-styles branch September 30, 2020 10:21
@github-actions github-actions bot added this to the Gutenberg 9.2 milestone Sep 30, 2020
This was referenced Sep 30, 2020
Closed
Closed
kevin940726 pushed a commit that referenced this pull request Oct 6, 2020
@adamziel @kevin940726
Add EditorStyles CSS to the widgets editor (#25699)
b7105ee 
* Add EditorStyles CSS to the widgets editor

* Use gutenberg_extend_block_editor_styles instead of all block_editor_settings
kevin940726 pushed a commit that referenced this pull request Oct 6, 2020
@adamziel @kevin940726
Add EditorStyles CSS to the widgets editor (#25699)
da00f1b 
* Add EditorStyles CSS to the widgets editor

* Use gutenberg_extend_block_editor_styles instead of all block_editor_settings
@Mamaduka Mamaduka mentioned this pull request Dec 2, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@youknowriad youknowriad youknowriad left review comments

@draganescu draganescu draganescu approved these changes

@spacedmonkey spacedmonkey Awaiting requested review from spacedmonkey

@TimothyBJacobs TimothyBJacobs Awaiting requested review from TimothyBJacobs

Assignees

@adamziel adamziel

Labels
[Block] Legacy Widget Affects the Legacy Widget Block - used for displaying Classic Widgets [Feature] Widgets Screen The block-based screen that replaced widgets.php. [Package] Edit Widgets /packages/edit-widgets
Projects
No open projects
Block-based Widgets Editor
Done
Milestone
Gutenberg 9.2
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@adamziel @youknowriad @draganescu