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

Refactor image block's image editing tools into separate components. #27089

Merged
merged 8 commits into from
Dec 4, 2020

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Nov 19, 2020

Description

While trying to come up with a solution to #24676, I found the image editing tools could be modularised more.

There's some really great functionality, but the UI is all in a single component that replaces almost the entire image block's UI when active:
https://github.com/WordPress/gutenberg/blob/master/packages/block-library/src/image/image-editor.js

One of the things I wanted to try is rendering the toolbar buttons in a dropdown. Dropdown has renderToggle and renderContent callback functions, and there's no way to easily have the 'Crop' button as a toggle and the various image editing components as the 'content'.

This breaks up the single component into multiple components/hooks:

  • ImageEditingProvider is a provider that should wrap all the image editing components, it holds the state for the edits.
  • useImageEditingContext is a hook that allows an individual component to access values like 'position', 'aspect' from context as well as functions for modifying those values like 'setPosition', 'setAspect'.
  • AspectRatioDropdown
  • RotationButton
  • ImageCropper
  • ZoomDropdown.

How has this been tested?

Some e2e tests have been added in a separate PR (#27262).

Manual testing also recommended.

Screenshots

Types of changes

Refactor

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.

@talldan talldan added [Type] Code Quality Issues or PRs that relate to code quality [Block] Image Affects the Image Block labels Nov 19, 2020
@talldan talldan requested a review from ajlende as a code owner November 19, 2020 09:18
@talldan talldan marked this pull request as draft November 19, 2020 09:20
@github-actions
Copy link

github-actions bot commented Nov 19, 2020

Size Change: +665 B (0%)

Total Size: 1.19 MB

Filename Size Change
build/block-library/index.js 149 kB +665 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.8 kB 0 B
build/api-fetch/index.js 3.42 kB 0 B
build/autop/index.js 2.84 kB 0 B
build/blob/index.js 665 B 0 B
build/block-directory/index.js 8.72 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/index.js 128 kB 0 B
build/block-editor/style-rtl.css 11.2 kB 0 B
build/block-editor/style.css 11.2 kB 0 B
build/block-library/editor-rtl.css 8.95 kB 0 B
build/block-library/editor.css 8.95 kB 0 B
build/block-library/style-rtl.css 8.27 kB 0 B
build/block-library/style.css 8.27 kB 0 B
build/block-library/theme-rtl.css 789 B 0 B
build/block-library/theme.css 790 B 0 B
build/block-serialization-default-parser/index.js 1.87 kB 0 B
build/block-serialization-spec-parser/index.js 3.06 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 172 kB 0 B
build/components/style-rtl.css 15.3 kB 0 B
build/components/style.css 15.3 kB 0 B
build/compose/index.js 9.95 kB 0 B
build/core-data/index.js 14.8 kB 0 B
build/data-controls/index.js 828 B 0 B
build/data/index.js 8.98 kB 0 B
build/date/index.js 11.2 kB 0 B
build/deprecated/index.js 769 B 0 B
build/dom-ready/index.js 571 B 0 B
build/dom/index.js 4.95 kB 0 B
build/edit-navigation/index.js 11.1 kB 0 B
build/edit-navigation/style-rtl.css 881 B 0 B
build/edit-navigation/style.css 885 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.42 kB 0 B
build/edit-post/style.css 6.4 kB 0 B
build/edit-site/index.js 24.1 kB 0 B
build/edit-site/style-rtl.css 3.91 kB 0 B
build/edit-site/style.css 3.91 kB 0 B
build/edit-widgets/index.js 26.3 kB 0 B
build/edit-widgets/style-rtl.css 3.13 kB 0 B
build/edit-widgets/style.css 3.13 kB 0 B
build/editor/editor-styles-rtl.css 476 B 0 B
build/editor/editor-styles.css 478 B 0 B
build/editor/index.js 43.3 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.85 kB 0 B
build/element/index.js 4.62 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/index.js 6.86 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.27 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 698 B 0 B
build/keyboard-shortcuts/index.js 2.54 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.1 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.82 kB 0 B
build/nux/index.js 3.42 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.43 kB 0 B
build/priority-queue/index.js 790 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/reusable-blocks/index.js 2.92 kB 0 B
build/rich-text/index.js 13.4 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.69 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.86 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

@talldan talldan force-pushed the try/image-editing-refactor branch from c1f4139 to 4ab1444 Compare November 30, 2020 05:11
@talldan talldan marked this pull request as ready for review November 30, 2020 09:57
@draganescu
Copy link
Contributor

I have done extensive manual testing to the image block on this branch and found no problems.

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.

The refactoring introduced no new bugs in my testing and the refactored code adds a bit more clarity (e.g. rotateClockwise instead of rotate, the separate hooks for saving and transforming etc.).

:shipit: @talldan

@talldan
Copy link
Contributor Author

talldan commented Dec 4, 2020

Thanks @draganescu!

@talldan talldan merged commit 78f6d92 into master Dec 4, 2020
@talldan talldan deleted the try/image-editing-refactor branch December 4, 2020 08:29
@github-actions github-actions bot added this to the Gutenberg 9.6 milestone Dec 4, 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] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants