HTML for VisualDiff mode button appears in mobile diff pages (disabled by hack)
Closed, ResolvedPublic
Actions

Description

As the diffs for mobile and desktop site converge one of the side effects was the visual diff control now loads in the mobile diff sans the javascript that makes it work.

The visual diff control should check to see if it is in mobile mode (using MobileContext::shouldDisplayMobileView if available) or should integrate itself into the mobile site.

For the time being it is being hidden via hack in MobileFrontend in resources/mobile.special.mobilediff.styles/mobilediff.less.
https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/master/resources/mobile.special.mobilediff.styles/mobilediff.less#L9

Acceptance criteria

The control is either conditionally added or added and works.
MobileFrontend hack removed

Details

	Project	Branch	Lines +/-	Subject
	mediawiki/extensions/MobileFrontend	master	+0 -5	Re-enable visual diff mode
	mediawiki/extensions/VisualEditor	master	+8 -0	Enable visual diff code on mobile

Customize query in gerrit

Related Objects
Search...

Status	Subtype	Assigned	Task
Declined		dchen	T118706 Conduct heuristic evaluation of image upload and insert flow in VisualEditor
Open		None	T115858 Design improvements for mw.ForeignStructuredUpload.BookletLayout
Open		None	T115865 Insert image in content immediately after it's uploaded, skipping the "General settings" step
Duplicate		None	T115864 Figure out if the description of the image can be used as the caption on-wiki
Open		None	T53032 When inserting an image, set its caption by default to be the Commons image description
Open		None	T12863 Links on commons upload summaries do not link to commons
Open		None	T5498 Image history is confusing
Stalled		None	T96384 Integrate file revisions with description page history
Duplicate		None	T39535 Wikimedia Commons should support filtering by color
Open		None	T39534 Wikimedia Commons should support searching by color
Open		None	T19503 Provide metadata support on Wikimedia Commons
Open		None	T122038 Moving/Deleting a video file does not move/delete subtitles files, nor does it inform the file mover of their existence
Open		None	T135221 Make TimedText content an integral part of the File page
Open		None	T2167 Use a dedicated interface for adding meta-data like interwiki links, rather than wikitext
Open		None	T132072 Integrate page meta-data as a new content model revision slot for consistency and atomicity
Resolved		ppelberg	T54448 Transclusion dialog should not suggest documentation sub pages as templates
Resolved		Krinkle	T52512 TemplateData: Page properties should not be stored twice (both Template and Template/doc)
Open		None	T56140 Move TemplateData to its own JSON-content namespace and associate with Template-namespace, or to its own TemplateData content model and revision slot
Open		None	T51662 VisualEditor: Use Multimedia/Wikidata's proposed rich structured meta-data in the image insertion dialog
Open		None	T68108 [Epic] Store media information for files on Wikimedia Commons as structured data
Resolved		Halfak	T252280 Improve Wikilabels UI
Open		None	T105518 Make Wiki Labels mobile compatible
Open		None	T104072 Implement single column diff option (from mobile app)
Stalled		None	T174043 Deploy Multi-Content Revisions
Open		None	T203157 Make the spam whitelist its own slot
Open		None	T174022 Implement multi-content revisions
Open		None	T189220 Ensure that relevant extensions are MCR-aware
Open		None	T202763 Update extensions which customize content diff rendering
Open		None	T94370 Implement a mechanism that recognizes syntax errors of wikicode as a Huggle extension
Resolved		Petrb	T57793 Scoring should perhaps take into account added text rather than all text
Open		None	T100082 Provide useful diffs to high-volume consumers of recent changes
Open		None	T56328 Provide intraline diff format in API action=compare
Open		None	T162210 [EPIC] Upstream MobileFrontend components to core and other extensions (tracking)
Open		None	T238507 MobileFrontend sometimes goes to diff page instead of pending changes review page
Open		None	T117279 [EPIC] Core should provide inline diffs as well as side by side (Move InlineDifferenceEngine into core / remove MobileDiff)
Resolved		ovasileva	T242310 Regression: issues with MobileDiff
Resolved		marcella	T242351 HTML for VisualDiff mode button appears in mobile diff pages (disabled by hack)
Open		None	T244673 VisualDiff feature on mobile has an inconsistent style/layout
Resolved	BUG REPORT	Esanders	T245172 VisualDiff toggle buttons appear in mobile when the relevant page doesn't have wikitext content model

Event Timeline

Jdlrobson created this task.Jan 9 2020, 5:12 PM

Restricted Application added a project: VisualEditor. · View Herald TranscriptJan 9 2020, 5:12 PM

Jdlrobson updated the task description. (Show Details)Jan 9 2020, 5:13 PM

Jdlrobson renamed this task from VisualDiff mode button appears in diff pages (disabled) to HTML for VisualDiff mode button appears in mobile diff pages (disabled).Jan 9 2020, 5:26 PM

VisualDiff feature doesn't work without JS, so why not load the button using JS? It doesn't make any sense.

Jdlrobson updated the task description. (Show Details)Jan 9 2020, 5:42 PM

Question: @Jdlrobson do you plan to work on this? Or the assigned field was just inherited from the parent task?

Jdlrobson renamed this task from HTML for VisualDiff mode button appears in mobile diff pages (disabled) to HTML for VisualDiff mode button appears in mobile diff pages (disabled by hack).Jan 10 2020, 5:11 PM

Jdlrobson removed Jdlrobson as the assignee of this task.

Jdlrobson updated the task description. (Show Details)

Jdlrobson moved this task from Needs triage to Triaged on the Mobile board.Jan 10 2020, 6:01 PM

In T242351#5790208, @Masumrezarock100 wrote:

VisualDiff feature doesn't work without JS, so why not load the button using JS? It doesn't make any sense.

This way we avoid the contents of the page moving around when JS code loads (it only changes the button from enabled to disabled). The button is supposed to be hidden when JS is disabled, but the code for that doesn't get loaded on mobile.

Change 568556 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] Enable visual diff code on mobile

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

Change 568557 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/MobileFrontend@master] Re-enable visual diff mode

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

matmarex edited projects, added VisualEditor (Current work); removed VisualEditor.Jan 29 2020, 7:16 PM

matmarex moved this task from Incoming to Code review on the VisualEditor (Current work) board.

Change 568556 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Enable visual diff code on mobile

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

Change 568557 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Re-enable visual diff mode

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

In T242351#5838862, @matmarex wrote:

This way we avoid the contents of the page moving around when JS code loads (it only changes the button from enabled to disabled). The button is supposed to be hidden when JS is disabled, but the code for that doesn't get loaded on mobile.

Thanks for explaining that. But the above patches looks like they will load the JS on mobile. Can we be sure that the VisualDiff feature will work properly on tall and small screens?

ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.18; 2020-02-04).Jan 29 2020, 9:01 PM

This is working. But the design looks bad on mobile. Leaving this to @alexhollender for review. Could you take a look at https://en.m.wikipedia.beta.wmflabs.org/wiki/Special:MobileDiff/411223...412204?diffmode=visual (with VisualDiff beta feature enabled) and check if this design is okay?

In T242351#5838967, @Masumrezarock100 wrote:

Thanks for explaining that. But the above patches looks like they will load the JS on mobile. Can we be sure that the VisualDiff feature will work properly on tall and small screens?

It works okay, no worse than visual editor. It's probably pretty bad in some cases, but there's nothing we can do if the content itself is not properly made for mobile.

In T242351#5838982, @Masumrezarock100 wrote:

This is working. But the design looks bad on mobile.

If you mean just the switcher buttons, then it seems Jon and the gang have some further plans to redesign it, see parent task T240608: Standardise a control for switching diff types from side by side to inline/visual. It should be easier to work on that now that we fixed the issue here.

matmarex moved this task from Code review to Product owner review on the VisualEditor (Current work) board.Jan 29 2020, 9:17 PM

matmarex removed a project: Patch-For-Review.

The switched buttons looks fine to me but I am worried about the diff itself. The different colors in diffs don't look good at all. VisualDiff was designed for the desktop side by side diff. The design looks on mobile because mobile only shows inline (unified) diffs which was recently introduced to core, and this extension is not optimized for such diff design.

Screenshot of https://en.m.wikipedia.beta.wmflabs.org/wiki/Special:MobileDiff/411223...412204?diffmode=visual

Masumrezarock100 mentioned this in T242310: Regression: issues with MobileDiff.Jan 29 2020, 9:48 PM

Given its a beta feature I am not concerned about these issues right now but please open a new ticket. Possibly hiding the button may be preferable in the meantime but there is also value to displaying something in beta in a broken form.

In T242351#5838982, @Masumrezarock100 wrote:

This is working. But the design looks bad on mobile. Leaving this to @alexhollender for review. Could you take a look at https://en.m.wikipedia.beta.wmflabs.org/wiki/Special:MobileDiff/411223...412204?diffmode=visual (with VisualDiff beta feature enabled) and check if this design is okay?

It seems like there might be some other layout issues with the page, but as far as I can see the toggle button looks fine:

JTannerWMF added a project: Editing-team (Q3 2019-2020 Kanban Board).Feb 4 2020, 10:15 PM

JTannerWMF moved this task from Needs Discussion / Investigation to Ready for Sign Off on the Editing-team (Q3 2019-2020 Kanban Board) board.Feb 4 2020, 10:17 PM

matmarex edited projects, added VisualEditor; removed VisualEditor (Current work).Feb 5 2020, 12:52 AM

I believe this is ready for your sign off.

Masumrezarock100 mentioned this in T244673: VisualDiff feature on mobile has an inconsistent style/layout.Feb 9 2020, 8:56 PM

In T242351#5839266, @Jdlrobson wrote:

Given its a beta feature I am not concerned about these issues right now but please open a new ticket. Possibly hiding the button may be preferable in the meantime but there is also value to displaying something in beta in a broken form.

Created T244673.

matmarex moved this task from To Triage to Triaged on the VisualEditor board.Feb 12 2020, 5:24 PM

marcella closed this task as Resolved.Mar 4 2020, 5:17 PM

Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptMar 4 2020, 5:17 PM

matmarex closed subtask T245172: VisualDiff toggle buttons appear in mobile when the relevant page doesn't have wikitext content model as Resolved.Thu, Sep 17, 6:18 PM

HTML for VisualDiff mode button appears in mobile diff pages (disabled by hack)Closed, ResolvedPublicActions