Page MenuHomePhabricator
Create Task
Maniphest T242351

HTML for VisualDiff mode button appears in mobile diff pages (disabled by hack)
Open, Needs TriagePublic
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

Related Gerrit Patches:
mediawiki/extensions/MobileFrontend : masterRe-enable visual diff mode
mediawiki/extensions/VisualEditor : masterEnable visual diff code on mobile

Related Objects
Search...

StatusSubtypeAssignedTask
 DeclineddchenT118706 Conduct heuristic evaluation of image upload and insert flow in VisualEditor
 OpenNoneT115858 Design improvements for mw.ForeignStructuredUpload.BookletLayout
 OpenNoneT115865 Insert image in content immediately after it's uploaded, skipping the "General settings" step
 DuplicateNoneT115864 Figure out if the description of the image can be used as the caption on-wiki
 OpenNoneT53032 When inserting an image, set its caption by default to be the Commons image description
 ResolvedAbitT122381 Provide a way to curate and fetch titles (short descriptions) for media files
 OpenNoneT12863 Links on commons upload summaries do not link to commons
 OpenNoneT5498 Image history is confusing
 OpenNoneT96384 Integrate file revisions with description page history
 OpenNoneT39534 Wikimedia Commons should support searching by color
 DuplicateNoneT39535 Wikimedia Commons should support filtering by color
 OpenNoneT19503 Provide metadata support on Wikimedia Commons
 OpenNoneT122038 Moving/Deleting a video file does not move/delete subtitles files, nor does it inform the file mover of their existence
 OpenNoneT135221 Make TimedText content an integral part of the File page
 OpenNoneT2167 Use a dedicated interface for adding meta-data like interwiki links, rather than wikitext
 OpenNoneT132072 Integrate page meta-data as a new content model revision slot for consistency and atomicity
 ResolvedppelbergT54448 Transclusion dialog should not suggest documentation sub pages as templates
 ResolvedKrinkleT52512 TemplateData: Page properties should not be stored twice (both Template and Template/doc)
 OpenNoneT56140 Move TemplateData to its own JSON-content namespace and associate with Template-namespace, or to its own TemplateData content model and revision slot
 OpenNoneT51662 VisualEditor: Use Multimedia/Wikidata's proposed rich structured meta-data in the image insertion dialog
 OpenNoneT68108 [Epic] Store media information for files on Wikimedia Commons as structured data
 OpenNoneT174043 Deploy Multi-Content Revisions
 OpenNoneT203157 Make the spam whitelist its own slot
 OpenNoneT174022 Implement multi-content revisions
 OpenNoneT189220 Ensure that relevant extensions are MCR-aware
 OpenNoneT202763 Update extensions which customize content diff rendering
 StalledNoneT94370 Implement a mechanism that recognizes syntax errors of wikicode as a Huggle extension
 ResolvedPetrbT57793 Scoring should perhaps take into account added text rather than all text
 OpenNoneT105518 Make Wiki Labels mobile compatible
OpenNoneT104072 Implement single column diff option (from mobile app)
 OpenNoneT100082 Provide useful diffs to high-volume consumers of recent changes
 OpenNoneT56328 Provide intraline diff format in API action=compare
 OpenNoneT162210 Upstream MobileFrontend components to core and other extensions (tracking)
 OpenNoneT238507 MobileFrontend sometimes goes to diff page instead of pending changes review page
 OpenNoneT117279 [EPIC] Core should provide inline diffs as well as side by side (Move InlineDifferenceEngine into core / remove MobileDiff)
 ResolvedovasilevaT242310 Regression: issues with MobileDiff
 OpenmarcellaT242351 HTML for VisualDiff mode button appears in mobile diff pages (disabled by hack)
 OpenmatmarexT244673 VisualDiff feature on mobile has an inconsistent style/layout
 OpenBUG REPORTNoneT245172 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
Masumrezarock100 added a comment.Jan 9 2020, 5:34 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
Masumrezarock100 added a comment.Jan 10 2020, 3:21 AM

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
matmarex added a subscriber: matmarex.Wed, Jan 29, 6:58 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.

gerritbot added a comment.Wed, Jan 29, 7:15 PM

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

gerritbot added a project: Patch-For-Review.Wed, Jan 29, 7:15 PM

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

gerritbot added a comment.Wed, Jan 29, 8:32 PM

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

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

gerritbot added a comment.Wed, Jan 29, 8:35 PM

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

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

Masumrezarock100 added a comment.EditedWed, Jan 29, 8:55 PM
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?

Masumrezarock100 added a subscriber: alexhollender.EditedWed, Jan 29, 9:05 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?

matmarex added a comment.Wed, Jan 29, 9:17 PM
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.

Masumrezarock100 added a comment.EditedWed, Jan 29, 9:39 PM

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

Jdlrobson added a comment.Wed, Jan 29, 11:51 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.

alexhollender added a comment.Tue, Feb 4, 9:49 PM
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 assigned this task to marcella.Thu, Feb 6, 11:26 PM
JTannerWMF added a subscriber: JTannerWMF.

I believe this is ready for your sign off.

Masumrezarock100 added a comment.Sun, Feb 9, 9:01 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.Wed, Feb 12, 5:24 PM
Content licensed under Creative Commons Attribution-ShareAlike 3.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL