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

Conversation

b6pzeusbc54tvhw5jgpyw8pwz2x6gs

In the document that has many wrapped contents, scroll handling commands (ctrl-d, ctrl-f and ctrl-e) can NOT move screen to end of document.

This PR refers to #968.
To explain the problem more details, I uploaded a video file.
video reproduce the problem

I hope it is helpful to check and review my PR.
Thank you.

# TODO: Atom v1.1.0 was released.
# But setFirstVisibleScreenRow() use getLineCount() not
# getScreenLineCount(). Once after it use getScreenLineCount(),
# [1. by setFirstVisibleScreenRow] can be changed to [2. by scrollTop]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But setFirstVisibleScreenRow() use getLineCount() not getScreenLineCount()

Thanks so much for catching this! I think that we should just fix that method. There are some issues with using the TextEditorElement::getScrollTop, because it doesn't always update until the next animation frame.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@b6pzeusbc54tvhw5jgpyw8pwz2x6gs Are you interested in submitting a PR to atom/atom fixing this? I can publish a hotfix release of Atom so that we don't need to wait for 1.7.0 to come out.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maxbrunsfeld , ok I will PR to atom/atom.

@maxbrunsfeld
Copy link
Contributor

Ok, I believed the atom/atom PR referenced above. Thanks @b6pzeusbc54tvhw5jgpyw8pwz2x6gs!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants