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

Image: Add the ability for a plugin to disable Image Editor #23966

Merged
merged 3 commits into from Jul 20, 2020

Conversation

mkaz
Copy link
Member

@mkaz mkaz commented Jul 15, 2020

Description

This adds the ability for a plugin to disable the new Image Editor tools, so if it is a feature they do not want to offer, or does not work then it can be disabled programmatically.

Uses the block-editor settings store to disable.

How has this been tested?

  1. Apply PR and confirm no change, everything works as expected (default does nothing different)

  2. Try disabling, add a simple plugin that includes something like the following and confirm the new image editing tools are disable as expected. Additionally confirm other image features still work as expected.

add_filter( 'block_editor_settings', function( $settings ) {
	$settings['imageEditing'] = false;
	return $settings;
} );

Types of changes

Disables Image Editing tools by checking if block-editor settings has been set to false.

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.

@mkaz mkaz requested a review from sirreal July 15, 2020 17:59
@mkaz mkaz requested a review from ajlende as a code owner July 15, 2020 17:59
@github-actions
Copy link

github-actions bot commented Jul 15, 2020

Size Change: +690 B (0%)

Total Size: 1.15 MB

Filename Size Change
build/block-editor/index.min.js 125 kB +646 B (0%)
build/block-library/index.min.js 132 kB -78 B (0%)
build/editor/index.min.js 45.2 kB +122 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.min.js 1.14 kB 0 B
build/annotations/index.min.js 3.67 kB 0 B
build/api-fetch/index.min.js 3.43 kB 0 B
build/autop/index.min.js 2.82 kB 0 B
build/blob/index.min.js 620 B 0 B
build/block-directory/index.min.js 7.63 kB 0 B
build/block-directory/style-rtl.css 944 B 0 B
build/block-directory/style.css 945 B 0 B
build/block-editor/style-rtl.css 10.8 kB 0 B
build/block-editor/style.css 10.8 kB 0 B
build/block-library/editor-rtl.css 7.58 kB 0 B
build/block-library/editor.css 7.57 kB 0 B
build/block-library/style-rtl.css 7.77 kB 0 B
build/block-library/style.css 7.77 kB 0 B
build/block-library/theme-rtl.css 728 B 0 B
build/block-library/theme.css 729 B 0 B
build/block-serialization-default-parser/index.min.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.min.js 3.1 kB 0 B
build/blocks/index.min.js 48.3 kB 0 B
build/components/index.min.js 200 kB 0 B
build/components/style-rtl.css 15.6 kB 0 B
build/components/style.css 15.6 kB 0 B
build/compose/index.min.js 9.67 kB 0 B
build/core-data/index.min.js 11.5 kB 0 B
build/data-controls/index.min.js 1.29 kB 0 B
build/data/index.min.js 8.45 kB 0 B
build/date/index.min.js 5.38 kB 0 B
build/deprecated/index.min.js 772 B 0 B
build/dom-ready/index.min.js 568 B 0 B
build/dom/index.min.js 3.23 kB 0 B
build/edit-navigation/index.min.js 10.8 kB 0 B
build/edit-navigation/style-rtl.css 1.08 kB 0 B
build/edit-navigation/style.css 1.08 kB 0 B
build/edit-post/index.min.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.min.js 16.8 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/edit-widgets/index.min.js 9.35 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/style-rtl.css 3.78 kB 0 B
build/editor/style.css 3.78 kB 0 B
build/element/index.min.js 4.65 kB 0 B
build/escape-html/index.min.js 733 B 0 B
build/format-library/index.min.js 7.72 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.min.js 2.13 kB 0 B
build/html-entities/index.min.js 621 B 0 B
build/i18n/index.min.js 3.56 kB 0 B
build/is-shallow-equal/index.min.js 711 B 0 B
build/keyboard-shortcuts/index.min.js 2.51 kB 0 B
build/keycodes/index.min.js 1.94 kB 0 B
build/list-reusable-blocks/index.min.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.min.js 5.32 kB 0 B
build/notices/index.min.js 1.79 kB 0 B
build/nux/index.min.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.min.js 2.56 kB 0 B
build/primitives/index.min.js 1.4 kB 0 B
build/priority-queue/index.min.js 789 B 0 B
build/redux-routine/index.min.js 2.85 kB 0 B
build/rich-text/index.min.js 13.9 kB 0 B
build/server-side-render/index.min.js 2.71 kB 0 B
build/shortcode/index.min.js 1.7 kB 0 B
build/token-list/index.min.js 1.27 kB 0 B
build/url/index.min.js 4.06 kB 0 B
build/viewport/index.min.js 1.85 kB 0 B
build/warning/index.min.js 1.14 kB 0 B
build/wordcount/index.min.js 1.17 kB 0 B

compressed-size-action

@mkaz mkaz force-pushed the try/option-disable-imgeditor branch from 2c1e240 to 844e5c8 Compare July 15, 2020 18:54
@sirreal
Copy link
Member

sirreal commented Jul 16, 2020

Adding variables to the global (window) concerns me. This seems like a slippery slope.

An alternative might be to use a data store to configure this type of thing. I haven't thought through all the implications of that, but a config store might be a way of exposing pretty granular configuration to the site.

What's the handling like for experimental features? Could it be used to handling configuration of some more regular features?

@ajlende
Copy link
Contributor

ajlende commented Jul 16, 2020

The experiments page settings live in the core/block-editor store, so it's not limited to just experimental settings. Seems like a reasonable store to use to me.

@@ -131,6 +132,9 @@ export const SETTINGS_DEFAULTS = {
{ slug: 'full', name: __( 'Full Size' ) },
],

// Allow plugin to disable Image Editor if need be
disableImageEditor: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

What's if it's the opposite: imageEditing: true

Copy link
Member Author

Choose a reason for hiding this comment

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

It looks like the other block-editor settings use disable for example custom colors, font sizes, and gradients. I followed their lead on that.

Copy link
Contributor

Choose a reason for hiding this comment

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

These things may change once global styles is here. imageEditing allow for potentially having a more advanced config later (object).

Copy link
Member Author

Choose a reason for hiding this comment

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

@youknowriad I'm fine with switching. Is it ok to create it as imageEditing and keep it as a boolean, defaulting to true?

We would then be changing its type down the line if we add settings, but setting to false would work. If create it as an object, it will be empty to start, which is falsey, so tricky to test for disabled.

Copy link
Contributor

@ajlende ajlende left a comment

Choose a reason for hiding this comment

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

Seems to work as intended 👍

@mkaz mkaz force-pushed the try/option-disable-imgeditor branch from 854dba6 to ccb12e1 Compare July 16, 2020 22:43
Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

Thanks! I tested and this is working well. It makes sense to allow this more complicated feature to be disabled.

I'm not as familiar with the settings and don't have much opinion on the naming. I'm happy to let you and @youknowriad make a decision (#23966 (review)).

@mkaz mkaz added [Block] Image Affects the Image Block Backport to WP Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Jul 17, 2020
@mkaz
Copy link
Member Author

mkaz commented Jul 17, 2020

Ok, I've updated to use the imageEditing settings, right now it is just a boolean and when set to false it is disabled. If we want to switch to an object to collect settings down the line we can, maintaining a false disables the feature.

I think this should be back ported to WP 5.5 so it goes out with the release in case any one needs to disable.

Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

I've retested and this works well 👍

@youknowriad
Copy link
Contributor

youknowriad commented Jul 20, 2020

Is there a dev note planned for the image editing feature? in which case, this should probably be mentioned there?

@youknowriad youknowriad merged commit 2cc76ba into master Jul 20, 2020
@youknowriad youknowriad deleted the try/option-disable-imgeditor branch July 20, 2020 08:28
@github-actions github-actions bot added this to the Gutenberg 8.6 milestone Jul 20, 2020
@mkaz mkaz mentioned this pull request Jul 20, 2020
12 tasks
@ellatrix ellatrix mentioned this pull request Jul 20, 2020
6 tasks
@mcsf mcsf added the [Type] New API New API to be used by plugin developers or package users. label Jul 21, 2020
@youknowriad youknowriad removed the Backport to WP Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Image Affects the Image Block [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.

None yet

5 participants