Block List: Extract scroll preservation as non-visual component #5085
Conversation
aduth
added
the
General Interface
aduth
force-pushed
the
add/scroll-reorder-preserve
branch
3 times, most recently
from
d98ba47
to
2afc4ed
Compare
| */ | ||
| restorePreviousOffset() { | ||
| const { selectionStart } = this.props; | ||
| const scrollContainer = getScrollContainer( this.node ); |
There was a problem hiding this comment.
The question I'm wondering is whether we should preserve the scrolling relatively to the scrollable parent of blockNode or the parent of this.node. I'm thinking maybe the most logical would be blockNode even if it's the same node right now.
There was a problem hiding this comment.
I'm thinking maybe the most logical would be
blockNodeeven if it's the same node right now.
Good point. Also helps simplify the component by removing the need for any return value or node binding. Updated in rebased eb63976.
| * @return {Element} Block DOM node. | ||
| */ | ||
| export function getBlockDOMNode( uid ) { | ||
| return document.querySelector( '[data-block="' + uid + '"]' ); |
There was a problem hiding this comment.
I'm seeing two other places where we use a similar technical to get the block DOM node, should we update those as well? (search for [data-block=")
There was a problem hiding this comment.
I'm seeing two other places where we use a similar technical to get the block DOM node, should we update those as well? (search for
[data-block=")
I could only find one other, but replaced it in fad28e2.
There's a reference to data-block in WritingFlow as well, but this wouldn't be as easy to replace and is proposed for removal anyways in #5289.
|
Scroll preservation you say? Is there any of this work that affects or might help #4775? |
No, I don't believe so, as the changes here are specific to reordering of blocks. |
aduth
force-pushed
the
add/scroll-reorder-preserve
branch
from
2afc4ed
to
fad28e2
Compare
This pull request seeks to extract scroll offset preserving behavior from the block component to a new non-visual component, to reflect that this behavior is not inherent to a block but rather the post editor in which it is rendered.
Implementation notes:
This pull request assumes that the behavior is specific to the post editor (
edit-post) and, as such, exposes a number of helpers which are implemented and used in the underlyingeditormodule to avoid reimplementing them.Notably, this includes a new
getBlockDOMNodehelper which, while giving me pause, seems preferable to DOM-based behaviors being stressed within the block component where they don't belong, and at least abstracts this behavior in case a future change in markup structure is made.Testing instructions:
Verify there is no regression in the behavior of preserving scroll position when rearranging a block (i.e. you should be able to repeatedly move up or down a block in a long post content).