Page MenuHomePhabricator

Implement $.textSelection's replaceSelection and encapsulateSelection in NWE
Closed, ResolvedPublic8 Story Points

Description

TODO seen here: https://gerrit.wikimedia.org/r/#/c/406416/1/modules/ve-mw/ui/ve.ui.MWWikitextSurface.js

We have methods on the surface for converting between source and nwe offsets so should be a case of re-implementing the method form jquery.textSelection.

Related Objects

StatusAssignedTask
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
ResolvedEsanders
OpenNone
OpenNone
ResolvedJules78120
OpenNone
ResolvedDannyH
OpenNone
OpenEsanders
OpenNone
OpenEsanders
OpenNone
ResolvedEsanders
Resolvedmatmarex

Event Timeline

Esanders created this task.Jan 29 2018, 6:28 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 29 2018, 6:28 PM
matmarex claimed this task.Jan 29 2018, 6:38 PM
Jdforrester-WMF triaged this task as Normal priority.Jan 30 2018, 4:51 PM
Jdforrester-WMF set the point value for this task to 1.
Jdforrester-WMF moved this task from To Triage to TR1: Releases on the VisualEditor board.

I've started working on this, and then realized that we should be able to implement $.textSelection's encapsulateSelection in terms of the other $.textSelection functions. This way we won't have to duplicate this 100-line monster in every custom wikitext editor.

Change 408465 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] jquery.textSelection: Implement 'encapsulateSelection' in terms of the other commands

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

Change 408466 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] ve.ui.MWWikitextSurface: Remove unnecessary textSelection 'encapsulateSelection' override

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

On second thought, I'm not sure if that was the best idea… This now essentially works by replacing the entire contents of the editor, which a) sometimes causes the scroll position to jump and b) generates inefficient transactions. I might have to come back to this, and do it the boring way.

Alternatively you could add some more arguments to setContents ( e.g. start, [end] ) such that you could use it for replacements.

Esanders added a comment.EditedFeb 7 2018, 2:47 PM

(NB there is another textSelection implementation in CodeMirror that is 90% encapsulateSelection that could be replaced with a single line codeMirror.doc.replaceSelection( insertText );)

Change 409110 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] jquery.textSelection: Add 'replaceSelection' method

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

Change 408465 merged by jenkins-bot:
[mediawiki/core@master] jquery.textSelection: Implement 'encapsulateSelection' in terms of the other commands

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

Change 409110 merged by jenkins-bot:
[mediawiki/core@master] jquery.textSelection: Add 'replaceSelection' method

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

Change 408466 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] ve.ui.MWWikitextSurface: Implement textSelection 'replaceSelection', 'encapsulateSelection'

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

Jdforrester-WMF renamed this task from Implement $.textSelection's encapsulateSelection in NWE to Implement $.textSelection's replaceSelection and encapsulateSelection in NWE.Feb 9 2018, 12:16 AM
Jdforrester-WMF closed this task as Resolved.
Jdforrester-WMF removed a project: Patch-For-Review.
Jdforrester-WMF changed the point value for this task from 1 to 18.
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptFeb 9 2018, 12:16 AM
Jdforrester-WMF changed the point value for this task from 18 to 8.Feb 12 2018, 5:50 PM
Tpt added a subscriber: Tpt.Feb 15 2018, 3:28 PM

This change seems to have broken Page: pages editing interface used by Wikisource: T187454

Hmm, that's unfortunate, but I think we can work around it in ProofreadPage. I'll reply on that task.