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 Directory: Explicitly close the inserter on block add. #24709

Merged

Conversation

StevenDufresne
Copy link
Contributor

@StevenDufresne StevenDufresne commented Aug 21, 2020

Description

This PR explicitly closes the Inserter after a block is successfully installed from the Block Directory.

Ideally, we would want the default inserter behaviour to determine whether the inserter should close or not. At the moment of this PR, the inserter closes when a block is added.

However, it appears like the async calls that are triggered when a user clicks "Install" on a block directory block do not trigger the same code paths/timings as regular blocks.

In order to solve #24606, there appear to be 2 choices:

  1. Modify popover-wrapper.js & with-focus-outside
    • There is logic that is preventing the triggering of the onClose function that closes the inserter.
  2. Explicitly close the inserter in the block directory package. (implemented here)

How has this been tested?

  1. Search for "p5"
  2. Click to install the block
  3. Notice the block is inserted and the inserter closes.

Drawbacks

As mentioned above if the main inserter goes back to not closing on block add, this will fall out of sync.

Types of changes

  • When the block is inserted successfully, trigger setIsInserterOpened and close the inserter

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.

@StevenDufresne StevenDufresne added the [Feature] Block Directory Related to the Block Directory, a repository of block plugins label Aug 21, 2020
@github-actions
Copy link

Size Change: +19 B (0%)

Total Size: 1.16 MB

Filename Size Change
build/block-directory/index.js 7.98 kB +19 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/style-rtl.css 953 B 0 B
build/block-directory/style.css 952 B 0 B
build/block-editor/index.js 126 kB 0 B
build/block-editor/style-rtl.css 10.7 kB 0 B
build/block-editor/style.css 10.7 kB 0 B
build/block-library/editor-rtl.css 8.5 kB 0 B
build/block-library/editor.css 8.5 kB 0 B
build/block-library/index.js 133 kB 0 B
build/block-library/style-rtl.css 7.42 kB 0 B
build/block-library/style.css 7.43 kB 0 B
build/block-library/theme-rtl.css 729 B 0 B
build/block-library/theme.css 730 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/blocks/index.js 47.6 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.68 kB 0 B
build/core-data/index.js 11.8 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.47 kB 0 B
build/edit-navigation/index.js 11 kB 0 B
build/edit-navigation/style-rtl.css 1.12 kB 0 B
build/edit-navigation/style.css 1.12 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/index.js 17 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.js 11.8 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/index.js 45.3 kB 0 B
build/editor/style-rtl.css 3.8 kB 0 B
build/editor/style.css 3.79 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.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.11 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.33 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.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.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

@StevenDufresne StevenDufresne marked this pull request as ready for review August 23, 2020 11:02
Copy link
Contributor

@ryelle ryelle left a comment

Choose a reason for hiding this comment

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

Working as expected in my testing — I also pulled in the change from #24610 and that test passes now too. Maybe it should be combined with this PR? (those other test failures would still be an issue)

👍 for merging this as-is if you'd rather keep the test PR separate though

@StevenDufresne
Copy link
Contributor Author

Working as expected in my testing — I also pulled in the change from #24610 and that test passes now too. Maybe it should be combined with this PR? (those other test failures would still be an issue)

👍 for merging this as-is if you'd rather keep the test PR separate though

I would like to keep that separate as the changes in #24610 have wider implications and I don't want to tie up this functionality with those updates. This PR adds an immediate fix for #24606.

@StevenDufresne StevenDufresne merged commit 150301e into master Aug 26, 2020
@StevenDufresne StevenDufresne deleted the fix/block-directory-broken-inserter-after-install branch August 26, 2020 04:05
@github-actions github-actions bot added this to the Gutenberg 8.9 milestone Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block Directory Related to the Block Directory, a repository of block plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inserter: Cannot close main inserter after installing block directory block.
2 participants