Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #5280: synchronize preview with editor when scrolls #5442

Conversation

Projects
None yet
4 participants
@a1994846931931
Copy link
Contributor

commented Jun 13, 2019

Signed-off-by: Cai Xuye a1994846931931@gmail.com

Synchronize markdown preview with editor when scrolls on both sides.

Before:
fix-markdown-preview-1

After:
fix-markdown-preview-2

@akosyakov akosyakov requested a review from AlexTugarev Jun 13, 2019

@akosyakov

This comment has been minimized.

Copy link
Member

commented Jun 13, 2019

@AlexTugarev Could you review please?

@svenefftinge
Copy link
Collaborator

left a comment

Cool, thank you!

@a1994846931931 a1994846931931 force-pushed the a1994846931931:fix-markdown-preview-synchronization branch from 71ba5ff to 25c9be0 Jun 14, 2019

@a1994846931931

This comment has been minimized.

Copy link
Contributor Author

commented Jun 14, 2019

@AlexTugarev I found another related issue - synchronization may break if the preview is quickly scrolled all the way up to the top. But it seems that it already exists on the current master. I think it's better to open a new issue on this. Also, the performance of finding the source code line could be improved.

fix-markdown-preview-2-break3

@AlexTugarev

This comment has been minimized.

Copy link
Member

commented Jun 14, 2019

looks nice! reviewing right now...

simple example

2019-06-14 08 05 29

@AlexTugarev
Copy link
Member

left a comment

@a1994846931931 thanks for this PR! Very cool! It's a great improvement. 💯

⚠️ Please apply the requested changes to the implementation of getVisibleRanges.

Also, as you pointed out correctly, the sync story needs a second revision to get rid of this nasty setTimeout-based locks/unlocks.

fix #5280: synchronize preview with editor when scrolls
Signed-off-by: Cai Xuye <a1994846931931@gmail.com>
@a1994846931931

This comment has been minimized.

Copy link
Contributor Author

commented Jun 14, 2019

Also, as you pointed out correctly, the sync story needs a second revision to get rid of this nasty setTimeout-based locks/unlocks.

Yes, indeed

@a1994846931931 a1994846931931 force-pushed the a1994846931931:fix-markdown-preview-synchronization branch from 25c9be0 to c58e8b8 Jun 14, 2019

@a1994846931931

This comment has been minimized.

Copy link
Contributor Author

commented Jun 14, 2019

⚠️ Please apply the requested changes to the implementation of getVisibleRanges.

@AlexTugarev Thanks for your detailed review! Codes are updated now.

@AlexTugarev AlexTugarev merged commit f1c23e2 into theia-ide:master Jun 14, 2019

5 checks passed

DCO DCO
Details
Gitpod Open a prebuilt online workspace in Gitpod
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
eclipsefdn/eca The author(s) of the pull request is covered by necessary legal agreements in order to proceed!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.