T324759 Inline Diff: Add legend and tooltips (original) (raw)
Introduce legends and tooltips to inline diff so that users, that are not familiar with the blue and yellow highlights, can better understand the type of edits.
Feature Summary
On the current experience, new users might not be able to understand an edit. Enhance the UI so that newcomers can better understand edits.
Acceptance Criteria
- A legend that would explain the colour highlights within a diff view.
- Tooltips are triggered that indicate the type of edit when a user hover over a yellow or blue highlighted area. The tooltip popup says "Content added" for blue highlighted area and "Content deleted" for yellow highlighted area.
- Should look and work correctly with and without the VisualEditor, RevisionSlider, and MobileFrontend extensions.
User Story
- As a user of the diff tool, I want to see a legend to explain the symbology.
- As a viewer of the Unified Inline Wikitext Diff on Desktop, I will be able to see a legend so that I can understand what visual markers mean.
- As a viewer of the Unified Inline Wikitext Diff on Desktop, I will be able to see a tooltip that indicates "Content deleted" when hovering over a yellow highlighted area.
- As a viewer of the Unified Inline Wikitext Diff on Desktop, I will be able to see a tooltip that indicates "Content added" when hovering over a blue highlighted area.
Designs
Related Changes in Gerrit:
Event Timeline
There are a very large number of changes, so older changes are hidden. Show Older Changes
We are also showing the switcher when doing a diff while editing ("Show Changes"). If you try to switch to the "Visual" diff (or have it already selected) you get an "Invalid response from server" error.
This does not affect Live Preview.
Mentioned in SAL (#wikimedia-operations) [2023-05-03T07:20:13Z] <taavi@deploy1002> taavi and samwilson: Backport for [[gerrit:914429|Remove duplicated diff-mode selector in save dialog (T324759)]] synced to the testservers: mwdebug2001.codfw.wmnet, mwdebug1001.eqiad.wmnet, mwdebug1002.eqiad.wmnet, mwdebug2002.codfw.wmnet
We are also showing the switcher when doing a diff while editing ("Show Changes"). If you try to switch to the "Visual" diff (or have it already selected) you get an "Invalid response from server" error.
The above patch prevents the switcher from being shown for 'Show Changes', which avoids the issue. As far as I know, we're not planning on enabling switching visual/source while viewing changes in the wikitext editor. The switcher should only be shown when viewing a diff or in VE's save dialog when viewing changes.
Further to T324759#8722735, I tested the alignment of the legend and switcher in 5 skins with various zoom levels on:
- Safari on enwiki beta
- Firefox on arwiki beta (to test RTL behaviour)
I didn't notice any alignment issues. The switcher and legend are now vertically aligned.
I checked that they interacted fine with RevisionSlider and FlaggedRevs on dewiki beta (e.g. https://de.wikipedia.beta.wmflabs.org/w/index.php?diff=25353&oldid=25352&title=Energie&diff-type=inline).
Change 915365 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Don't show the diff-mode selector if not viewing a diff
This appears to fix the bug described in T324759#8822493. It also fixes the bug when doing a rollback or undo action.
I checked as many different types of diffs as I could find to see that the legend or switcher do not appear when they shouldn't. I tested everything here except mobile app, SpamDiffTool and WikEdDiff.
We still seem to have a bug with Translatewiki. See T336262#8843883. I will continue testing there and move this task along.
I also raised T336480, T336481 and T336482 for less urgent bugs.
Locally, I also tested:
- wiki with VisualEditor but not wikidiff2
- wiki with neither VE nor wikidiff2
Yes, I think this is right.
I do wonder if we should reorganise the patches a bit, e.g.:
- add the new DifferenceEngineBeforeDiffTable hook, but with nothing in it (so it'd not change anything);
- update VE to use the new hook, to move the existing mode switch;
- add the type toggle to core, passing it in the hook's $parts paramater;
- add the legend, also in $parts.
That way, we don't have the awkward situation of things being implemented before they can be used (the legend) or of needing to merge extension and core patches at the same time in order for them to work properly and not break the display.
Yes, I think this is right.
I do wonder if we should reorganise the patches a bit, e.g.:
- add the new DifferenceEngineBeforeDiffTable hook, but with nothing in it (so it'd not change anything);
- update VE to use the new hook, to move the existing mode switch;
- add the type toggle to core, passing it in the hook's $parts paramater;
- add the legend, also in $parts.
That way, we don't have the awkward situation of things being implemented before they can be used (the legend) or of needing to merge extension and core patches at the same time in order for them to work properly and not break the display.
As discussed during our collaboration meeting, we'll continue with this approach and revert the MobileFrontend and VE patches.
I have tested on patchdemo as many of the diffs from the list as I can (although the ~15 items from "Wikibase/WikibaseLexeme" to "EntitySchema" cannot be tested on patchdemo). I have also tested diffs of revisions from a small number of different content types (e.g. javascript, json, wikitext).
As far as I can remember, I only saw the switcher in the case of desktop or mobile diff (accessed from revision history, not during editing) or Special:ComparePages, and only if the revision on the right (in two column view) is of type wikitext. (Interestingly, you can diff revisions of different pages and content types).
I noticed a few bugs but I don't think they are related to the current work. I will raise them separately.
To be able to test more of the diffs from the list and other things like multi-content revisions, it would be useful to have this on beta cluster. Therefore, I am happy for this to be merged.
@HMonroy @dmaza I tested almost all the diffs from the list on beta. I don't think the switcher and legend are appearing in any place they should not (with the exception noted below).
On Commons beta, I was able to test Multi-Content Revisions. For example, diffs which show the mediainfo slot (https://commons.wikimedia.beta.wmflabs.org/w/index.php?title=File%3A123_4.jpg&diff=289284&oldid=280359) or both main and mediainfo (https://commons.wikimedia.beta.wmflabs.org/w/index.php?diff=289821&oldid=178021). I tested MCR revisions for as many different types of diffs as I could.
In the case where the diff only shows a mediainfo slot (e.g. https://commons.wikimedia.beta.wmflabs.org/w/index.php?title=File%3A123_4.jpg&diff=289284&oldid=280359&diff-type=inline) the legend does appear in inline mode even though we don't support inline mode for that particular slot/role, instead displaying it in two column mode (see below). Perhaps the legend can also work for the two column diff, although the shade of blue and yellow is not an exact match. Of course, the two column mode does not have the tooltips.
The switcher also appears even though Visual diffs does not seem to support mediainfo (no error is returned but no diff is displayed either), although this is pre-existing behaviour.
In the case of a diff which has both the main and mediainfo slots (e.g. https://commons.wikimedia.beta.wmflabs.org/w/index.php?diff=289821&oldid=178021&diff-type=inline) we also show the switcher and legend. This makes more sense although we only show inline diff for the main slot.
It should also be noted that the tooltips are added to crossed-out and underlined changes, as shown below (example). This happens when a paragraph is moved and words added/removed from it. @JSengupta-WMF is this OK?
I was not able to test the Translate extension, SpamDiffTool or WikEdDiff.
@GMikesell-WMF might have more to add.
Test environment: https://en.wikipedia.beta.wmflabs.org and https://commons.wikimedia.beta.wmflabs.org
- MediaWiki 1.41.0-alpha (971e2cf) 09:40, 7 June 2023.
- VisualEditor 0.1.2 (ed3f643) 23:55, 6 June 2023.
- MobileFrontend 2.4.1 (10a974e) 18:27, 6 June 2023.
Test browser: Firefox 102.
@dom_walden first two cases are ok. Color on inline and two column diffs are different and the legends are matched with the inline diff colors. This is a larger issue that needs to be addressed with the design systems team and it's in their backlog. For the third case, I'm not sure why word addition/deletion on a moved paragraph is denoted by underline/strikethrough. Is it a known problem? They should also be shown with blue/yellow highlights.
...For the third case, I'm not sure why word addition/deletion on a moved paragraph is denoted by underline/strikethrough. Is it a known problem? They should also be shown with blue/yellow highlights.
I believe it is part of the existing design. Not sure if it is specified anywhere though. I don't know the technical implications of changing it.
Change 930279 had a related patch set uploaded (by Tim Starling; author: Tim Starling):
[mediawiki/core@master] diff: Move SlotDiffRenderer::getTablePrefix() parts assembly up to DifferenceEngine
I have nothing else to add to my testing from T324759#8909457. Moving back into code review.
For the third case, I'm not sure why word addition/deletion on a moved paragraph is denoted by underline/strikethrough. Is it a known problem? They should also be shown with blue/yellow highlights.
I've made a patch to fix this, it's ready for review.