627112 - Diff viewer would benefit from a diff overview bar

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Admin/Editor Tools, defect, P5)

Description

[Kris Maglione [:kmag]]

User-Agent:       Mozilla/5.0 (X11; Linux x86_64; rv:2.0b10pre) Gecko/20110116 Firefox/4.0b10pre
Build Identifier: 

Currently there's no way to see how many changes a given file contains or where said changes are without scrolling the entire length of the documents. While this is often not an issue, many submissions contain files of thousands of lines with a few one- or two-line scattered throughout, which a) require significant time and effort to find and b) are easy to miss when quickly scanning the length of the document.

I propose adding an overview bar to the left of the scrollbar which marks the location of changes in the file in question. The implementation is only about a dozen lines of JavaScript and some CSS, and I suspect the feature would be well appreciated by other editors.

Screenshot attached.

Reproducible: Always

Comment 1

[Kris Maglione [:kmag]]

Attachment: Screenshot

Comment 2

[Kris Maglione [:kmag]]

I should clarify by saying I've already written the code for this and am running it locally, and if there's a good chance that it will actually get committed, I'll gladly restructure it into JavaScript and CSS fragments that can be inserted into the current diff viewer.

Comment 3

[Wil Clouser [:clouserw]]

I think the whole viewer should be rethought and was going to kick this out of Q1.  However, if you have code for what you want here, I'll ask chenb to merge it in.

Comment 4

[Kris Maglione [:kmag]]

Attachment: Implementation

Implementation as a HTML fragment suitable for insertion into diff viewer headers. Should ideally be merged into diff stylesheets and external JS files.

Comment 5

[Wil Clouser [:clouserw]]

Thanks, over to chenba.  Looks like there are some syntax errors at the end of <style> to clean up.  Remora allows inline JS/CSS so if it comes down to it you're welcome to do that.  

Security bugs are higher priority than this.

Comment 6

[Kris Maglione [:kmag]]

Attachment: Implementation

Sorry about that. The original implementation was in a JavaScript CDATA section and one of closing tags slipped through when I was filtering out unrelated data.

Attached is an updated version giving the indicators a 1px min height to prevent hunks from disappearing in very long files.

Comment 7

[Barry Chen]

Attachment: patch with code from kmag

It worked fine for me.  Code's the same as kmag's attachment 506101 [details], but separated into two locations.

@kmag thanks

Comment 8

[Wil Clouser [:clouserw]]

Comment on attachment 515001 [details] [diff] [review]
patch with code from kmag

I hope this works with our new CDN and that .js file.  We'll find out.

Comment 9

[Barry Chen]

Committed @ r83193

Comment 10

[Wil Clouser [:clouserw]]

Thanks barry.  I see this on https://addons.allizom.org/en-US/firefox/files/diff/108581/

We can push this out next Thursday.

Comment 11

[Jorge Villalobos [:jorgev] (he/him)]

Reclassifying editor bugs and changing to a new whiteboard flag. Spam, spam, spam, spam...