Page MenuHomePhabricator

Indicate when text chunks have been moved, but not changed otherwise
Closed, DeclinedPublic

Description

Motivation:
Currently of diff pages, moved paragraphs are considered a deletion and a subsequent addition. This leads to the situation that people always need to read through all moved paragraphs to make sure, that nothing has changed apart from the position. Since showing the changes comes with many technical difficulties, as a first step, the idea is to only look at moves of paragraphs, which did not change content-wise. With this implemented, users will know whether or not they need to scan the paragraph for changes, even if we can't show them yet where changes are (if they exist).

Task:
On diffs, indicate with an arrow when a paragraph has been moved and nothing else within the paragraph has changed. @Jan_Dittrich will provide a design for that.

Important:
Diff engine changes need to be implemented both for php and c++. Right now, this ticket covers both implementations. Feel free to duplicate / create sub tasks....

Background:
This is part of fulfilling wish #2 of the 2015 German-speaking community wishlist survey: T139603: Show text changes when moving text chunks (#2)

Event Timeline

Here the design:

Screenshot from 2016-08-23 09-06-48.png (565×1 px, 69 KB)

I was unsure about highlighting the moved text blue, too, for now it is still gray. If it should be easy to change for us, though.
For the unlikely case that we have overlapping moves, the top and bottom points of the vertical line of the arrow could have an offset for each added move.

Wow, the example image was so extremely confusing that it actually wound up being fascinating. Chuckle. For quite a while I couldn't find any moved text at all. Grey paired with grey says "don't waste looking at this, don't waste time thinking about this". We are actively blind to grey text paired with grey text.

Aside from the blue and yellow, I didn't see anything at all until I caught the faint arrow in the middle. That's when I realized the he: line had moved, but even then I still badly misread it. I was still completely blind to the nl: line move. The nl: is grey text paired with grey text, with no indication at all that it moved. The only reason I caught the nl: move was after my confused realization that he: text was (impossibly!) paired with something. I stopped to examine what the heck he: got paired with. You were working with a rare case where two lines switched positions. It normally isn't possible to pair moved text with anything, and in this case nl: and he: still shouldn't be paired on the same line. Text is expected to only get paired with itself.

As for the "unlikely case that we have overlapping moves": If there is one move then there is a fair chance there are multiple moves. Alphabetizing or other rearrangements easily result in massive-overlapping of moves. However I don't think you need worry much about it. 99.9% of the cognitive load is solved merely by highlighting the fact that moved-unchanged text is moved-unchanged. If a lot of stuff was moved then you're thinking about why stuff was moved, and possibly reviewing the quality of the result.

In short:
(1) There definitely needs to be highlighting of moved text, green maybe.
(2) Don't put unrelated text on the same line as moved text.

I was still completely blind to the nl: line move. The nl: is grey text paired with grey text, with no indication at all that it moved.

Sorry, that is a mistake in the mockup, nl should have not been moved.

@Alsee: You suggested to highlight the moved text, possibly with green. This is a reasonable suggestion, though I would be careful with introducing new colors (they can make the interface uneasy, and they can be difficult for (partially) colorblind people)

Alternatively: Would it make sense to you to keep the highlighting like it is now so a moved text (former position) is yellow and a moved text in its future position is blue?

''Would it make sense to you to keep the highlighting like it is now so a moved text (former position) is yellow and a moved text in its future position is blue?''

That's what we have now. The whole reason for this Phab item is that the status quo is uncomfortable for editors. Yellow says removed text, blue says added text. There is a mental expenditure to evaluate yellow/blue and re-categorize it as a move.

Probably the first and simplest thing to do is change the -/+ symbols to =/=. Technically that is a solution, but the yellow/blue still dominates the visual-skim. The colors still shout that text changed. Looking for the = symbols is a slightly awkward secondary step, it's still "reading" even if it's only reading one character, and it still requires a mental override of the initial message saying that text changed.

I'm almost tempted to say moved text could be unpaired grey text with =/=. That does say "unchanged text", but grey fades to much into unaffected text. The fact that it's unpaired grey text is just too weak of a visual indicator to comfortably signal moved text. It might be better than what we have now, but yuck.

I understand the desire to avoid new colors, and I'm in full agreement that any solution needs validation as colorblind-friendly, but I'm having a hard time seeing how to avoid a new color. The goal is for moved text to clearly announce itself as moved, when skimming a diff without reading. That means a visibly different color/size/shape/texture. I don't think we want to mess with italics or bolding or fonts or other new weird changes. Based on the pattern of how information is displayed in diffs, this really begs for a color. Grey says unaffected, yellow says removed, blue says added, and foobar would say moved.

Edit: Black might be the right answer. Black and grey send nearly the same message that the text didn't change, and black isn't new to the page. I'd like to check a mock-up of it, but I think black will work (unless someone's monitor contrast is seriously out of whack).

diff-move2.png (465×1 px, 25 KB)

I just made a mock-up with =/= and using black. I think it works.
Edit: I changed my mock-up to treat blank lines as a -/+ change. That will make much more sense in general, unless we can group the blank lines in the same box with the moved text.

@Alsee : Thanks for sharing the mockup. I'm currently busy, but I'll design a next iteration soon and will consider your suggestion.

I've been thinking about it. I'm not sure it makes sense to solve the simpler design problem for unchanged-moves first. Showing moves-with-changes presumably also needs to be solved, and that solution will probably define what the solution should/must be for the no-change case. Hopefully the ideas here can be a good starting point.

Tobi_WMDE_SW added a subscriber: jkroll.

We are going for the implementation of showing moved paragraphs with or without changes, so this task is superfluous now