Writing Flow: Remove onBottomReached handling#5271
Merged
Conversation
youknowriad
approved these changes
Feb 27, 2018
Contributor
youknowriad
left a comment
There was a problem hiding this comment.
LGTM 👍 confirmed it's still working as intended.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
insertDefaultBlockis 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.