Sync scrolling in side-by-side diff #1073
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A long overdue issue in the side-by-side diff view is that scrolling isn’t sync'd, so you need to manually scroll each view to be able to compare what’s happening an you move down the page. This finally adds support.
This is a little complicated for a few reasons:
First, the two side-by-side views have a different origin than the main app, posing security issues. To solve this, we use the
postMessage
API:scroll
andmessage
events.scroll
event, it packages it up and posts a message about its new scroll position to the parent window (the main app).window.postMessage()
).message
event and updates its scroll position to match.Second, large insertions or deletions often cause matching parts of the page to be in different places on either side of the diff. For sync'd scrolling to be useful, we need to keep relevant parts of the page together. To handle this, we have a concept of “landmarks” — parts of the page that are present on both sides and should line up visually when scrolling. Then we keep track of scroll positions in terms of a landmark and an offset (actually a % distance between the landmark and the next landmark) instead of an absolute pixel position. Each side of the diff keeps track of where its landmarks are on screen so when it updates its scroll position, it keeps landmarks visually side-by-side.
Ideally, the differ would output placeholders for insertions on the deletion side, and vice-versa, so the changes themselves would be landmarks. It doesn’t do that right now, so instead we look for unchanged headings and other major elements to serve as landmarks.
This first pass is pretty messy, but seems to work alright. I need to clean up all the logging, and then I think this OK to land for analysts to get some use out of it.