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

Hide inserter's block preview when searching #23827

Merged
merged 3 commits into from
Jul 13, 2020

Conversation

ntsekouras
Copy link
Contributor

@ntsekouras ntsekouras commented Jul 9, 2020

Description

Fixes #23596

When searching for blocks to use, from the inserter menu, hovering over a block button shows a preview.

If we start typing/searching, the preview stays visible and If no blocks are found, there's no way to reset the hovered item, so the preview can't close.

Details steps to reproduce are in the related issue #23596

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.

@ntsekouras ntsekouras added [Type] Bug An existing feature does not function as intended [Feature] Inserter The main way to insert blocks using the + button in the editing interface labels Jul 9, 2020
@ntsekouras ntsekouras self-assigned this Jul 9, 2020
@ntsekouras ntsekouras requested a review from mcsf July 9, 2020 12:48
@youknowriad
Copy link
Contributor

I wonder if the right fix is more "call setHoveredItem( null )" when the hovered element gets "unmounted" as searching is just one way of unmounting the block element. That said, I'm not sure how simple is it to implement.

@github-actions
Copy link

github-actions bot commented Jul 9, 2020

Size Change: +16 B (0%)

Total Size: 1.14 MB

Filename Size Change
build/block-editor/index.js 115 kB +16 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.39 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.67 kB 0 B
build/block-directory/style-rtl.css 944 B 0 B
build/block-directory/style.css 945 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 7.6 kB 0 B
build/block-library/editor.css 7.59 kB 0 B
build/block-library/index.js 132 kB 0 B
build/block-library/style-rtl.css 7.77 kB 0 B
build/block-library/style.css 7.77 kB 0 B
build/block-library/theme-rtl.css 728 B 0 B
build/block-library/theme.css 729 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 48.2 kB 0 B
build/components/index.js 199 kB 0 B
build/components/style-rtl.css 15.9 kB 0 B
build/components/style.css 15.8 kB 0 B
build/compose/index.js 9.67 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.46 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 569 B 0 B
build/dom/index.js 3.23 kB 0 B
build/edit-navigation/index.js 10.8 kB 0 B
build/edit-navigation/style-rtl.css 1.08 kB 0 B
build/edit-navigation/style.css 1.08 kB 0 B
build/edit-post/index.js 304 kB 0 B
build/edit-post/style-rtl.css 5.59 kB 0 B
build/edit-post/style.css 5.58 kB 0 B
build/edit-site/index.js 16.6 kB 0 B
build/edit-site/style-rtl.css 3.03 kB 0 B
build/edit-site/style.css 3.03 kB 0 B
build/edit-widgets/index.js 9.35 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.1 kB 0 B
build/editor/style-rtl.css 3.78 kB 0 B
build/editor/style.css 3.78 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.72 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 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 709 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 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/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.4 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.71 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.13 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@ntsekouras
Copy link
Contributor Author

Hey @youknowriad !

If we start typing, even if the previous block is still in the filtered list, it doesn't help us on selecting it, since the block has no focus and we cannot select it ( by pressing Enter ).

We can only select a block by clicking it or with tab navigation, that also change the Previews.

So in general I'm not sure if there is any value to the user to see a previous hovered block's Preview. What do you think?

@youknowriad
Copy link
Contributor

I agree that when searching, even if we're hovering the block, it's not that important to show the preview but I'm more thinking if there are other interactions where the preview can linger because the "onMouseOut" event don't trigger. For example:

  • Hover block
  • Navigate with keyboard to the inserter tabs
  • Click arrow to switch tabs.

@ntsekouras
Copy link
Contributor Author

if there are other interactions where the preview can linger because the "onMouseOut" event don't trigger.

You are right about that! I'll look into it

@ntsekouras ntsekouras force-pushed the fix/hide-inserter-block-preview-when-typing branch from bcdf0c7 to e8456ff Compare July 13, 2020 08:48
@ntsekouras
Copy link
Contributor Author

Hey @youknowriad! What do you think of this approach?

@@ -106,6 +108,9 @@ export function BlockTypesTab( {
return result;
}, [ filteredItems, collections ] );

// hide block preview on unmount
useEffect( () => () => onUnmount(), [] );
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we just call onHover( null ) here instead of a new prop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! Thank you @youknowriad. I was thinking at first to have a handler for unMount, but since it can be achieved internally, there is no need for the extra prop for 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.

Thanks, LGTM

@ntsekouras ntsekouras merged commit c2af546 into master Jul 13, 2020
@ntsekouras ntsekouras deleted the fix/hide-inserter-block-preview-when-typing branch July 13, 2020 10:08
@github-actions github-actions bot added this to the Gutenberg 8.6 milestone Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Inserter The main way to insert blocks using the + button in the editing interface [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Block preview doesn't go away when searching for blocks
2 participants