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

Fix combobox suggestion list closure when clicking scrollbar #27367

Merged
merged 1 commit into from
Dec 1, 2020

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Nov 30, 2020

Description

First attempt at fixing #27283.

The bug is caused by ComboxBoxControl hiding the suggestions list whenever its input is blurred, e.g. when clicking on something in the suggestions list, a sibling HTML element.

This PR uses withFocusOutside to check for blur of the entire control rather than just the input.

The downside of withFocusOutside is that it adds an extra wrapping div around the component, which is kind of nasty. But in testing didn't seem to cause any styling issues.

I've separately looked at refactoring withFocusOutside to a hook (#27369) so that we can lose the wrapping div, which I've had some success with, but this would then require lots of changes to BaseControl to accept various event handler props from the hook. Quite a big change for a backport to a WordPress 5.6 release candidate.

How has this been tested?

  1. Go to the page editor
  2. Activate the parent page selector
  3. Click on scrollbar to scroll down or up : the selector closes itself

Screenshots

Types of changes

Bug fix (non-breaking change which fixes an issue)

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.

@talldan talldan added [Feature] UI Components Impacts or related to the UI component system [Type] Regression Related to a regression in the latest release labels Nov 30, 2020
@talldan talldan self-assigned this Nov 30, 2020
@talldan talldan linked an issue Nov 30, 2020 that may be closed by this pull request
@github-actions
Copy link

Size Change: +59 B (0%)

Total Size: 1.19 MB

Filename Size Change
build/blocks/index.js 48.1 kB +1 B
build/components/index.js 172 kB +59 B (0%)
build/core-data/index.js 14.8 kB +1 B
build/data/index.js 8.98 kB +1 B
build/edit-post/index.js 306 kB -1 B
build/element/index.js 4.63 kB +1 B
build/rich-text/index.js 13.4 kB -3 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.8 kB 0 B
build/api-fetch/index.js 3.42 kB 0 B
build/autop/index.js 2.84 kB 0 B
build/blob/index.js 665 B 0 B
build/block-directory/index.js 8.72 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.2 kB 0 B
build/block-editor/style.css 11.2 kB 0 B
build/block-library/editor-rtl.css 8.95 kB 0 B
build/block-library/editor.css 8.95 kB 0 B
build/block-library/index.js 148 kB 0 B
build/block-library/style-rtl.css 8.27 kB 0 B
build/block-library/style.css 8.27 kB 0 B
build/block-library/theme-rtl.css 789 B 0 B
build/block-library/theme.css 790 B 0 B
build/block-serialization-default-parser/index.js 1.87 kB 0 B
build/block-serialization-spec-parser/index.js 3.06 kB 0 B
build/components/style-rtl.css 15.3 kB 0 B
build/components/style.css 15.3 kB 0 B
build/compose/index.js 9.95 kB 0 B
build/data-controls/index.js 827 B 0 B
build/date/index.js 11.2 kB 0 B
build/deprecated/index.js 769 B 0 B
build/dom-ready/index.js 571 B 0 B
build/dom/index.js 4.95 kB 0 B
build/edit-navigation/index.js 11.1 kB 0 B
build/edit-navigation/style-rtl.css 881 B 0 B
build/edit-navigation/style.css 885 B 0 B
build/edit-post/style-rtl.css 6.42 kB 0 B
build/edit-post/style.css 6.4 kB 0 B
build/edit-site/index.js 24.1 kB 0 B
build/edit-site/style-rtl.css 3.91 kB 0 B
build/edit-site/style.css 3.91 kB 0 B
build/edit-widgets/index.js 26.3 kB 0 B
build/edit-widgets/style-rtl.css 3.13 kB 0 B
build/edit-widgets/style.css 3.13 kB 0 B
build/editor/editor-styles-rtl.css 476 B 0 B
build/editor/editor-styles.css 478 B 0 B
build/editor/index.js 43.3 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.85 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/index.js 6.86 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.27 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 698 B 0 B
build/keyboard-shortcuts/index.js 2.54 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.1 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.82 kB 0 B
build/nux/index.js 3.42 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.43 kB 0 B
build/priority-queue/index.js 790 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/reusable-blocks/index.js 2.93 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.69 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.86 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

Copy link
Contributor

@tellthemachines tellthemachines left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! It's working well; I also tested keyboard navigation as well as screen reader interaction on VoiceOver and NVDA and nothing seems broken.

@tellthemachines tellthemachines added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Dec 1, 2020
@tellthemachines tellthemachines merged commit f900f7c into master Dec 1, 2020
@tellthemachines tellthemachines deleted the fix/combobox-control-focus-management branch December 1, 2020 03:23
@github-actions github-actions bot added this to the Gutenberg 9.6 milestone Dec 1, 2020
tellthemachines added a commit that referenced this pull request Dec 1, 2020
* Provide a minimum of code wrapping for the code block. (#26623)

* Block Support: Fix font size style when applying block support (#26762)

* Fix Separator editor styles (#27071)

* Fix the Post author selector for contributors (#26554)

Co-authored-by: Riad Benguella <[email protected]>

* Align single half width column to left (#27142)

* remove the auto margin for individual column blocks

* update margin values for blocks in blocks to zero insted of auto

* Add backward compatibility support for lightBlockWrapper in getSaveElement (#27189)

* Code block: paste plain text (#27236)

* paste plain text option

* Add e2e test

* Fix crash when null date passed to TimePicker (#27316)

* Fix crash when null date passed.

* Update test

* Fix GH actions "cancel" step (#27025)

* use new syntax for setting env var

* Update package-lock

* Update package-lock again

* Remove the button only option from the UI until it can be wired up to something that works in the front end. (#27379)

* Fix combobox csuggestion list closure when clicking scrollbar (#27367)

Co-authored-by: Joen A <[email protected]>
Co-authored-by: Aaron Robertshaw <[email protected]>
Co-authored-by: Nik Tsekouras <[email protected]>
Co-authored-by: Adam Silverstein <[email protected]>
Co-authored-by: Riad Benguella <[email protected]>
Co-authored-by: andrei draganescu <[email protected]>
Co-authored-by: Daniel Richards <[email protected]>
Co-authored-by: Ella van Durpe <[email protected]>
Co-authored-by: Noah Allen <[email protected]>
Co-authored-by: Andy Peatling <[email protected]>
@tellthemachines tellthemachines mentioned this pull request Dec 1, 2020
6 tasks
@tellthemachines tellthemachines removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Dec 1, 2020
@talldan talldan mentioned this pull request Dec 7, 2020
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] UI Components Impacts or related to the UI component system [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using the scrollbar of the parent page selector closes the selector.
2 participants