Page MenuHomePhabricator

Unsubmitted text in TitlesMultiselectWidget on Sp:Block should be treated as submitted when widget becomes unfocussed
Closed, ResolvedPublic

Description

Problem

If the input in the 'Pages' field is not submitted into TitlesMultiselectWidget there is no warning that it will not be saved.

Unsubmitted (will not save in partial block)Submitted (will save)

Video of this behavior: https://drive.google.com/open?id=1Z0EdTOQdSrGiDadRnAJxtn7g8kFGaTsq


Acceptance Criteria

  • We should make it as if the user pressed enter when they tab out or lose focus of this widget.
  • If the page name is invalid (e.g. redlink) an error message/state can appear either before or after the admin submits the block
  • If the page name is a duplicate of a page already provided, it can be discarded with no error message
  • If the page is the 11th (the current max is 10) an error message/state can appear either before or after the admin submits the block

Details

Related Gerrit Patches:

Event Timeline

TBolliger created this task.Nov 1 2018, 3:40 PM
Restricted Application added subscribers: MGChecker, Aklapper. ยท View Herald TranscriptNov 1 2018, 3:40 PM
TBolliger renamed this task from Better handled unsubmitted text in TitlesMultiselectWidget to Better handle unsubmitted text in TitlesMultiselectWidget on Sp:Block.Nov 1 2018, 3:41 PM
TBolliger assigned this task to Prtksxna.
TBolliger updated the task description. (Show Details)

Prateek, could you please look at this and let us know what the expected behavior should be? Thank you!

TBolliger triaged this task as High priority.Nov 1 2018, 3:59 PM
dbarratt added a subscriber: dbarratt.EditedNov 1 2018, 4:50 PM

@TBolliger What if you've added one or two pages and then you start typing a third, and click Submit should it accept that text or no?

@TBolliger What if you've added one or two pages and then you start typing a third, and click Submit should it accept that text or no?

Yes, it should accept that text. Whatever solution we decide should work for the first input or the Nth input.

Let's say it's the 11th, in that case it would either warn or auto-accept and display the "maximum of 10" error message that already exists.

My personal opinion is that this input should DWIM and accept the text as if it has been submitted. If it is invalid (e.g. the page does not exist or it is the 11th entry) we have other error messages to display.

aezell added a subscriber: aezell.Nov 1 2018, 5:13 PM

I reckon this is OOUI territory but it seems like we'd need to decide what action/character means you are done with that item. Spaces won't work because pages have spaces in the name, right? Some apps that do this with tags use the comma as a signal. At any rate, blurring the field should submit these at a minimum.

I reckon this is OOUI territory but it seems like we'd need to decide what action/character means you are done with that item. Spaces won't work because pages have spaces in the name, right? Some apps that do this with tags use the comma as a signal. At any rate, blurring the field should submit these at a minimum.

Good question, Alex. I would say we should start small and accept their exact input as one submission. In other words, treat spaces and commas as parts of the page name. e.g. if they type George Washington to accept it as George Washington and Omaha, Nebraska as Omaha, Nebraska.

jrbs added a subscriber: jrbs.Nov 1 2018, 8:35 PM

I reckon this is OOUI territory but it seems like we'd need to decide what action/character means you are done with that item. Spaces won't work because pages have spaces in the name, right? Some apps that do this with tags use the comma as a signal. At any rate, blurring the field should submit these at a minimum.

Good question, Alex. I would say we should start small and accept their exact input as one submission. In other words, treat spaces and commas as parts of the page name. e.g. if they type George Washington to accept it as George Washington and Omaha, Nebraska as Omaha, Nebraska.

IMO, the only good solution to this is to use the return key.

Return key for users who prefer the keyboard, or click on the drop-down suggestion for users who prefer a mouse/tapping.

The problem with the dropdown โ€” which is a different bag entirely โ€” is the lag that occurs when the pagename is short and/or the user is really fast.

This is OOUI territory indeed โ€” T188886: TagMultiSelectWidget misleadingly leaves uncommitted input text visible when unfocused

I agree with @TBolliger's idea of DWIM - we should make it as if the user pressed enter when they tab out or lose focus of this widget (https://gerrit.wikimedia.org/r/421815).

TBolliger renamed this task from Better handle unsubmitted text in TitlesMultiselectWidget on Sp:Block to Unsubmitted text in TitlesMultiselectWidget on Sp:Block should be treated as submitted when widget becomes unfocussed.Nov 2 2018, 4:19 PM
TBolliger updated the task description. (Show Details)

Sounds good @Prtksxna !

In estimation today, we should determine what we should do with both this ticket and T188886 โ€” same fix? different? should we fix both?

FYI, see this task with a discussion of what is the best UX; it's not completely straight forward what the best solution is for the widget: T188886: TagMultiSelectWidget misleadingly leaves uncommitted input text visible when unfocused

We can find a specific fix for this specific use case if needed, though the ideal would be fixing the widget in general. The widget, though, needs to be generalized, and I'm not sure we can completely generalize the expectations here across uses. See the discussion on that ticket.

TBolliger reassigned this task from Prtksxna to Mooeypoo.Nov 2 2018, 6:41 PM
TBolliger added a subscriber: Prtksxna.

This is a release blocker for Italian Wikipedia (or any other production Wikimedia community) but we might not want to wait for T188886 to be properly fixed. Moriel will be investigating this and we will reassess at Sprint Planning next Wednesday if we want to make our own hack.

Change 471368 had a related patch set uploaded (by Mooeypoo; owner: Mooeypoo):
[oojs/ui@master] TagMultiselectWidget: Make widget invalid if there's text in input

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

Change 471368 merged by jenkins-bot:
[oojs/ui@master] TagMultiselectWidget: Make widget invalid if there's text in input

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

Volker_E removed a subscriber: gerritbot.

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

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

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

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

TBolliger updated the task description. (Show Details)Nov 7 2018, 6:21 PM
dbarratt closed this task as Resolved.Nov 8 2018, 4:14 PM
dbarratt moved this task from Review to Done on the Anti-Harassment (AHT Sprint 32) board.

Works like a dream! Great work @Mooeypoo @Tchanders @Volker_E !!