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

Block API: deprecate experimental Block.* component #25515

Merged
merged 7 commits into from
Sep 24, 2020

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented Sep 21, 2020

Description

With the newly introduced useBlockWrapperProps in #23034, the experimental Block.* component can be deprecated. Since plugins are likely to use this component (even though it's experimental), we should not remove it for at least a few releases.

How has this been tested?

Screenshots

Types of changes

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.

@ellatrix ellatrix added the [Feature] Block API API that allows to express the block paradigm. label Sep 21, 2020
@github-actions
Copy link

github-actions bot commented Sep 21, 2020

Size Change: -1 B

Total Size: 1.17 MB

Filename Size Change
build/block-editor/index.js 128 kB +17 B (0%)
build/block-library/index.js 135 kB -15 B (0%)
build/blocks/index.js 47.5 kB -1 B
build/components/index.js 167 kB -3 B (0%)
build/data/index.js 8.43 kB +1 B
build/editor/index.js 45.4 kB +1 B
build/rich-text/index.js 13.7 kB -1 B
ℹ️ 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/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.58 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/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/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.4 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.5 kB 0 B
build/edit-widgets/style-rtl.css 2.8 kB 0 B
build/edit-widgets/style.css 2.8 kB 0 B
build/editor/editor-styles-rtl.css 492 B 0 B
build/editor/editor-styles.css 493 B 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.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
Copy link
Member

The test failures (or at least one of them) are legitimate. I tried going through the steps of "Navigating the block hierarchy › should navigate using the block hierarchy dropdown menu" manually, and when I tried to perform the "tab twice" action on line 69, the inserter closed.

No clue what's causing that issue yet, but I'm pretty sure it's not related to the tweak in packages/block-editor/src/components/block-list/index.js, since I currently have another PR with your change copied over temporarily, and the tests aren't failing there.

@ellatrix
Copy link
Member Author

@ZebulanStanphill Yes, it's a strange issue... it seems related to inner blocks and perhaps unnecessary re-renders.

@ellatrix
Copy link
Member Author

ellatrix commented Sep 22, 2020

I'm going to leave it out of this PR and adjust it together with the introduction of an inner blocks hook.

@ellatrix ellatrix changed the title Block API: deprecate experimental Block.* component Block API: update some remaining Block.* instances Sep 22, 2020
@ellatrix ellatrix changed the title Block API: update some remaining Block.* instances Block API: deprecate experimental Block.* component Sep 22, 2020
hasInnerBlocks={ innerBlocks.length > 0 }
/>
</TagName>
<Content
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's just me but I don't see what these two components bring aside indirection.

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is that you cannot use useBlockWrapperProps without having an element that set a ref that you can add event listeners to.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it's the "return null" that causes the issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right. If you have multiple paths that are all creating a block, it would be fine to "share" the hook. Not sure if in this care we can just return a placeholder block or something. A block not returning anything is strange. There's a block, but you can't see it?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, it's probably some hidden bug somewhere there but can be handled separately tbh.

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 reverted the previous change and now it will always render a block. If the post ID is not resolved it will output "Loading..." Not sure if that's the best, but we can change the loading thing later on if needed. Imo we should always render a block even when it's still loading.

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right, good fix, thanks for the update.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ugh, some tests are failing now. :)

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

I didn't test everything here but the changes look good.

@ellatrix ellatrix force-pushed the try/deprecate-block-component branch 2 times, most recently from c4dcbc6 to ff0b08f Compare September 24, 2020 15:38
@ellatrix ellatrix merged commit 3831c51 into master Sep 24, 2020
@ellatrix ellatrix deleted the try/deprecate-block-component branch September 24, 2020 21:46
@github-actions github-actions bot added this to the Gutenberg 9.1 milestone Sep 24, 2020
@adamziel
Copy link
Contributor

Changes to column/edit.js and columns/edit.js broke drag and drop when there's a columns block present in the post editor:

Zrzut ekranu 2020-09-28 o 14 46 12

cc @ellatrix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants