Skip to content
This repository has been archived by the owner on Feb 6, 2023. It is now read-only.

Editor scrolls to bottom when pressing enter in the middle of a big paragraph #304

Closed
jainp opened this issue Apr 13, 2016 · 45 comments · May be fixed by saurabharch/guesstimate-app#5
Closed

Comments

@jainp
Copy link

jainp commented Apr 13, 2016

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
Copy link

How many characters in the paragraph?

@jainp
Copy link
Author

jainp commented Apr 15, 2016

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
Copy link

Ok, I'll check it out.

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

@jainp
Copy link
Author

jainp commented Apr 16, 2016

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 <
notifications@github.com> 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
#304 (comment)

@johanneslumpe
Copy link
Contributor

@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
Copy link
Contributor

johanneslumpe commented Apr 17, 2016

@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
Copy link

houkanshan commented Sep 29, 2016

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
Copy link

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
Copy link

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
Copy link

rezaaa commented Feb 20, 2017

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
Copy link

erisnuts commented Mar 17, 2017

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

@michelson
Copy link

michelson commented Mar 17, 2017

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
Copy link

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

@tarun-dugar
Copy link

still facing this issue :(

@markpradhan
Copy link

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

@andpor
Copy link

andpor commented May 17, 2017

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
Copy link

andpor commented May 17, 2017

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
Copy link

andpor commented May 17, 2017

@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
Copy link

andpor commented May 17, 2017

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
Copy link

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

@andpor
Copy link

andpor commented May 17, 2017

Will do

@andpor
Copy link

andpor commented May 27, 2017

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

@andpor
Copy link

andpor commented Jun 17, 2017

When is this going to get merged?

@bradbyte
Copy link

bradbyte commented Sep 6, 2017

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

@bradbyte
Copy link

bradbyte commented Sep 6, 2017

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
Copy link

andpor commented Sep 8, 2017

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
Copy link

andpor commented Oct 11, 2017

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

@andpor
Copy link

andpor commented Oct 20, 2017

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

@jackyho112
Copy link
Contributor

jackyho112 commented Nov 12, 2017

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

nj6dfwjjxg

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
Copy link
Contributor

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.

@jackyho112 jackyho112 reopened this Jan 4, 2018
ukrbublik added a commit to ukrbublik/draft-js-mod that referenced this issue Jan 4, 2018
Reworked code for adopting scroll position to new block (after pressing Enter). Deals with huge blocks nicely.
@thibaudcolas
Copy link
Contributor

thibaudcolas commented Jan 7, 2018

Here is a GIF from the Draft.js website:

draftjs-big-block-scroll-issue

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

@justinlazaro-ubidy
Copy link

Any updates?

@Arinteck
Copy link

We're steel waiting for fix. :(

@thibaudcolas
Copy link
Contributor

#1223 (comment)

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
Copy link

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

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

is fixed

@lucasvmiguel
Copy link

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

.DraftEditor-root {
  overflow: auto;
}

works

Didn't work for me... =(

@andpor
Copy link

andpor commented Apr 25, 2019

@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
Copy link
Contributor

thibaudcolas commented Apr 25, 2019

@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
Copy link
Contributor

You can take unit tests from my PR: #1598

crowicked added a commit to crowicked/draft-js that referenced this issue Oct 1, 2019
@crowicked
Copy link
Contributor

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

mmissey pushed a commit to mmissey/draft-js that referenced this issue Mar 24, 2020
Summary:
Should fix facebookarchive#304.
Thanks andpor.

**Summary**
based on Andrzej's facebookarchive#1223, issue has been there for an incredible 3.5 years. Let's fix this.
Pull Request resolved: facebookarchive#2197

Reviewed By: elboman

Differential Revision: D18314441

Pulled By: mrkev

fbshipit-source-id: 99284101bab838c574341b44b3cfcd935f1dc0e2
vilemj-Viclick pushed a commit to kontent-ai/draft-js that referenced this issue Jul 16, 2020
Summary:
Should fix facebookarchive#304.
Thanks andpor.

**Summary**
based on Andrzej's facebookarchive#1223, issue has been there for an incredible 3.5 years. Let's fix this.
Pull Request resolved: facebookarchive#2197

Reviewed By: elboman

Differential Revision: D18314441

Pulled By: mrkev

fbshipit-source-id: 99284101bab838c574341b44b3cfcd935f1dc0e2
@harvey88
Copy link

harvey88 commented Jul 2, 2021

This bug is produced if I click enter with huge text. I added text in editor, then click the enter and cursor moves to the end. https://draftjs.org/. Amount symbols 31560

alicayan008 pushed a commit to alicayan008/draft-js that referenced this issue Jul 4, 2023
Summary:
Should fix facebookarchive/draft-js#304.
Thanks andpor.

**Summary**
based on Andrzej's facebookarchive/draft-js#1223, issue has been there for an incredible 3.5 years. Let's fix this.
Pull Request resolved: facebookarchive/draft-js#2197

Reviewed By: elboman

Differential Revision: D18314441

Pulled By: mrkev

fbshipit-source-id: 99284101bab838c574341b44b3cfcd935f1dc0e2
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet