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

Desktop: Scroll position is not remembered/preserved in Markdown Editor and Viewer #5708

Closed
ken1kob opened this issue Nov 11, 2021 · 8 comments · Fixed by #5826
Closed

Desktop: Scroll position is not remembered/preserved in Markdown Editor and Viewer #5708

ken1kob opened this issue Nov 11, 2021 · 8 comments · Fixed by #5826
Labels
bug It's a bug

Comments

@ken1kob
Copy link
Contributor

ken1kob commented Nov 11, 2021

The following related problems about Markdown Editor and Viewer have existed for a long time.
The GOAL of this issue is to ANALYZE the causes of them and to SOLVE them.

  • (A) Scroll position is not remembered, when a note selection is changed.
  • (B) Scroll position is not preserved, when the editor layout (Editor/Viewer/Split) changes.

Ref:

@ken1kob ken1kob added the bug It's a bug label Nov 11, 2021
@ken1kob
Copy link
Contributor Author

ken1kob commented Nov 11, 2021

Analysis Part 1

This analysis is the revision of a forum post, and for the versions <= 2.5.X.

At first, WYSIWYG editor has no problem. So, I talk about Markdown Editor and Viewer.

For (A) Note Selection Change:

Layout Result (remembered) Result (moved to)
Viewer Always -
Editor - Alaways, top (mostly), middle/bottom (sometimes)
Split Sometimes Otherwise, as above

For (B) Layout Change:

From To Result (preserved) Result (moved to)
Viewer Editor Always, with a gap -
Editor Viewer Sometimes, with a gap Otherwise, bottom
Split Editor Sometimes, with a gap Otherwise, bottom
Editor Split Sometimes, with a gap Otherwise, bottom
Split Viewer Always, with a gap -
Viewer Split Always, with a gap -

Since most results are the mixture of remembered/preseved and not, it is difficult to examine them. To complicate matters, if a position looked preserved, it would be nothing more than moved to somewhere near the right place. This is the reason that many reports conflict (the problem occurs or not, and it can be reproduced or not).
Anyway, the problem is still alive and not solved.

@laurent22
Copy link
Owner

One issue is that certain assets, images in particular, are loaded with a delay. So even though we save the scroll position as a percentage there can still be a shift if images are loaded after we scroll. Plugins possibly can make this even more complicated. That doesn't explain why is sometimes doesn't scroll at all though.

@ken1kob
Copy link
Contributor Author

ken1kob commented Nov 11, 2021

That doesn't explain why is sometimes doesn't scroll at all though.

I noticed that remembering scroll position after images are loaded is sometimes not performed. Because interval timer sometimes misses the completion of loading images. This missing is fixed by replacing an interval timer with a ResizeObserver in the sync-scroll PR.

However, it is only one of many causes of the problems. The problems still remains now.

@ken1kob
Copy link
Contributor Author

ken1kob commented Nov 18, 2021

Analysis Part 2

In ver. 2.6+, sync-scroll feature is added.
However, the same problems still remain in the current dev (ver. 2.6.1).
Most of the above table entries are the same, and some of them are different.
The differences come mainly from the changes of timing and protocols.

@ken1kob
Copy link
Contributor Author

ken1kob commented Nov 19, 2021

Analysis Part 3

Causes of the problems which I found are here:

  • Common:
    • [C1] Since Viewer and Editor run concurrently in different processes, a race condition may occur when they are initialized and resized.
    • [C2] During rendering and loading, the height of content is varying. When a content's height or a component's height changes, a scroll event is fired. Some of such non-user scroll events are not considered.
  • Note selection change:
    • [N1] The completion of rendering is sometimes not detected. Since scroll position is rembered at the time of the completion, it is sometimes lost.
    • [N2] Remembered scroll position is sometimes immediately updated by a bad position sent from the other side.
    • [N3] An incorrect scroll position is sometimes recorded.
  • Layout change:
    • [L1] Scroll positions are incompatible between Editor and Viewer.
    • [L2] Viewer's scroll positions are incompatible among 3 layouts (Editor/Viewer/Split).
    • [L3] Editor's scroll positions are incompatible among 3 layouts. Especially, Editor in Viewer layout has no valid scroll position.
    • [L4] When the layout changes, each of Editor and Viewer sends its scroll position to the other side without any arbitration. Sometimes, incorrect one is taken.

@ken1kob
Copy link
Contributor Author

ken1kob commented Nov 21, 2021

The current sync scroll cannot resolve Cause L1. Because to translate scroll positions, both Editor and Viewer need to be present, but by changing the layout, one of them are hidden.
Besides, the current sync scroll design also provides no solution about Causes L2 and L3.
So, a slight design change of sync scroll is required.

@ken1kob
Copy link
Contributor Author

ken1kob commented Nov 25, 2021

Approaches to Solve

The above causes are organized into the following five categories. And, for each category, its planned approach is presented. These approaches can be implemented without much effort by using the current sync scroll implementation.

Cause Category Causes Approach to Solve
Race conditions and ipc protocols C1, N2, L4 To separate scrolling functions of Editor process and Viewer process as possible, and to simplify ipc protocols
Scroll position incompatibilities L1, L2, L3 To keep GUI-independent (i.e. line-based) scroll positions
Unconsidered non-user scroll events C2 To ignore such events
Undetected completions of rendering N1 Already solved by sync-scroll
Incorrectly recorded position N3 (Under investigation)

laurent22 pushed a commit that referenced this issue Dec 15, 2021
Features:
- Scroll position is preserved when the editor layout changes.
- Scroll position is remembered when a note selection changes.

Modifications:
- The current Sync Scroll feature (in v2.6.2) is modified to use line-percent-based scroll positions.
- Scroll position translation functions, Viewer-to-Editor and Editor-to-Viewer, are separated into V2L / L2E and E2L / L2V respectively.
- The scrollmap is moved from gui/utils/SyncScrollMap.ts to note-viewer/scrollmap.js.
- IPC Protocol about the scrollmap becomes not necessary and is removed.
- Ignores non-user scroll events to avoid sync with incorrect scroll positions.
- When CodeMirror is not ready, setEditorPercentScroll() is waited.
- Fixes the bug: An incorrect scroll position is sometimes recorded.
- Since scroll positions become line-percent-based, the following incompatibilities of scroll positions are fixed:
  - Between Editor and Viewer.
  - Between Viewer Layout and Split Layout of Viewer
  - Between Editor Layout and Split Layout of Editor
@GwynethLlewelyn
Copy link

I have to say, @ken1kob, you've done an outstanding job at doing all the analysis!

Unfortunately for all of us, even after almost three years — at least in the Apple world (macOS/iOS/iPadOS) the issue has not gone nor even has been mitigated: the cursor is clearly never remembered across notes/windows/modes (markdown/editor).

I was hoping that the regression would help, since this issue was temporarily fixed at some point. I can't say exactly when — it's been far too much time ago!! — but clearly, as Joplin has been developed further, whatever worked back then has long ago been deprecated/ignored/updated, and there might be no easy way to "go back" to a time when things sort-of-worked.

Instead, it's time to look ahead to the future, and imagine what can be done to save the cursor position in a reasonable way.

I understand that the switching between notes with images, or from editor/markdown view (which will have different rendering styles, different fonts, etc.), will always mean that some degree of imprecision will be present, namely, the cursor might even be restored in its right place, but there are no guarantees that the scrollbars will position the text within the page in exactly the same position. That might be next-to-impossible to achieve.

But I'd settle with far lower expectations than that. 99% of my own notes don't have images, so I wouldn't be bothered if the odd note or two would consistently get the cursor wrong — so long as all the other text-based ones would be reasonably correct. I also don't expect pixel-perfect positioning: I'd be more than happy with having the cursor in the same position, even if the scrollbars are not. Not being an expert in frontend design, I cannot say how hard it is, given a row/column cursor position, to tell the canvas area to place that position as close to the centre as possible. It's been eons since I attempted something like that. I'm even familiar with the different way of handling the issue of "jumping around" which happens, for instance, when using a HTML page with some # anchors — not all browsers will handle the "jump" in the same way (and therefore many web designers prefer to replace the browser-native jumping with their own JavaScript). Joplin, therefore, would never be "perfect" in that sense — it would display things differently on different environments, on different screen sizes, and so forth.

I'm aware of all the above 😁 and I most certainly don't expect "perfection", not even close to that. I'd be more than happy to a least have the cursor position in the right place (even if it required to manually move the cursor up and then down to refresh the screen and position everything correctly). As it is working today, we always lose the cursor position, no matter what we do 😭

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug It's a bug
Projects
None yet
3 participants