Page MenuHomePhabricator

Disable VE editing of maplink / mapframe when the editor is not able to
Closed, ResolvedPublic0 Story Points

Description

There are a few mapframe/maplink configurations that Visual Editor doesn't support yet.

  • Maplink and mapframe auto-positioning
    • absence of longitude / latitude / zoom parameters
Raw editorVE inline editingVE map dialogExpected result
  • Editing non-static map features
    • when GeoJSON contains ExternalData blocks
Raw editorVE inline editingVE map dialogExpected result
  • Wikivoyage tags using group and show parameters

Currently we are never blocking the user from entering in the Visual Editor mode, although the editor experience can be awkward and frustrating in these situations, and can lead to unexpected results, broken map, and data loss. When we know the editor is not working properly, we should at the very least warn users that the editor is not a stable, full-featured product yet.

Another option is to fix this lack of features by implementing them, but this is likely to take more time, if done right.

Event Timeline

JGirault created this task.Feb 13 2017, 7:49 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 13 2017, 7:49 PM
JGirault updated the task description. (Show Details)Feb 13 2017, 10:10 PM

See T158013: EPIC: to a full-feature map visual editor

@JGirault Can you link me to a test instance or show a few screenshots that explain this a bit more? I'm having trouble following.

JGirault updated the task description. (Show Details)Feb 14 2017, 7:54 PM

@Deskana, I added some screenshots to show the current lack of support and awkward experience.

My knowledge of VE is limited, so I need a little help here. @Esanders, @Jdforrester-WMF

The current node highlight is such as:

The idea here is to disable the Edit button, and add an informative message such as:

  • The map is only editable with source editing because auto positioning is not yet supported by the Visual Editor.
  • The map is only editable with source editing because external data sources are not yet supported by the Visual Editor.
  • The map is only editable with source editing because data groups are not yet supported by the Visual Editor.

In which case the node highlight could look like :

I have a few questions:

From a UX standpoint, and according to VE patterns, is such a solution acceptable?

More technical questions:

  • How to disable the Edit button?
  • How to put a message in the div.ve-ui-linearContextItem-body ?
  • How to disable the following CSS properties from div.ve-ui-linearContextItem-body :
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;

(^ in other words, can I add a custom class to div.ve-ui-linearContextItem-body ?)

Thanks for your help!

From a product POV, I think we should disable unsupported modes, yes.

What you want is for the DM node to not recognise <mapframe> as an editable type by it, which means it won't load the Kartographer-specific code, so the generic extension handler will let you edit it "raw" (as wikitext) instead. I'm not sure if this is something we currently support, however – getMatchRdfaTypes is normally the method, which doesn't extend to node contents…

https://phabricator.wikimedia.org/diffusion/EKAR/browse/master/modules/ve-maps/ve.dm.MWMapsNode.js

Jdforrester-WMF set the point value for this task to 0.

@Esanders, could you help out?

@Esanders Can you help us with the implementation on this when you get a chance? It's a must have for us wrapping up work; see T155601 for more details.

Deskana triaged this task as High priority.Mar 3 2017, 6:46 PM

Can we get some help with this, please? This was identified as "must have" in T155601: Stabilizing Interactive Products so it's quite urgent. I'm not sure exactly what is required here; @JGirault will know.

@Esanders Is this something you or someone else in the VisualEditor team can help with? It's been nearly a month since the first request. If you can't due to time constraints that's totally understandable, and I'd like to know that so I can plan accordingly. Thanks!

Esanders added a comment.EditedMar 27 2017, 5:54 PM

Hi, sorry for missing the pings . We do something similar for references:

I'll find the parts of the code that do this, or come ask me on IRC.

You'll want to extend the ve.ui.LinearContextItem like in ve.ui.MWReferenceContextItem in the Cite extension. You can disable the edit button by adding an isEditable method to ve.dm.MWMapsNode.

You'll want to extend the ve.ui.LinearContextItem like in ve.ui.MWReferenceContextItem in the Cite extension. You can disable the edit button by adding an isEditable method to ve.dm.MWMapsNode.

Thanks @Esanders, that should get me started. I'll play with that, and come back to you if I have any questions.

JGirault moved this task from Backlog to In progress on the Maps-Sprint board.Mar 28 2017, 6:58 PM
JGirault claimed this task.

Change 345272 had a related patch set uploaded (by JGirault):
[mediawiki/extensions/Kartographer@master] Disable VE when features are not supported

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

With this patch:

Maplink not supported in VE
Mapframe not supported in VE
Mapframe supported in VE

Todo:

  • Double clicking on the maplink/mapframe still opens the editing dialog.
  • Also, clicking on the Edit button doesn't work, although the map should be editable.

@Esanders can you take a look at the patch, review the implementation, and help me figure out those two issues ?

Ok now only the double click on the maplink still opens the dialog. I think I have to extend ve.ui.Command and run mwMaps through this.model.isEditable...

Thanks Julien! I think the messages you added look good.

Change 345272 merged by jenkins-bot:
[mediawiki/extensions/Kartographer@master] Disable VE when features are not supported

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

Deskana moved this task from Needs review to Done on the Maps-Sprint board.Apr 17 2017, 6:49 PM
Deskana closed this task as Resolved.

Looks like this one is done now, and on its way for deployment. Yay!

Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptApr 17 2017, 6:49 PM