Page MenuHomePhabricator

Handle visual diffing of table cell merges
Open, LowPublic40 Estimated Story Points

Assigned To
None
Authored By
Esanders
Nov 2 2016, 9:35 PM
Referenced Files
F34908420: image.png
Jan 6 2022, 6:00 PM
F34908439: image.png
Jan 6 2022, 6:00 PM
F34908427: image.png
Jan 6 2022, 6:00 PM
F34908437: image.png
Jan 6 2022, 6:00 PM
F34908432: image.png
Jan 6 2022, 6:00 PM
F34908434: image.png
Jan 6 2022, 6:00 PM
F34908425: image.png
Jan 6 2022, 6:00 PM
F34908423: image.png
Jan 6 2022, 6:00 PM
Tokens
"Like" token, awarded by Amire80.

Description

Because tables are matrices and not really trees, the tree differ can show odd results for more complex operations, for example cell 'A' merged with 'E' below:

pasted_file (113×105 px, 3 KB)

Event Timeline

In general though this problem seems like potentially a lot of work for an uncommon edge case.

Another example is a row/col deletion that results a colspan/rowspan change:

Here 7,8,10 is deleted which makes 9 shorter:

pasted_file (179×150 px, 7 KB)

The output is still more legible than wikitext though.

Jdforrester-WMF set the point value for this task to 8.
Esanders changed the point value for this task from 8 to 40.Nov 11 2016, 4:43 PM

We also need to decide how we'd like table (un)merges to appear.

Unmergres can simple be shown as the insertion of all but the top left cell:

pasted_file (93×163 px, 3 KB)
->
pasted_file (93×253 px, 5 KB)

Going the other way is a little more complex, to be discussed?

pasted_file (140×213 px, 8 KB)

Agree on un-merges. Not sure what you're showing in "the other way" – is that adding things to an already merged cell?

Agree on un-merges. Not sure what you're showing in "the other way" – is that adding things to an already merged cell?

It shows merging 4 cells (in red) into 1 cell (green). The easier solution is just to not show the removed content.

Change 455321 had a related patch set uploaded (by Esanders; author: Tchanders):

[VisualEditor/VisualEditor@master] Daff table diffing

https://gerrit.wikimedia.org/r/455321

Before patchAfter patchComments
A+E merge
image.png (139×358 px, 5 KB)
image.png (128×391 px, 4 KB)
Layout less broken visually, but the removed content is not shown
Merge
image.png (172×375 px, 10 KB)
image.png (98×388 px, 5 KB)
As above
Unmerge
image.png (154×430 px, 9 KB)
image.png (116×426 px, 8 KB)
Almost no change
Row removal
image.png (201×416 px, 9 KB)
image.png (191×421 px, 9 KB)
Both quite broken. Displaying removed cells will always cause problems when colspan/rowspan is involved

I think not showing the remove content in cell merges is better than breaking the layout, and we can iterate towards the design shown above:

pasted_file (140×213 px, 8 KB)