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

Add support for scroll position sync #2404

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

guanglinn
Copy link
Contributor

@guanglinn guanglinn commented Sep 3, 2024

Add basic support for scroll position sync between Markdown preview (view-mode) and its source code (edit-mode).

This feature supports:

  • Sync position from edit to preview;
  • Sync position from preview to edit.

The related issues:
#2373 #2189 #1070 #1060 #679.

@guanglinn
Copy link
Contributor Author

guanglinn commented Sep 3, 2024

You can see the demonstration ↓

2024-09-03-markdown-scroll-sync.mp4

@guanglinn guanglinn changed the title Add support for scroll sync Add support for scroll position sync Sep 5, 2024
@gsantner
Copy link
Owner

gsantner commented Sep 16, 2024

has conflicts

@guanglinn
Copy link
Contributor Author

guanglinn commented Sep 17, 2024

Conflicts are resolved :)

Copy link
Owner

@gsantner gsantner left a comment

Choose a reason for hiding this comment

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

Tried locally,

unfortunately not works quite well for both directions. From View-Mode to Edit-Mode it seems to works good enough.
But from Edit to View mode it seems to not work, always jumps to top of document.
This is the builtin Markdown reference.

screenrecord-2024-09-25_00.59.29.webm

@gsantner
Copy link
Owner

I've merged your other PR to master as well, made conflicts again

@guanglinn
Copy link
Contributor Author

Conflicts are resolved now.

@guanglinn
Copy link
Contributor Author

guanglinn commented Sep 25, 2024

Sure, from Edit to View, it works well for quite a lot common text blocks such as Headings, Bold, Italic, Blockquotes, Lists, Links, Images, ..., but it works not quite well for some special text blocks.

I have tried to solve this, it's quite difficult for myself, and I think this is already a big improvement for Markor to provide a rough position synch for Markdown.

As mentioned above, you can merge it first, it is stable now, and I will improve it if I have some better solutions later, or other contributors would make it perfect on this basis.

@gsantner
Copy link
Owner

gsantner commented Sep 25, 2024

What I would suggest for now: Save/Restore last webview scroll position after short delay (when blocks have the right rendered size).

As far I recall this has been before like that, but somehow it doesn't work currently on master either

@guanglinn
Copy link
Contributor Author

guanglinn commented Sep 25, 2024

Can an option be added? Allow users to choose to let Markor restore last WebView scroll position, or sync WebView scroll position from Edit.

@gsantner
Copy link
Owner

I don't think it's neccassary. When scroll sync work good enough in praxis there is also the editor position saved/restored - so will be view-mode position when switiching to.

@gsantner
Copy link
Owner

gsantner commented Sep 25, 2024

Unfortunatley also keep in mind what I tried to say always - Markor is not only Markdown. This all relies on Markdown specific injections/adjusting the original.

This won't work for stuff that doesn't base on Markdown converter. So we need in any case such a fallback. And saving/restoring the view-mode scroll position should be viable for that.

It can't be resolved generically for everything and not easy to get much stable.

@guanglinn
Copy link
Contributor Author

guanglinn commented Sep 26, 2024

Improvements: If we want to go back to the last view position, just long click the toolbar.

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

Successfully merging this pull request may close these issues.

2 participants