Page MenuHomePhabricator

Pressing backspace to delete a tag in namespace multiselect widget populates the input with the tag data instead of the tag label
Closed, ResolvedPublic

Description

From T214197: Pressing backspace next to a selected item/pill converts that namespace back to its namespace number. Given that the widget doesn't expose namespace numbers to the user anywhere else, we should probably try and stick with names.

This should be fixed by overriding doInputBackspace from TagMultiselectWidget in MenuTagMultiselectWidget. The menu options display the label rather than the data, so it makes more sense to populate the input with the label.


Expected behavior

  • Pressing backspace next to an accepted item/pill should replace the pill with the namespace name, not the number.

Event Timeline

Tchanders triaged this task as Normal priority.

Quick clarification (just to save implementation/confusion time in finding the code in the different parent widgets) :

This should be fixed by overriding doInputBackspace from TagMultiselectWidget in MenuTagMultiselectWidget

It should be overridden in mw.widgets.TitlesMultiselectWidget; it originates in OO.ui.TagMultiselectWidget.prototype.doInputBackspace where the item is placed as item.getData() instead of item.getLabel()

@Mooeypoo I think it makes sense to change to item.getLabel() in MenuTagMultiselectWidget because the label is what is exposed in the menu, so most users of this widget are probably more used to interacting with labels than data. (Incidentally the bug was reported for the NamespacesMultiselectWidget, so we'd want to change it there if not in the MenuTagMultiselectWidget.)

Thinking about it more, is there a reason not to simply change it in TagMultiselectWidget? Imagine these four scenarios...

User typed dataUser typed label
Backspace reveals dataExpected UIPotentially confusing, since user hasn't seen data before
Backspace reveals labelNot what user typed, but at least they've already seen label in tagExpected UI

Might populating the input with the label might be the better default UI because it avoids the case of the data being revealed for the first time by pressing backspace?

@Mooeypoo I think it makes sense to change to item.getLabel() in MenuTagMultiselectWidget because the label is what is exposed in the menu, so most users of this widget are probably more used to interacting with labels than data.

I concur.

Change 491551 had a related patch set uploaded (by Tchanders; owner: Tchanders):
[oojs/ui@master] TagMultiselectWidget: populate input with item label on backspace

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

Change 491551 merged by jenkins-bot:
[oojs/ui@master] TagMultiselectWidget: populate input with item label on backspace

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

Volker_E moved this task from Backlog to OOUI-0.30.2 on the OOUI board.Feb 19 2019, 11:37 PM
Volker_E edited projects, added OOUI (OOUI-0.30.2); removed OOUI.
Volker_E moved this task from OOUI-0.30.2 to OOUI-0.30.3 on the OOUI board.Feb 21 2019, 3:47 AM
Volker_E edited projects, added OOUI (OOUI-0.30.3); removed OOUI (OOUI-0.30.2).
Volker_E closed this task as Resolved.Feb 21 2019, 3:49 AM
Volker_E assigned this task to Tchanders.
Volker_E removed a project: Patch-For-Review.

Change 491943 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/core@master] Update OOUI to v0.30.3

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

Change 491943 merged by jenkins-bot:
[mediawiki/core@master] Update OOUI to v0.30.3

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