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

Replace the widgets screen experiment with a theme support #24087

Merged
merged 5 commits into from
Aug 26, 2020

Conversation

youknowriad
Copy link
Contributor

@youknowriad youknowriad commented Jul 21, 2020

closes #24685

The idea at the moment is to try to ship the new block-based widgets screen in 5.6. If we want to make this a reality, we need more testing and a more stable implementation.

This PR does a few things to help with that:

  • Replace the experiments flag with a theme support flag to allow themes to opt-in into the block-based widgets screen.
  • Hides and replaces the existing widgets pages and panel with the block-based ones when theme support is enabled.

@youknowriad youknowriad added the [Feature] Widgets Screen The block-based screen that replaced widgets.php. label Jul 21, 2020
@youknowriad youknowriad self-assigned this Jul 21, 2020
@youknowriad youknowriad added [Type] Feature New feature to highlight in changelogs. [Type] New API New API to be used by plugin developers or package users. labels Jul 21, 2020
gutenberg.php Outdated
@@ -181,3 +184,5 @@ function register_site_icon_url( $response ) {
}

add_filter( 'rest_index', 'register_site_icon_url' );

add_theme_support( 'block-based-widgets' );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The support is enabled by default in the Gutenberg plugin to bring visibility to the feature and get more testing, but in Core it could be made opt-in instead of opt-out.

Copy link
Member

Choose a reason for hiding this comment

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

Can we rename it to something like "widgets-block-editor"?

@youknowriad youknowriad requested a review from mapk July 21, 2020 08:42
@github-actions
Copy link

github-actions bot commented Jul 21, 2020

Size Change: 0 B

Total Size: 1.16 MB

Filename Size Change
build/edit-widgets/index.js 11.8 kB -10 B (0%)
build/edit-widgets/style-rtl.css 2.46 kB +5 B (0%)
build/edit-widgets/style.css 2.45 kB +5 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.67 kB 0 B
build/api-fetch/index.js 3.44 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 7.99 kB 0 B
build/block-directory/style-rtl.css 953 B 0 B
build/block-directory/style.css 952 B 0 B
build/block-editor/index.js 126 kB 0 B
build/block-editor/style-rtl.css 10.7 kB 0 B
build/block-editor/style.css 10.7 kB 0 B
build/block-library/editor-rtl.css 8.5 kB 0 B
build/block-library/editor.css 8.5 kB 0 B
build/block-library/index.js 135 kB 0 B
build/block-library/style-rtl.css 7.45 kB 0 B
build/block-library/style.css 7.45 kB 0 B
build/block-library/theme-rtl.css 729 B 0 B
build/block-library/theme.css 730 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/blocks/index.js 47.7 kB 0 B
build/components/index.js 200 kB 0 B
build/components/style-rtl.css 15.7 kB 0 B
build/components/style.css 15.7 kB 0 B
build/compose/index.js 9.67 kB 0 B
build/core-data/index.js 12.3 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.55 kB 0 B
build/date/index.js 5.38 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.48 kB 0 B
build/edit-navigation/index.js 11.7 kB 0 B
build/edit-navigation/style-rtl.css 1.16 kB 0 B
build/edit-navigation/style.css 1.16 kB 0 B
build/edit-post/index.js 304 kB 0 B
build/edit-post/style-rtl.css 5.61 kB 0 B
build/edit-post/style.css 5.61 kB 0 B
build/edit-site/index.js 17 kB 0 B
build/edit-site/style-rtl.css 3.06 kB 0 B
build/edit-site/style.css 3.06 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/index.js 45.3 kB 0 B
build/editor/style-rtl.css 3.8 kB 0 B
build/editor/style.css 3.79 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.71 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 2.13 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 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.32 kB 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 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.41 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13.9 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@youknowriad youknowriad requested a review from a team July 21, 2020 08:55
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.

I like this idea! I tested it locally and it works great.
I am approving but a second opinion might be worth it considering we're hijacking the widgets link in the Gutenberg plugin to point to an experimental screen.

Going back to the old screen would mean I have to disable the Gutenberg plugin?

@youknowriad
Copy link
Contributor Author

Going back to the old screen would mean I have to disable the Gutenberg plugin?

Alternatively, you could do

remove_theme_support( 'block-based-widgets' );

@mapk
Copy link
Contributor

mapk commented Jul 21, 2020

Requiring the theme to opt-in for this feels like a safe step in the right direction, especially if we're replacing the Widgets screen entirely.

Without theme support

Screen Shot 2020-07-21 at 9 50 05 AM

With theme support

Screen Shot 2020-07-21 at 9 58 03 AM

@noisysocks
Copy link
Member

noisysocks commented Jul 30, 2020

One problem with this is that if a user switches away from a theme that supports block widgets, then goes to the (old) widgets screen and clicks Save, they will lose any blocks that were stored. See #24267.

A question I have about making this "opt-in" is: What about old themes that rarely receive updates? I imagined that a requirement of the blocks in widgets project was that it would let users add blocks to their site no matter what theme they're on.

@youknowriad
Copy link
Contributor Author

I imagined that a requirement of the blocks in widgets project was that it would let users add blocks to their site no matter what theme they're on.

It's still a question to be addressed but I have doubts that we'll be able to make it work regardless of the theme and widgets used thus the opt-in choice.

One problem with this is that if a user switches away from a theme that supports block widgets, then goes to the (old) widgets screen and clicks Save, they will lose any blocks that were stored. See #24267.

This is a good question and one that we'd need to solve regardless of how the screen is enabled. I'd like for this feature to get more exposure and for us to give more priority to solving these issues.

@draganescu
Copy link
Contributor

draganescu commented Jul 30, 2020

let users add blocks to their site no matter what theme they're on.
This is a nice idea, but, just like with navigation, there is no way to guarantee that it will work. It's does advance the desirable outcome of using blocks with no updates but if the result is some ugly hotchpotch on the front end it will do more harm than good to this outcome. My point is that as long as people can add any block the theme should be tweaked to support that.

In explorations with @adamziel we wondered if the path to the widget editor is reversible. Maybe as @mtias suggested we could add in the classic editor plugin, or in some other plugin, the possibility to revert to the old screens (both for navigation and for widgets). However, are they mutually exclusive? In my opinion they should be, as in if you switch back there should be no guarantee that you can easily continue where you left off. However @adamziel suggested in this thread, that by following the current data structure there won't be any issue to just move between the two screens at any moment, which is a nice to have.

I like this PR because it allows us to put these new editors in the hands of community developers before they're ready to be shipped with the plugin. They could test this and see where things can be improved. It also does not change anything about the future when we merge the new editors: when that happens the opt in is NOT to get the new editor, the opt in is to enable the new editor to allow blocks next to menus or widgets. This PR is only interesting at this stage. After the merge everyone will get the new editors but only the opting in themes will allow blocks to be used.

@TimothyBJacobs
Copy link
Member

It'd be nice to also use register_theme_feature when available.

@youknowriad
Copy link
Contributor Author

Merging this based on the last discussions on #core-editor chat.

@youknowriad youknowriad merged commit 80777ae into master Aug 26, 2020
@youknowriad youknowriad deleted the update/widgets-screen branch August 26, 2020 23:27
@mapk
Copy link
Contributor

mapk commented Aug 27, 2020

💥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Widgets Screen The block-based screen that replaced widgets.php. [Type] Feature New feature to highlight in changelogs. [Type] New API New API to be used by plugin developers or package users.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce the new Widgets editor to the world
7 participants