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.

Sign up for GitHub

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

Refactor reusable block edit component using hooks (and fix interactions with multiple instances of the same reusable block) #21427

Merged
merged 5 commits into from
Oct 13, 2020
Merged

Refactor reusable block edit component using hooks (and fix interactions with multiple instances of the same reusable block) #21427

merged 5 commits into from
Oct 13, 2020

Conversation

ZebulanStanphill
Copy link
Member

@ZebulanStanphill ZebulanStanphill commented Apr 6, 2020 •
edited
Loading

Description

This PR refactors the ReusableBlockEdit to use React hooks.
It also fixes two issues with the current implementation in master:

  • Editing/saving an instance of a reusable block didn't automatically update other instances in the editor.
  • Editing a reusable block, putting focus in the title field, and then pressing Esc to cancel editing the reusable block didn't work properly. (It would mess up the preview of the reusable block in the editor.)

@ZebulanStanphill ZebulanStanphill added [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) [Type] Code Quality Issues or PRs that relate to code quality Needs Technical Feedback Needs testing from a developer perspective. [Package] Block library /packages/block-library [Block] Block The "Reusable Block" Block labels Apr 6, 2020
@github-actions
Copy link

github-actions bot commented Apr 6, 2020 •
edited
Loading

Size Change: -118 B (0%)

Total Size: 1.17 MB

Filename Size Change
build/block-library/index.js 135 kB -118 B (0%)
ℹ️ 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.53 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.1 kB 0 B
build/block-editor/style.css 11.1 kB 0 B
build/block-library/editor-rtl.css 8.57 kB 0 B
build/block-library/editor.css 8.58 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/index.js 167 kB 0 B
build/components/style-rtl.css 15.5 kB 0 B
build/components/style.css 15.5 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.43 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/edit-site/index.js 20.5 kB 0 B
build/edit-site/style-rtl.css 3.54 kB 0 B
build/edit-site/style.css 3.54 kB 0 B
build/edit-widgets/index.js 17.9 kB 0 B
build/edit-widgets/style-rtl.css 2.82 kB 0 B
build/edit-widgets/style.css 2.82 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/rich-text/index.js 13.7 kB 0 B
build/server-side-render/index.js 2.61 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

@ZebulanStanphill ZebulanStanphill force-pushed the update/reusable-block-edit-hooks branch 8 times, most recently from f1b74e7 to 5d5eef5 Compare April 7, 2020 04:21
@ZebulanStanphill ZebulanStanphill changed the title [WIP] Refactor reusable block edit component using hooks. Refactor reusable block edit component using hooks. Apr 7, 2020
@ZebulanStanphill ZebulanStanphill marked this pull request as ready for review April 7, 2020 05:12
@ZebulanStanphill ZebulanStanphill force-pushed the update/reusable-block-edit-hooks branch 2 times, most recently from 7800a14 to 3773085 Compare April 7, 2020 14:20
@ZebulanStanphill ZebulanStanphill added the [Type] Bug An existing feature does not function as intended label Apr 7, 2020
@ZebulanStanphill ZebulanStanphill force-pushed the update/reusable-block-edit-hooks branch 7 times, most recently from d0c1873 to c17ca09 Compare April 13, 2020 00:14
@ZebulanStanphill
Copy link
Member Author

62852c5 fixed the last issue with this PR. It should be good to go now. 🥳

@ZebulanStanphill ZebulanStanphill added Backport to WP 6.6 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta and removed [Status] Blocked Used to indicate that a current effort isn't able to move forward Needs Technical Feedback Needs testing from a developer perspective. labels Jul 17, 2020
@ZebulanStanphill
Copy link
Member Author

I'd like a final code review prior to merge. Since the bugs this PR fixes are rather significant (in my opinion), I've added the "Backport to WP Core" label. Feel free to remove it if you disagree.

@ZebulanStanphill ZebulanStanphill force-pushed the update/reusable-block-edit-hooks branch from 62852c5 to cd545a7 Compare July 31, 2020 20:10
@ZebulanStanphill
Copy link
Member Author

Rebased. @aduth, if you have the time, could you review this again? (Your previous review is blocking.)

@aduth
Copy link
Member

aduth commented Aug 1, 2020

I don't know that I can review this right away, but I can at least dismiss my own older review 👍

@aduth aduth dismissed their stale review August 1, 2020 15:05

Outdated

@youknowriad youknowriad removed the Backport to WP 6.6 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Aug 3, 2020
@ZebulanStanphill ZebulanStanphill changed the title Refactor reusable block edit component using hooks Refactor reusable block edit component using hooks (and fix interactions with multiple instances of the same reusable block) Sep 18, 2020
@ZebulanStanphill ZebulanStanphill added this to Needs Review in WordPress 5.6.x Must Haves via automation Sep 18, 2020
@ZebulanStanphill ZebulanStanphill requested review from ellatrix and Tug and removed request for Tug October 9, 2020 01:45
@ZebulanStanphill
Copy link
Member Author

I tested this branch myself again, trying everything I could to try and break something, but I was unable to. The behavior seems to be rock solid, and the PR definitely fixes the issues in master.

In the interest of getting this included in WP 5.6, I'm going to go ahead and merge this.

@ZebulanStanphill ZebulanStanphill merged commit 2293e34 into master Oct 13, 2020
WordPress 5.6.x Must Haves automation moved this from Needs Review to Done Oct 13, 2020
@ZebulanStanphill ZebulanStanphill deleted the update/reusable-block-edit-hooks branch October 13, 2020 19:51
@github-actions github-actions bot added this to the Gutenberg 9.2 milestone Oct 13, 2020
@adamziel adamziel mentioned this pull request Nov 27, 2020
Closed
6 tasks
@fluiddot fluiddot mentioned this pull request Dec 23, 2020
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajitbohra ajitbohra Awaiting requested review from ajitbohra

@jorgefilipecosta jorgefilipecosta Awaiting requested review from jorgefilipecosta

@Soean Soean Awaiting requested review from Soean

@talldan talldan Awaiting requested review from talldan

@aduth aduth Awaiting requested review from aduth

@chrisvanpatten chrisvanpatten Awaiting requested review from chrisvanpatten

@ellatrix ellatrix Awaiting requested review from ellatrix

Assignees
No one assigned
Labels
[Block] Block The "Reusable Block" Block [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) [Package] Block library /packages/block-library [Type] Bug An existing feature does not function as intended [Type] Code Quality Issues or PRs that relate to code quality
Projects
No open projects
WordPress 5.6.x Must Haves
Done
Milestone
Gutenberg 9.2
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ZebulanStanphill @chrisvanpatten @aduth @youknowriad