Page MenuHomePhabricator

Visual diff: Provide a more detailed description of template changes
Closed, ResolvedPublic8 Story Points

Description

We currently just show "Template parameters changed". The complication here is that a transclusion can be made up of multiple templates (e.g. a multi-template-generated table), but as a first step we could at least just handle the single template case.

ve.dm.MWTransclusionNode.static.describeChanges = function () {
	// TODO: Provide a more detailed description of template changes
	return [ ve.msg( 'visualeditor-changedesc-mwtransclusion' ) ];
};

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 27 2018, 12:13 PM
Deskana set the point value for this task to 8.
Deskana triaged this task as Normal priority.
DLynch claimed this task.Feb 28 2018, 6:06 PM
DLynch moved this task from TR6: Visual diffs to Current work on the VisualEditor board.
DLynch edited projects, added VisualEditor (Current work); removed VisualEditor.

Change 417003 had a related patch set uploaded (by DLynch; owner: DLynch):
[mediawiki/extensions/VisualEditor@master] MWTransclusionNode: describe parameter changes for Visual Diffs

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

Change 417314 had a related patch set uploaded (by DLynch; owner: DLynch):
[VisualEditor/VisualEditor@master] DiffElement: Allow rich content for change descriptions

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

Change 417314 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] DiffElement: Allow rich content for change descriptions

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

Change 417776 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (3aabb95af)

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

Change 417776 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (3aabb95af)

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

DLynch added a comment.EditedMar 9 2018, 9:51 PM

With that richer content, we get:

Each of these list-items is the base dm.Model's describeChange output. Looking at it, I'm thinking that now HTML works in this area, it might make sense to have that describeChange wrap the to/from values in a <code> or similar, so it's easier to tell. One area where some sort of flagging would make sense would be the empty-string-but-present case, which is legitimate and comes up in templates probably more than anywhere else.

Change 420849 had a related patch set uploaded (by DLynch; owner: DLynch):
[VisualEditor/VisualEditor@master] DiffElement: Apply margin adjustment to ul as well in labels

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

Change 420849 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] DiffElement: Apply margin adjustment to ul as well in labels

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

Change 417003 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] MWTransclusionNode: describe parameter changes for Visual Diffs

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

Change 421365 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (bca1212a4)

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

Change 421365 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (bca1212a4)

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

Deskana closed this task as Resolved.Mar 23 2018, 2:14 PM
Deskana added a subscriber: Deskana.

This is really neat!

I'm thinking that now HTML works in this area, it might make sense to have that describeChange wrap the to/from values in a <code> or similar, so it's easier to tell.

T195243

One area where some sort of flagging would make sense would be the empty-string-but-present case, which is legitimate and comes up in templates probably more than anywhere else.

T195764