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

Writing Flow: Avoid auto-focus on nested blocks #5092

Closed
wants to merge 1 commit into from

Conversation

aduth
Copy link
Member

@aduth aduth commented Feb 16, 2018

Related: #4872 (comment)

This pull request seeks to resolve an issue where it is difficult to use the ellipsis More Options menu for the Column block due to focus transitioning into the block. The changes here revise the logic for focusing the newly-selected block to ensure that the selected block is not within a nested block, since if it is, the selected block will change and the menu will become dismissed.

Testing instructions:

Verify that you can open the More Options ellipsis menu, both for the Columns block and a non-nesting block. In the latter case, ensure that focus is still transitioned within the block.

Verify there are no regressions in the behavior where this auto-focus behavior is necessary:

  • Block splitting
  • Block merging
  • Block replacement (paste)

@aduth aduth added [Type] Bug An existing feature does not function as intended [Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... labels Feb 16, 2018
@aduth aduth added this to the 2.2 milestone Feb 16, 2018
// Avoid selecting the target if it's:
// - The default block appender (or generally uneditable)
// - Within a nested block
if ( target.readOnly || target.closest( '[data-block]' ) !== blockContainer ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This breaks some behaviors:

  • splitting inside container blocks like "columns".
  • selecting an image and hitting Enter inside container blocks

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I can confirm this, though I wouldn't have expected it, since these behaviors should be finding inputs within the selected block, at which point it should be finding the selected nested's inputs correctly. Will investigate...

@aduth
Copy link
Member Author

aduth commented Feb 16, 2018

There's a few related issues which contribute to the breakage in nested writing flow:

  • A block's selection occurs in mousedown, whereas DefaultBlockAppender responds to click, meaning when clicking the default block appender in a columns block, the "Columns" block will become selected before the insertion, and per the logic introduced here, avoid focusing anything since at that point, the default block appender (read only) is still present in the DOM
    • Fix may be to either change DefaultBlockAppender to append onMouseDown, or change BlockListBlock's to select onClick
  • When splitting blocks in a nested paragraph, the WritingFlow logic for finding the selected block produces a null value. Presumably the newly split block is not yet in the DOM. This doesn't occur at the top-level (correctly finds the newly split block), and I'm not yet sure why there's a distinction.

@aduth
Copy link
Member Author

aduth commented Feb 16, 2018

Or we could add a setTimeout to the WritingFlow's block selection focusing 😬

@aduth
Copy link
Member Author

aduth commented Feb 16, 2018

A block's selection occurs in mousedown, whereas DefaultBlockAppender responds to click, meaning when clicking the default block appender in a columns block, the "Columns" block will become selected before the insertion, and per the logic introduced here, avoid focusing anything since at that point, the default block appender (read only) is still present in the DOM

On second thought, even though this is true, the later insertion of the default block should change the selection and trigger another WritingFlow component update...

@aduth
Copy link
Member Author

aduth commented Feb 16, 2018

In fact, I think these are the same issue: That the second component updates, but the new default block isn't yet present in the DOM, same as noted for splitting.

@youknowriad
Copy link
Contributor

youknowriad commented Feb 16, 2018

@aduth This might have an impact here #5100 It could fix some of the undesired focus calls.

@aduth
Copy link
Member Author

aduth commented Feb 16, 2018

Closed in favor of #5102

@aduth aduth closed this Feb 16, 2018
@aduth aduth deleted the fix/writing-flow-columns-menu branch February 16, 2018 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants