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 Library: Implement pass-through behavior for Column block #16024

Closed
wants to merge 3 commits into from

Conversation

aduth
Copy link
Member

@aduth aduth commented Jun 6, 2019

Related: #15927
Closes #7694
Partially addresses #15660

This pull request seeks to implement a "pass-through" behavior for blocks.

It is implemented as two parts:

  • Anticipate wrapper props which disable a block's ability to be focused by bypassing the default selection behavior (allow the event to propagate to an assumed ancestor).
  • Assign as a prop of the Column block wrapper to disable focus in Column block.

With these changes, it should become impossible to select a Column block either by clicking on it, or by using arrow keys to navigate blocks. It is technically still possible to select a Column block via the Block Navigation menu. I don't personally have an issue with this, since the block is technically still present, and the primary motivation is in solving the usability problems around standard selection interactions (which this achieves).

Blocked: While this pull request is considered complete so far as implementing the pass-through behavior, it would need to be merged only in combination with a separate pull request which would recreate the existing Column attributes customizations of widths and vertical alignment assignment. My plan is to create separate pull requests for these distinct efforts, and merge them all at once only when they have each been approved.

Testing Instructions:

Verify that click and arrow key navigation to a Column block is not possible, but that otherwise behaviors are unaffected:

  • Click-through behavior implemented in Selecting Parent Blocks: Try clickthrough #15537
  • Columns selection
  • Selection of blocks within a Column
  • "Focus" selection of various block types (e.g. Paragraph, Image, Code blocks each have distinct selection behaviors worth validating)

@aduth aduth added [Status] Blocked Used to indicate that a current effort isn't able to move forward [Package] Block library /packages/block-library [Block] Columns Affects the Columns Block [Package] Block editor /packages/block-editor labels Jun 6, 2019
@aduth aduth requested a review from jasmussen June 6, 2019 19:26
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

There is one line which needs to be double checked. Otherwise, code looks good and tests well.

packages/block-editor/src/components/block-list/block.js Outdated Show resolved Hide resolved
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

I noticed that there are 2 failing e2e tests. They both seem to be related to the changes introduced.

Co-Authored-By: Grzegorz (Greg) Ziółkowski <[email protected]>
@aduth
Copy link
Member Author

aduth commented Jun 10, 2019

I noticed that there are 2 failing e2e tests. They both seem to be related to the changes introduced.

Yeah, while I've not yet looked at the specific failings, in reflection I recall there being some test cases written as assuming the column as a "stop" point for, e.g. ArrowDown. These would need to be updated since that's no longer true.

@aduth
Copy link
Member Author

aduth commented May 7, 2020

We may revisit this in the future, but for now this is not mergeable in its current state.

@aduth aduth closed this May 7, 2020
@aduth aduth deleted the try/column-passhtrough branch May 7, 2020 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Columns Affects the Columns Block [Package] Block editor /packages/block-editor [Package] Block library /packages/block-library [Status] Blocked Used to indicate that a current effort isn't able to move forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Blocks: Consider a "passthrough" supports for nested block wrappers
2 participants