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: Remove onBottomReached handling #5271

Merged
merged 1 commit into from Feb 27, 2018

Conversation

aduth
Copy link
Member

@aduth aduth commented Feb 26, 2018

Related: #3973

This pull request seeks to remove explicit handling of "bottom reached" within writing flow. This logic is redundant because the default writing flow behavior of transitioning focus into the next input upon reaching the vertical edge will automatically cause this to take effect, since the DefaultBlockAppender is an input:

Implementation notes:

Discovered this in course of trying to resolve a related issue with nested blocks, where the "on bottom reached" logic will not work as-is anyways since insertDefaultBlock is not being provided with the root UID of the block list in which the end has been reached.

Testing instructions:

Verify there are no regressions in creating a new default block at the end of content by pressing the Down arrow.

@aduth aduth added the [Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... label Feb 26, 2018
@aduth aduth requested a review from mcsf February 26, 2018 20:09
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.

LGTM 👍 confirmed it's still working as intended.

@aduth aduth merged commit cc87946 into master Feb 27, 2018
@aduth aduth deleted the remove/writing-flow-bottom-reached branch February 27, 2018 17:05
aduth added a commit that referenced this pull request Mar 2, 2018
Originally used for down-to-new-paragraph in #3973, removed as of #5271
aduth added a commit that referenced this pull request Mar 6, 2018
Originally used for down-to-new-paragraph in #3973, removed as of #5271
aduth added a commit that referenced this pull request Mar 7, 2018
* Writing Flow: Select block by own focus handler

* Block List: Mark default appender events as block-handled

* Writing Flow: ...

* Block: Move tabIndex to wrapper component

See: #2934
See: https://codepen.io/aduth/pen/MQxRME

Something needs to capture the focus event, which is skipped by button press on the block's toolbar, but bubbles up wrongly to the parent's edit wrapper (which has tabIndex) and causes parent block to select itself.

* Writing Flow: Update block focusable target

* Block: Remove unused block classname constant

Originally used for down-to-new-paragraph in #3973, removed as of #5271

* Block List: Capture focus event on block wrapper

Corresponding to move of tabIndex from inner edit element to wrapper, we also need to capture focus from wrapper. This way, when focus is applied via WritingFlow, the block appropriately becomes selected. This may also make "onSelect" of movers unnecessary.

* Block List: Remove pointer handling of onSelect

Can now rely on focus behavior captured from wrapper node to reflect selection onto blocks, even those without their own focusable elements

* Block List: Avoid select on focus if in multi-selection

Multi-selection causes focus event to be triggered on the block, but at that point `isSelected` is false, so it will trigger `onSelect` and a subsequent `focusTabbables`, thus breaking the intended multi-selection flow.

* Block Mover: Remove explicit select on block move

Focus will bubble to block wrapper (or in case of Firefox/Safari, where button click itself does not trigger focus, focus on the wrapper itself) to incur self-select.

* Block List: Singular select block on clicking multi-selected

This reverts commit 90a5dab.
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...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants