Draft-js: Editor scrolls to bottom when pressing enter in the middle of a big paragraph

[jainp]

When you have a big paragraph and you press enter in the middle, instead of keeping the focus in the middle of the page, the editor scrolls all the way to the bottom.

Steps to reproduce:

1. Go to https://facebook.github.io/draft-js/
2. Copy paste in a large continuous paragraph without any newlines in between.
3. Scroll to the middle of the big paragraph and press enter.
4. The editor will have scrolled to the bottom

This doesn't seem to happen consistently all the time, but I've seen it come up enough times on big paragraphs.

[hellendag]

How many characters in the paragraph?

[jainp]

I tested with about 8000 characters in one paragraph. It generally occurs when the paragraph is big enough that I need to scroll to see all of it. If I scroll to the somewhere in the middle and press enter, that's when the editor scrolls to the bottom.

[hellendag]

Ok, I'll check it out.

What's the use case for an 8000-character paragraph, btw? :)

[jainp]

The 8000 was just to make the bug appear on desktop :). My use case was on
mobile where the paragraph can easily go past the visible screen area (with
much fewer characters) and the auto scrolling to the bottom becomes much
more pronounced.
On Sat, Apr 16, 2016 at 11:25 AM Isaac Salier-Hellendag <
[email protected]> wrote:

Ok, I'll check it out.

What's the use case for an 8000-character paragraph, btw? :)

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/facebook/draft-js/issues/304#issuecomment-210868675

[johanneslumpe]

@jainp @hellendag It doesn't matter what the paragraph length it. This bug occurs when a block is split and the end of the block is outside of the viewport. It can be reproduced by using very little test and just positioning the editor in a way that the paragraph isn't fully visible in the viewport.

[johanneslumpe]

@hellendag unfortunately I have no time right not to investigate this any further, but I've tracked down the issue to this line: https://github.com/facebook/draft-js/blob/master/src/model/transaction/splitBlockInContentState.js#L61

Removing the selection merging gets rid of the scrolling issue but obviously leaves the selection in the previous block. So I assume it has something to do with the way the selection is updated.

// edit
Ok some more digging: https://github.com/facebook/draft-js/blob/master/src/component/selection/setDraftEditorSelection.js#L76
That's where it breaks. Seems like modifying the selection in any way there, no matter whether it's focus or adding points, forces the scrolling change. It doesn't seem to occur with a normal contenteditable element outside of draft when modifying the selection in this way.

[houkanshan]

I think it's this line?

https://github.com/facebook/draft-js/blob/5d5f1b4b89c509d17697965ce9a0d596ed220c43/src/component/contents/DraftEditorBlock.react.js#L103

If the bottom of the focused block is outside of the viewport (scrollDelta > 0), a scrolling will be performed, and the scrolling intend to align the bottom of the block with the bottom of the viewport.

[michelson]

Hi @hellendag ,
I've made a PR #775 that fixes this issue, it fails one test case related on the calculation of scroll , before update that test case I would like to know if you would accept this change. I've tested manually and it works.
This is a really blocking bug in terms of UX imho, I hope we can solve this soon.

best

[michelson]

thinking it twice, maybe it would be a good idea to not calculate scroll deltas at all, and just scroll into the current cursor (the active element) after enter.
Any thoughts ?

[rezaaa]

I have this issue too.
I added a scroll inside editor content and i have the same issue with scroll position.

When do you plan to fix it? @hellendag

[erisnuts]

Is there any progress on this issue. It's really a lot of pain for my team. @hellendag ?

[michelson]

In my Draft integration on my text editor I've added some logic on handleReturn, it's not optimal, but at least improves the UX on this issue.
you can see the solution at: https://github.com/michelson/dante2/blob/master/src/components/dante_editor.js#L574

[hhlevnjak]

Hi, is there any progress on fixing this? It seems this is still an issue in the latest version.

[tarun-dugar]

still facing this issue :(

[markpradhan]

@hellendag This is a major issue, but looks like it is being neglected, is it ever going to be fixed?

[andpor]

This is a very serious issues, how come it is not yet addressed?

The use cases are very simple, having paragraphs with 8000 chars is not needed. I have 3 editors on one page with each tall enough for 6-7 lines of text per size of view, they are vertically scrollable. Once I have more then 7 lines of text, any edit with return pressed in the top visible lines is auto-scrolling the editor to the end. This is a showstopper for us...no user will be willing to accept this scrollbar jumping behaviour.

Please fix asap.

[andpor]

Actually I have tested this more and it looks like the behaviour of auto scrolling to end on return is related to the component hierarchy and potentially css styles. I have a bare bones draftjs editor up and running just by itself and return works fine. But as soon as I use it as a deeply nested component inside the bigger app, pressing return starts exhibiting the scroll issue. I am using draftjs 0.10.1 and 0.10.0 and React 15.4.2...

if I have 3 editors one below the other on same page.

first one behaves properly
second & third are acting up with scroll to end on RETURN...

[andpor]

@houkanshan - you are correct. That line of code causes the issue.
https://github.com/facebook/draft-js/blob/5d5f1b4b89c509d17697965ce9a0d596ed220c43/src/component/contents/DraftEditorBlock.react.js#L117

or Scroll.setTop below...

else {
      var blockBottom = blockNode.offsetHeight + blockNode.offsetTop;
      var scrollBottom = scrollParent.offsetHeight + scrollPosition.y;
      scrollDelta = blockBottom - scrollBottom;
      if (scrollDelta > 0) {
        Scroll.setTop(
          scrollParent,
          Scroll.getTop(scrollParent) + scrollDelta + SCROLL_BUFFER
        );
      }

I commented it out and now all editors everywhere work fine. The calculations or the logic is 100% incorrect.

Please fix this!

[andpor]

The fix is TRIVIAL.

change this line:
var scrollBottom = scrollParent.offsetHeight + scrollPosition.y;

to read:
var scrollBottom = scrollParent.offsetTop + scrollParent.offsetHeight + scrollPosition.y;

Why would it take so long to fix ????

[howardjing]

@andpor, if you could submit a PR with that fix, that would be great!

[andpor]

Will do

[andpor]

PR Submitted....let's see how long it will take to merge...1223

[andpor]

When is this going to get merged?

[bradbyte]

We are facing this issue as well.. please merge. Thank you!

[bradbyte]

We were able to get a work around for this by injecting overflow: auto into our root div. This caused the scrollParent to not be window, moving the logic to the 2nd block (this line)
https://github.com/facebook/draft-js/blob/0a895226c417dac43c41950be6129e667cab2755/src/component/contents/DraftEditorBlock.react.js#L118 For some reason, even without the offsetY code from the PR #1223, this style injection works. It has been an intermittent issue whenever the rendering component was in an iframe, so the style injection code is only invoked in those edge cases.

[andpor]

Almost two months have passed and this PR is still not merged...despite multiple people raising the defect...any reason why this is still not merged ?

[andpor]

4 months have passed a the PR is still open. Why has it not been merged yet ??????

[maerzhase]

@andpor as @bradbumbalough wrote on Sep 6 you can have a work-around fix until the fix is merged.

We were able to get a work around for this by injecting overflow: auto into our root div.

[michelson]

I can confirm that

.DraftEditor-root {
  overflow: auto;
}

fixes the issue

[maerzhase]

@michelson didn't test your solution — but for me

.DraftEditor-root {
  overflow: auto;
}

works

[michelson]

@maerzhase , whoops , I meant overflow: auto; works , I misstyped the style, I've corrected now

[andpor]

I think the fix has been incorporated into draftjs codebase somehow although PR does not appear as merged.

[jackyho112]

The issue seems to have been fixed now. I could not reproduce the problem right now.

Closing the issue for now. Please ask to reopen if you are still experiencing the problem. And attaching a gif to demonstrate the issue would be greatly appreciated! 😄

[ukrbublik]

This issue still exists. Can easily reproduce it on draftjs.org.
Just try to insert huge block with height > window height (better 2x height :) ) and split it with Enter.

[thibaudcolas]

Here is a GIF from the Draft.js website:

The issue seems consistently reproduceable if the paragraph is big enough. In this example, the it was 9000 chars long.

[justinlazaro-ubidy]

Any updates?

[Arinteck]

We're steel waiting for fix. :(

[thibaudcolas]

https://github.com/facebook/draft-js/pull/1223#issuecomment-427669940

From what I can see what's missing here is for someone to rebase the existing PR and fix the conflicts. @justinlazaro-ubidy @Arinteck could either of you attempt this if you want it fixed?

[huchangfa123]

set DraftEditor-root overflow auto is useful, if use css-module need add : global

:global(.DraftEditor-root) {
  overflow: auto;
}

is fixed

[lucasvmiguel]

@michelson didn't test your solution — but for me

.DraftEditor-root {
  overflow: auto;
}

works

Didn't work for me... =(

[andpor]

@lucasvmiguel not surprised.... my approach seems like the only one that really works but it has taken VERY long time to be merged....why? i do not know....

@thibaudcolas what is the latest? why is this taking so long??

[thibaudcolas]

@andpor not sure why you @-me, I have no idea. This is probably better asked on the PR than here. If you want my two cents, looking at the PR, no matter how small you think the change is I'm pretty sure it would be useful if you:

• Fixed the build. I can see two failures: https://travis-ci.org/facebook/draft-js/builds/501979548
• Described the fix in the PR, and perhaps as a comment directly in the code as well. It's pretty hard to know what to make of the change if you just describe it as "Minor code change. No impact on any other code.".
• Added some unit tests for this, so there are no regressions in the future.
• I also see you've replaced const keywords by var, which doesn't seem like a desirable change.

[ukrbublik]

You can take unit tests from my PR: https://github.com/facebook/draft-js/pull/1598

[crowicked]

Submitted a PR based on @andpor's PR, just fixed the typo/extra space.
Hope this gets merged soon.