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

[WIP] Be smarter about how we line up markdown code with the live preview #704

Closed

Conversation

codeincontext
Copy link
Contributor

UNFINISHED. There's a case where the ends of the scroll panes don't line up. Submitted for review + comments

Fixes #481
Fixes #535

Currently we sync these two panes by working out how many % the user is down the left pane, and extending that ratio to the right side. This is good enough for text, but in long posts, or those with elements like images it can go askew.

These changes split both the markdown and the preview into sections (divided by headings). It determines that the user is X percent through section Y, and then applies that same scroll to the other pane.

Complications

Rather than interfacing with CodeMirror itself, it scans the HTML that CodeMirror creates. For optimisation reasons, CodeMirror pages content in and out as you scroll (see the viewportMargin option) - meaning that all the content isn't output as HTML at the same time.

To combat this, there is now a 'shadow' CodeMirror on the page which is a direct copy of the main one, except that it's full-size, invisible and used only for measurements. Regenerating the entire HTML of a CodeMirror can slow down on long documents, so this can't be regenerated with every scroll event. In order to make scrolling silky smooth, the shadow is therefore only regenerated when the user makes a change that affects the height of either pane.

It is now bi-directional - you can scroll either side. This feels awesome to use, but means that you can't scroll them independently. If for some reason our calculations screwed up, you wouldn't be able to manually scroll the preview to where it should be. I suggest that we include a little lock icon that fades in when you scroll the preview. Clicking the lock would 'unlock' the preview, so that you can move it independently. Obviously this is a design detail and is up to @JohnONolan

It's not throttled/debounced at the moment. Need to test across browsers and devices, and see whether this is needed (expect it is, even if it's with a tiny duration). Otherwise, it could be a good move to use requestAnimationFrame.

@matthojo
Copy link
Contributor

This sounds epic. 🍩

@codeincontext
Copy link
Contributor Author

Video: http://c.skatty.me/RFVr

Need to test it across a load of posts to make sure it doesn't do anything funny

@JohnONolan
Copy link
Member

@skattyadz could you rebase this onto latest master so we can test a bit more / maybe merge it for 0.3 if it's stable enough?

Fixes TryGhost#481
Fixes TryGhost#535

Currently we sync these two panes by working out how many % the user is down the left pane, and extending that ratio to the right side. This is good enough for text, but in long posts, or those with elements like images it can go askew.

These changes split both the markdown and the preview into sections (divided by headings). It determines that the user is X percent through section Y, and then applies that same scroll to the other pane.
@codeincontext
Copy link
Contributor Author

Updated for massive win

@JohnONolan
Copy link
Member

Not sure if this is going to be stable enough for release - I'm especially cautious about not being able to manually correct the scroll position when it does fuck up (eg. both panels are permanently tied)

A few notes from a few minutes use:


Notifications break it (left scrollbar gets stuck)

scrollbug1


Lots of large content blocks (eg images) at the end break it

scrollbug2

@codeincontext
Copy link
Contributor Author

That's definitely with the latest update? (thought I fixed that :( )

But I agree that it's not 100% slick. And the fact that you can't scroll independently is a problem.

@ErisDS
Copy link
Member

ErisDS commented Oct 24, 2013

@skattyadz What are your current thoughts on this? Do we need to fix & merge, or is it the wrong approach etc?

@codeincontext
Copy link
Contributor Author

Ok, so the tricky thing with this was working out the Y position of each of the headings. When we roll out a new editor, this should be easy, but for now I've done a silly hack with a 'shadow editor'. I still can't see a better way, so let's go with it.

Actionable points:

  • Need to look at John's issues, and test on a tonne of content
  • Make it one-way, or allow user to 'unlock' the two panes
  • Check X-browser support and performance on slower machines (add a throttle, etc)

I'm cool to do this, but it would be awesome if someone looked it over and make sure I'm not being silly anywhere. It's more complicated than I'd like it to be - but not much can be done about it

-a

@tomByrer
Copy link

https://stackedit.io/ seems to sync scrolling very well.

@aleemb
Copy link

aleemb commented Jan 23, 2014

Neat work around! It will be a damn shame if the article doesn't have headings though. Wouldn't it be better to create sections based on image markers instead of headings?

Also if you are creating a shadow of the left pane, would it be possible to replace the image markdown in the shadow with the images themselves (or fill it with enough blank lines based on line-height to be just as tall as the image) to create a closer approximation? This way you wouldn't need to do the sections/headings magic.

@codeincontext
Copy link
Contributor Author

I've got some uncommited code that actually doesn't use a 'shadow' at all, but uses the codemirror functions to get the height of a line. If I work out some simple regex for images, I should just as easily be able to do that I reckon.

Really gutted this is the oldest PR on this project. I'll try and pick it up once my current #{dayjob} project is shipped

@ErisDS
Copy link
Member

ErisDS commented Jan 23, 2014

We have an entire milestone (0.7) planned to deal with issues with the editor. In the meantime there are other priorities so it's fine for this PR to sit here as long as is needed.

@codeincontext
Copy link
Contributor Author

<3 <3

@CWSpear
Copy link

CWSpear commented Jan 23, 2014

@tomByrer that stackedit thing was pretty cool. It still seems to struggle a little bit with images, which seems to also be the biggest breakdown in Ghost. It will re-sync after the image, but during the image and right before, in my "very scientific" tests, it broke down a bit.

@tomByrer
Copy link

Thanks Eris!
@CWSpear cheers :) I can imagine that images would be a pain; only 1 line of Markdown that can be 100em big.
If you use a tall pix like http://dummyimage.com/100x900/000/fff.gif, the editor seems to scroll decently to me. As the image reference is around the middle of the editor, the image rendering is in the middle, but when it is near the bottom it syncs well. Near the top, it does unsync the rest of the text. So here is an idea:

  • MD image ref near top: syncs with bottom of image
  • MD image ref around middle: syncs with center of image
  • MD image ref close to bottom: syncs with image top

Maybe the best way to do this is making extra hidden spans inside the the <img> rendering, & using extra javascript logic to switch syncing refs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects:editor Work relating to the Koenig Editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editor scrolling gets stuck when typing lots of text Editor UI - Scroll events are not smooth
7 participants