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

Export IgnoreNestedEvents component #14707

Closed
wants to merge 1 commit into from

Conversation

swissspidy
Copy link
Member

Description

In one of our components we want to use the IgnoreNestedEvents component. We realized that it is currently not being exposed by the @wordpress/block-editor package. This PR changes that.

Types of changes

New feature (non-breaking change which adds functionality)

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.

@swissspidy swissspidy added [Type] Enhancement A suggestion for improvement. [Package] Block editor /packages/block-editor labels Mar 29, 2019
@gziolo
Copy link
Member

gziolo commented Mar 29, 2019

@youknowriad should know the reason why it wasn't part of the public API. I'm not quite sure why it is part of block-editor package in the first place rather than components :)

@youknowriad
Copy link
Contributor

This component was not exposed previously in editor. @aduth might answer better here. For me the question now is:

  • Is this worth exposing (which also means maintaining)?
  • Is this a generic component? Should it go in components ?
  • Is this the best way to expose this behavior? Could this be a hook and share in the compose package instead?

@aduth
Copy link
Member

aduth commented Apr 1, 2019

I'd also be curious if you considered event.stopPropagation() for your use-case, as it can be a simpler alternative for most usage. I don't recall specifically why this was not sufficient for use in the editor, though may have also been related to the fact that we didn't necessarily want to prevent default event behavior.

This is a pretty complex component, and one which I could expect to become a pain to maintain, particularly if React follows-through with some of their proposed events refactoring (#4751, #13525).

  • Is this a generic component? Should it go in components ?

It would be fair to say so, I think, yes.

I guess as far as making a decision here, it could be helpful to have a clearer explanation of what this component provides which is not sufficiently possible with alternatives, and whether as described it's abstracted in a way which would be adaptable to future revisions in React events behavior.

@swissspidy
Copy link
Member Author

I'd also be curious if you considered event.stopPropagation() for your use-case, as it can be a simpler alternative for most usage.

In our use case, we needed to filter the default BlockMover component in order to support dragging inner blocks freely within their parent (using absolute positioning and such). For that, we had to copy over the whole component and basically change a few lines here and there.

In BlockListBlock, the block mover is also wrapped in IgnoreNestedEvents, so we just wanted to copy the behavior there.

We use it like this:

<IgnoreNestedEvents childHandledEvents={ [ 'onDragStart', 'onMouseDown' ] }>
  { /* ... */ }
</IgnoreNestedEvents>

@swissspidy swissspidy closed this Feb 24, 2020
@aduth aduth deleted the fix/export-ignore-nested-events branch February 25, 2020 19:40
@aduth
Copy link
Member

aduth commented Feb 25, 2020

It should be noted that this component was removed from the codebase as of #19397 (mentioned also at #19397 (comment)).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Block editor /packages/block-editor [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants