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

Components: Remove redundant onClickOutside handler from Dropdown #11253

Merged
merged 1 commit into from
Oct 31, 2018

Conversation

aduth
Copy link
Member

@aduth aduth commented Oct 30, 2018

Related: #2888

This pull request seeks to simplify the Dropdown component, removing redundant handling of "click outside" behavior which is otherwise already encapsulated in the rendered Popover's onClose handling, also handled by Dropdown to perform the close action.

onClickOutside = onClose,

https://github.com/WordPress/gutenberg/blob/master/packages/components/src/popover/detect-outside.js

Testing instructions:

Verify there are no regressions in the behavior of Dropdown (e.g. Inserter), notably click-outside behavior.

@aduth aduth added [Feature] UI Components Impacts or related to the UI component system [Type] Code Quality Issues or PRs that relate to code quality labels Oct 30, 2018
@aduth aduth requested a review from youknowriad October 30, 2018 17:11
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.

Tks 👍

@aduth aduth merged commit 58725c4 into master Oct 31, 2018
@youknowriad youknowriad deleted the remove/dropdown-click-outside branch October 31, 2018 12:45
@aduth aduth added this to the 4.3 milestone Oct 31, 2018
daniloercoli added a commit that referenced this pull request Oct 31, 2018
…rnmobile/port-quote-block-step-1

* 'master' of https://github.com/WordPress/gutenberg: (21 commits)
  Fix property path on get() call (#10962)
  Fixed typos on block api documentation (#11298)
  Export `switchToBlockType` to be used mobile side when merging two blocks. (#11294)
  RichText: Remove unused `ref` assignment to RichText (#11222)
  Remove findDOMNode from Tooltip component (#11169)
  Components: Remove redundant onClickOutside handler from Dropdown (#11253)
  added myself to the contributors list (#11260)
  Add complete post type labels for Resuable Blocks (#11278)
  Increase specificity for active radio/checkbox input styling (#11290)
  Fixed "artifact" misspelling in docs. (#11291)
  Nux package: fix incorrect named deprecated import (#11283)
  Rename parentClientId to rootClientId for consistency (#11274)
  chore(release): update changelog files
  chore(release): publish
  Update plugin version to 4.2.0. (#11258)
  Data: Use turbo-combine-reducers in place of Redux (#11255)
  Revert using Icon in IconButton to avoid regression in plugin icons (pinned icons) (#11256)
  Block List: Use default Inserter for sibling insertion (#11018)
  Editor: Optimize Inserter props generation and reconciliation (#11243)
  RichText: fix format placeholder (#11102)
  ...

# Conflicts:
#	packages/block-library/src/quote/index.js
aduth added a commit that referenced this pull request Nov 8, 2018
aduth added a commit that referenced this pull request Nov 8, 2018
* Testing: Correct truthy test of immediately saveable demo

Previously assumed the selector would throw if not found, but in-fact returns `null`.

See: https://github.com/GoogleChrome/puppeteer/blob/master/docs/api.md#pageselector

* Revert "Components: Remove redundant onClickOutside handler from Dropdown (#11253)"

This reverts commit 58725c4.

* Testing: Verify Popover toggle behavior

* Components: Document Dropdown click outside behavior

* Components: Convert Dropdown container to use createRef
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] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants