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

Create unstable selectors for getting all controlled inner blocks #24835

Merged
merged 4 commits into from
Aug 31, 2020

Conversation

noahtallen
Copy link
Member

@noahtallen noahtallen commented Aug 27, 2020

closes #24690

Description

Based on discussion here and in slack, this PR moves the code which gets blocks including controlled inner blocks to separate unstable selectors. Previously, this behaviour was accessible via parameters to the normal getBlock/getBlocks selectors.

This changes nothing functionally, just cleans up the API for getBlock/getBlocks.

How has this been tested?

locally, in edit site.

Screenshots

behaviour of block navigation is the same:
image

Types of changes

Code quality.

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.

@noahtallen noahtallen added [Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P [Type] Code Quality Issues or PRs that relate to code quality [Package] Block editor /packages/block-editor labels Aug 27, 2020
@noahtallen noahtallen self-assigned this Aug 27, 2020
@github-actions
Copy link

github-actions bot commented Aug 27, 2020

Size Change: -21 B (0%)

Total Size: 1.17 MB

Filename Size Change
build/block-editor/index.js 126 kB -17 B (0%)
build/block-library/index.js 136 kB +1 B
build/blocks/index.js 47.7 kB +2 B (0%)
build/edit-navigation/index.js 11.6 kB -1 B
build/edit-site/index.js 17 kB -1 B
build/edit-widgets/index.js 11.9 kB -1 B
build/editor/index.js 45.3 kB -2 B (0%)
build/keyboard-shortcuts/index.js 2.52 kB +1 B
build/nux/index.js 3.4 kB -3 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/style-rtl.css 10.8 kB 0 B
build/block-editor/style.css 10.8 kB 0 B
build/block-library/editor-rtl.css 8.52 kB 0 B
build/block-library/editor.css 8.52 kB 0 B
build/block-library/style-rtl.css 7.47 kB 0 B
build/block-library/style.css 7.47 kB 0 B
build/block-library/theme-rtl.css 741 B 0 B
build/block-library/theme.css 742 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/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/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/style-rtl.css 3.06 kB 0 B
build/edit-site/style.css 3.06 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 492 B 0 B
build/editor/editor-styles.css 493 B 0 B
build/editor/style-rtl.css 3.81 kB 0 B
build/editor/style.css 3.81 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.57 kB 0 B
build/is-shallow-equal/index.js 710 B 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/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

} = select( 'core/block-editor' );
const selectedBlockClientId = getSelectedBlockClientId();
return {
rootBlocks: getBlocks( '', { includeControlledInnerBlocks: true } ),
rootBlocks: __unstableGetBlocksWithControlledInnerBlocks( '' ),
Copy link
Member

Choose a reason for hiding this comment

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

Would something like getBlocksTree be more clear / succinct? Not sure "with controlled inner blocks" clarifies much at a glance.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I think that would be a better approach. Indicating that this is "all blocks in the tree of the editor," rather than the blocks attached to a specific block parent. I was struggling over the naming here, so thanks for the suggestion!

@noahtallen noahtallen force-pushed the try/move-get-block-selectors-to-unstable branch from 0250907 to 1ffddc7 Compare August 28, 2020 00:01
@youknowriad youknowriad added the [Type] Performance Related to performance efforts label Aug 31, 2020
@youknowriad youknowriad force-pushed the try/move-get-block-selectors-to-unstable branch from fa6109b to 56eec24 Compare August 31, 2020 12:17
@youknowriad youknowriad merged commit c7dfc62 into master Aug 31, 2020
@youknowriad youknowriad deleted the try/move-get-block-selectors-to-unstable branch August 31, 2020 12:34
@github-actions github-actions bot added this to the Gutenberg 8.9 milestone Aug 31, 2020
@noahtallen
Copy link
Member Author

I also wonder if we can merge both __unstableGetBlockTree and __unstableGetBlockWithBlockTree in a single function.

I was thinking about how we might do this, but the consumer needed to be able to get an array of blocks with getBlockTree, and also just a standalone block object with the other one. It seemed weird to force that into a single selector 🤔

Thanks for fixing the cache key!

@youknowriad
Copy link
Contributor

I was thinking about how we might do this, but the consumer needed to be able to get an array of blocks with getBlockTree, and also just a standalone block object with the other one. It seemed weird to force that into a single selector

Not that important anyway, I was just thinking maybe an inline function for getBlock inside getBlockTree could work there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P [Package] Block editor /packages/block-editor [Type] Code Quality Issues or PRs that relate to code quality [Type] Performance Related to performance efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gutenberg editor much less responsive after writing ~300 words
3 participants