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

Detect focus leaving wpbody-content to clear selected block #1266

Closed
wants to merge 1 commit into from

Conversation

aduth
Copy link
Member

@aduth aduth commented Jun 19, 2017

Related: #1079

This pull request seeks to improve block clearing by detecting focus having left the #wpbody-content element. Notably, this preserves existing behavior of preserving the selected block while adjusting its settings from the Post Settings drawer, but clearing the selected block when clicking elsewhere in the admin chrome (admin bar, sidebar) or below post content (especially noticeable for new posts with few blocks).

Implementation notes:

A bit of hypocrisy here in making assumptions about ancestor DOM elements from the context of VisualEditor. I debated where best this logic should exist, since it's fairly specific to the visual editing mode. I'm open to suggestions here, and in the meantime have documented the expected behavior inline.

I'd hoped that by detecting focus leaving #wpbody-content, we could remove the logic for detecting and clearing on Escape since Escape should cause focus to move to the top-level body element. However, in my testing this did not work correctly. It may be that we'd have to force TinyMCE to blur on Escape (from Editable), but even for other block types such as Image, this behavior was not working. I might spend some more time investigating this.

Related to #1079, "canvas click" has been changed from detecting click target as one of two assigned ref nodes to an implementation where propagation is stopped from the Block component on click. I don't feel great about either of these implementations personally.

Unfortunately, this new implementation reverts to a behavior where the selected block is cleared when moving from the page to the browser Developer Tools inspector.

Testing instructions:

Verify that a selected block can be cleared by clicking above the editor column, on its sides, below the editor column, in the dashboard admin bar, or in the dashboard sidebar.

Verify that a selected block is not cleared by clicking between blocks, clicking in the editor header, or clicking in the Post Settings sidebar.

@aduth aduth added the General Interface Parts of the UI which don't fall neatly under other labels. label Jun 19, 2017
@aduth
Copy link
Member Author

aduth commented Jul 17, 2017

With multi-select, we've become more dependent on refs in the <VisualEditorBlockList /> component and it's not as easy to eliminate the ref as proposed in the approach here. This will need a re-think.

@aduth aduth closed this Jul 17, 2017
@aduth aduth deleted the update/click-outside-block-clear branch July 17, 2017 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
General Interface Parts of the UI which don't fall neatly under other labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant