Diff viewer would benefit from a diff overview bar

RESOLVED FIXED in 5.12.12

Status

defect
P5
normal
RESOLVED FIXED
9 years ago
3 years ago

People

(Reporter: kmag, Assigned: chenba)

Tracking

unspecified
5.12.12

Details

(Whiteboard: [ReviewTeam], )

Attachments

(3 attachments, 1 obsolete attachment)

Reporter

Description

9 years ago
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
Reporter

Comment 1

9 years ago
Posted image Screenshot
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [required amo-editors]
Target Milestone: --- → Q1 2011
Reporter

Comment 2

9 years ago
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.
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.
Assignee: nobody → maglione.k
Priority: -- → P5
Reporter

Comment 4

9 years ago
Posted file Implementation (obsolete) —
Implementation as a HTML fragment suitable for insertion into diff viewer headers. Should ideally be merged into diff stylesheets and external JS files.
Attachment #505608 - Attachment mime type: text/html → text/plain
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.
Assignee: maglione.k → chenba
Reporter

Comment 6

9 years ago
Posted file 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.
Attachment #505608 - Attachment is obsolete: true
Target Milestone: Q1 2011 → 5.12.12
Assignee

Comment 7

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

@kmag thanks
Attachment #515001 - Flags: review?(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.
Attachment #515001 - Flags: review?(clouserw) → review+
Assignee

Comment 9

8 years ago
Committed @ r83193
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Thanks barry.  I see this on https://addons.allizom.org/en-US/firefox/files/diff/108581/

We can push this out next Thursday.
Reclassifying editor bugs and changing to a new whiteboard flag. Spam, spam, spam, spam...
Whiteboard: [required amo-editors] → [ReviewTeam]
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.