Page MenuHomePhabricator

mixin.DraggableElement `draggable` in constructor config without effect
Closed, ResolvedPublic

Description

OO.ui.mixin.DraggableElement has two ways to set its draggable state - via the constructor config, or via the toggleDraggable method.

There is a list of problems in this redundant implementation.

Via the constructor:

  • the class oo-ui-draggableElement-undraggable, indicating that the object is not supporting dragging at the moment, is not added (compare toggleDraggable method)
  • the draggable attr does not get set based on the object property but is always true (might not have negative effect, but is weird)

Via the toggleDraggable method:

  • large parts of the method are only run if a change to the object property is performed. That means that calling toggleDraggable(false); stays without effect after constructing the object with config { draggable: false } and leaves the object in an undesired state (while specifying the opposite twice)

Might be what T129184 tried to describe.

Event Timeline

Change 370965 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[oojs/ui@master] DraggableElement: Make toggling draggability consistent

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

Change 370965 merged by jenkins-bot:
[oojs/ui@master] DraggableElement: Make toggling draggability consistent

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

Volker_E assigned this task to matmarex.
Volker_E triaged this task as Medium priority.
Volker_E removed a project: Patch-For-Review.

Change 935701 had a related patch set uploaded (by WMDE-Fisch; author: WMDE-Fisch):

[mediawiki/extensions/AdvancedSearch@master] Remove OOUI workarounds that got fixed upstream

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

Change 935701 merged by jenkins-bot:

[mediawiki/extensions/AdvancedSearch@master] Remove OOUI workaround that got fixed upstream

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