Page MenuHomePhabricator

TagMultiSelectWidget misleadingly leaves uncommitted input text visible when unfocused
Closed, ResolvedPublic

Description

When focus is removed from a CapsuleMultiSelectWidget and the text input field is empty, it will attempt to commit the text. The end result is that the text either becomes a valid entry in the widget or it disappears.

When focus is removed from TagMultiSelectWidget, it leaves the uncommitted text in place. It is very easy[1] to think that this text is part of the value of the widget, when in fact it is not.

[1]: As in I kept having issues with ApiSandbox not seeming to recognize titles entered into the "titles" field, and finally realized it was a regression due to the change in widgets.

To reproduce:

  1. Open https://en.wikipedia.org/wiki/Special:ApiSandbox
  2. Select "purge" in the "action" dropdown.
  3. Select the "action=purge" tab on the left.
  4. Activate the "titles" widget.
  5. Type "Foo" into the widget. Do not press enter.
  6. Remove focus from the widget, by clicking elsewhere or tabbing. Observe that "Foo" is displayed in the widget, as if it were part of the value of the widget.
  7. Click "Make request"

The sandbox output will be

{
    "batchcomplete": "",
    "purge": []
}

The expected output is

{
    "batchcomplete": "",
    "purge": [
        {
            "ns": 0,
            "title": "Foo",
            "purged": ""
        }
    ]
}

The expected output can be achieved by re-focusing the widget and pressing enter. While this does display in the widget as a different state from the erroneous state, the erroneous state is still unnecessarily confusing.

Related Objects

Event Timeline

Anomie created this task.Mar 4 2018, 11:39 PM

Change 416621 had a related patch set uploaded (by Mooeypoo; owner: Mooeypoo):
[oojs/ui@master] TagMultiselectWidget: Blur inserted text on input blur

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

That's a good point. We should either clear the input on unfocus, or, at least, "dim" the text to a level where it's clearer that the input itself is not the data.
I'm a bit concerned about clearing the input on unfocused, since that will cause the user to lose typed data. I have an idea about how to fix this, but since it's a bit elaborate, I want @Volker_E and @matmarex opinion here:

  • If the user unfocuses the input while the input contains text, the text will be transformed into a placeholder
  • If the user focuses the input that has a placeholder that is not the original (so, it has a previously typed in text) the input will now be refilled with the text again

This should make it clear that text inside the input that has not been literally added as a tag is not active.
(By the way, if the user's freetext is exactly like the placeholder, it will be blanked when the user re-clicks the input, but I think that's fine, unless you think otherwise and then we could preserve the state better)

The commit above includes this behavior, so you can pull it and play around and see if this works.

Alternatively, we could just nullify the input on blur.

@Volker_E and @matmarex -- your thoughts ?

Volker_E moved this task from Backlog to Reviewing on the OOUI board.Mar 6 2018, 3:59 AM

Followup - as @Anomie pointed out in the gerrit patch, this doesn't work as well in the MenuTagMultiselectWidget because the nullification of the input is a "change" event and the menu then pops open.

In MenuTagMultiselectWidget, the input is a filtering option, and I think we should then either clear it out or override the blurring behavior for the menu iteration of the widget. I'm not sure which. Another option is that we could also have the menu act exactly the same but prevent reopening of the menu if the action was done because of the placeholder, which could be better for consistency.

Waiting for @Volker_E's input on what's the best UX overall here.

Could it be an option to add the remaining text as a tag onBlur? It is quite easy to undo if the user did not intend to do this. Might not be possible when allowArbitrary is false.

Change 421815 had a related patch set uploaded (by Prtksxna; owner: Prtksxna):
[oojs/ui@master] TagMultiSelectWidget: addTagFromInput onInputBlur

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

Mooeypoo added a comment.EditedMar 26 2018, 7:05 AM

Could it be an option to add the remaining text as a tag onBlur? It is quite easy to undo if the user did not intend to do this. Might not be possible when allowArbitrary is false.

I personally think we should make as few decisions for the user as possible, and would prefer giving a hint that something isn't added rather than choose between adding or deleting for the user -- but I trust your judgment on best UX, and I don't think it's super critical either way.

That said, we will likely need to re-examine whether this should have the same behavior in MenuTagMultiselectWidget (probably not?) and/or if it should be overridden there, since Menu*Widget has more restrictions on what can be added, and also the input serves more as a "filter" or "search" rather than a freetext adding method. I'm not sure about this, but wanted to flag in case you want a different behavior there.

Also, another point -- for inputs that don't have "allowArbitrary" the text remains in the input, which brings us back to the same problem. Should we mark that text? should we add the tag as an invalid tag?

Also, another point -- for inputs that don't have "allowArbitrary" the text remains in the input, which brings us back to the same problem. Should we mark that text? should we add the tag as an invalid tag?

Yeah its a problem for those.

I was thinking aloud on the patch if we should remove that text as well. I see your point of "make as few decisions for the user as possible", especially when we are getting rid of their input.

Also, another point -- for inputs that don't have "allowArbitrary" the text remains in the input, which brings us back to the same problem. Should we mark that text? should we add the tag as an invalid tag?

Yeah its a problem for those.
I was thinking aloud on the patch if we should remove that text as well. I see your point of "make as few decisions for the user as possible", especially when we are getting rid of their input.

Yeah I am concerned that the widget is relatively so complex (in user expectations) that making the sdecisions can be more confusing; a user can not notice that they started typing, moved to another input and when they come back they may not notice that their half-written text was added as a tag or was deleted...

I dunno, I usually prefer not assuming that user intention in those cases, which is why my patch above was working on making the indication clearer -- as in, if we switch so the remaining text now looks dimmer, it can be clearer to the user that whatever they left inside the input is unused -- but they're not losing the data in case it was something they thought they needed, etc.

I don't think it's critical either way, honestly, but whatever we choose, we should make consistent, and we should try and make sure we're as clear as possible. Perhaps adding those "invalid" red tags is useful (but there's a config to allow/not-allow invalid tags, so we'll have to then think what will happen when those aren't allowed)

... the config options and child classes make this whole conditional approach super convoluted. We should see what makes more sense for the most cases, and try and make it as clear as possible.

Good luck to us! ;)

Change 421815 abandoned by Prtksxna:
TagMultiSelectWidget: addTagFromInput onInputBlur

Reason:
Bug already has a patch.

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

Sorry @Mooeypoo! I did not notice that there was already a patch on this bug. My bad!

a user can not notice that they started typing, moved to another input and when they come back they may not notice that their half-written text was added as a tag or was deleted.

So, in the patch that I had submitted, when you type something, forget to hit enter and just hit tab, it adds the tag and keeps you in the widget. I think it is clear that the tag was added and the user is still in the same widget to make a correction.

Perhaps adding those "invalid" red tags is useful (but there's a config to allow/not-allow invalid tags, so we'll have to then think what will happen when those aren't allowed)

Yeah, maybe we need another state for invalid tags that won't be submitted. But that complicates things even further.

@Mooeypoo, I see your point. Having different behaviors for different values of allowArbitrary will create an inconsistent experience for people. I agree, whatever we do, whether its as styling change or adding the tag, it should be consistent.

I had the idea of marking the whole widget as invalid (red outline) if there is a floating input. But that might give the wrong impression that it blocks submitting the form. What do you think?

Having different behaviors for different values of allowArbitrary will create an inconsistent experience for people.

I note the behaviors for different values of allowArbitrary are already different: false doesn't let you enter arbitrary tags, while true does.

Personally, the behavior I as a user have expected is for it to turn the input into a tag if it's valid (either because allowArbitrary is true or because the input exactly matches a valid option). If it's not valid, perhaps the suggestion below if you don't like clearing it like the old widget did.

I had the idea of marking the whole widget as invalid (red outline) if there is a floating input. But that might give the wrong impression that it blocks submitting the form. What do you think?

Perhaps that should be the right impression: mark the widget as invalid and prevent submitting the form until the pending value is cleared in some manner.

I note the behaviors for different values of allowArbitrary are already different: false doesn't let you enter arbitrary tags, while true does.

Fair point.

Personally, the behavior I as a user have expected is for it to turn the input into a tag if it's valid (either because allowArbitrary is true or because the input exactly matches a valid option). If it's not valid, perhaps the suggestion below if you don't like clearing it like the old widget did.

That was my intuition, but I wanted to be sure, so discussing here :)

Perhaps that should be the right impression: mark the widget as invalid and prevent submitting the form until the pending value is cleared in some manner.

Hm. @Mooeypoo @Volker_E @matmarex thoughts?

I have to admit, because I dislike making decisions for the user, my personal preference is to make it as "quiet" and unintrusive as possible.

If the issue is that it's unclear that text remaining in the input box is unused, then my patch makes it clear by muting it.

I think that this type of behavior is consistent enough to be used with all types of widgets without having too many edge cases or weird expectation of behavior, and without having to take into account arbitrary values or force the user to deal with an invalid widget. Also, invalid widget in general might be confusing when we have invalid tags; would the entire widget be invalid if it has invalid tags to begin with? Do we mark it with a whole red border on the widget or just the input, to be consistent with other widgets? I know @Volker_E was also talking about how invalid states in OOUI are problematic in general and will be reconsidered in behavior.

In short, honestly, I think that the behavior I'd prefer is one where we make the least amount of decisions for the users and the least amount of "stopping the flow" of action. Stopping tabs is obstructing flow; adding invalid tag is obstructing flow and can be confusing; invalidating the entire widget is obstructing flow... and the user most likely didn't even notice.

By making the input dim -- but keeping the value stored -- it seemed to make it clear that it isn't used, and if you mistakenly left the widget you didn't lose your data - you click back on it, and it's there.

So, not to tout my own horn, but honestly, I think the available patch is the superior solution -- with only adjustments to MenuTagMultiselectWidget that we'll need to decide how to best handle (though we can start by having it behavior the exact same)

and the user most likely didn't even notice.

That's a problem with your solution too. Greying out the text makes it more noticeable that something is up, yes, but it's still relatively easy to overlook.

Part of the problem here is that most widgets don't have this ability to leave input in a "pending" state. A normal text input widget, any change you make is immediately effective and will be submitted with the form. A dropdown list, changing the dropdown is immediately effective and will be submitted with the form. A checkbox or radio button, same thing. A file selection widget brings up a modal dialog, and once that dialog is closed the widget either has or doesn't have a file. CapsuleMultiSelectWidget doesn't have this "pending" state either, when you defocus the widget everything displayed is part of the data being submitted.

If the issue is that it's unclear that text remaining in the input box is unused, then my patch makes it clear by muting it.

I don't think dimming the text will bring enough attention to this.

In short, honestly, I think that the behavior I'd prefer is one where we make the least amount of decisions for the users and the least amount of "stopping the flow" of action. Stopping tabs is obstructing flow; adding invalid tag is obstructing flow and can be confusing; invalidating the entire widget is obstructing flow... and the user most likely didn't even notice.

The patch I had uploaded wasn't stopping tab flow, the user could tab out of the widget once the tag was added. So yes, 2 tabs instead of 1, but not stopping them.

I think the available patch is the superior solution -- with only adjustments to MenuTagMultiselectWidget that we'll need to decide how to best handle (though we can start by having it behavior the exact same)

While I disagree, I am happy to have it merged, and see if users still get confused, instead of more discussion now :)

Majr added a subscriber: Majr.EditedMay 12 2018, 5:32 AM

The main issue I have with the way the widget works currently is for anything you type to actually have an effect, the user must press enter, which is totally counter-intuitive in a text input. Every other form has already conditioned the user to never press enter in text inputs until they are completely done and want to submit the form, and to instead press tab or click on the next field when they are done with the current one.

This wouldn't necessarily be a problem if there was any indication that this particular field works differently to a normal text input (like a button to "add" the entered text), but—at least in the case of ApiSandbox—it just looks like a normal text input, where pressing enter would submit the entire form.

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

This was raised again, this time with partial blocks. My newly proposed design is simpler than the other and, hopefully, will pass the scrutiny of UX so we can have a fix for this: If there's text left over in the widget, it becomes invalid, and the text is emphasized.

@Volker_E there was no design set for -invalid flag for the TagMultiselectWidget at all, so I copied the rules that were set on the tag with the LESS variables colors. I assume that's not acceptable for you, so please feel free to fix the design to whatever you think is better :)

Since it's now not only a problem in 2 cases but also is a release blocker, we need to settle on best solution we can and move on. We can always tweak this later.

Volker_E added a comment.EditedNov 5 2018, 8:12 PM

@Mooeypoo Thanks for pinging again. Comparing this and T208507 I'd like to emphasize that we shouldn't look at TagMultiselectWidget in isolation. And I agree with with what @Prtksxna and @Anomie have stated here before.
A user goes through a form of different widgets, either by clicking or by tabbing. If I enter a value into a TextInputWidget and click submit it will take this input as value.

  • With allowArbitrary: true I'd expect the same from TagMultiselectWidget, turning the input into a tag automatically. The user has already taken a decision at this point, namely inputting the value on the keyboard. The only time that I can think of this where this automatic conversion after tabbing out would lead to mistaken input is, when you tab over and a false keystroke slips in on the way of tabbing further, or mis-clicking. But that seems like a rare edge case tbh. And if it's not an auto-updating interface, you might be able to go back to the input and delete it. Not sure if a double-tabbing like @Prtksxna proposed is even necessary as it could be more confusing in user flow than just auto-converting inputs on way out.
  • With allowArbitrary: false the input should get the invalid as laid out in @Mooeypoo's recent patch.

A couple of problems:

  • What about allowDisplayInvalidTags then? If it's true, invalid tags are ALLOWED to be added; they're added as invalid (red) and the widget becomes invalid. If it's false, the addition is not allowed.
  • What about cases where the input isn't how you add, but rather how you filter -- that is, in the case of MenuTagMultiselectWidget, the input serves as a filter to the menu items; you pick your item that will be added ,but the text in the actual input isn't the text you want. In that case, do we add the first item in the menu if it is not already there, etc etc.

My point here is this: This widget has a relatively large array of configuration options that allow it to be super robust, but it also means that a lot of the behavior is relatively delicate. We've been discussing back and forth a bit on this ticket and we need to solve this in a way that brings the most generalized solution.

My personal preference is not to make a decision for the user, but I can see the idea of automatically adding a tag if it's valid.

However, let's make sure we're consistent; if we allow for invalid tags, do we add the tag automatically when it's invalid? If we have a menu, do we add the tag at all or mark the widget invalid? etc. This can be very confusing, so I opted for the most straight-forward approach. We can fix the code to reflect changes we want, but let's try to solve this quickly, seeing as it's a problem in 2 places already, and a release blocker on one.

My personal preference is not to make a decision for the user

I don't think this is making a decision for the user. Whenever the input *can* be turned into a tag it should be, only iff it is impossible to make a tag from the input should we go to an invalid state.

I think capturing <TAB> to create a tag iff the input is non-empty would make this less of an issue. The user would have to blur the input with a mouse click to leave it non-empty.

In the case of MenuTagMultiselectWidget I assume that's what happens if you press ENTER, and so the first item should be highlighted. ENTER, TAB and blur should all do the same thing.

Volker_E moved this task from Reviewing to OOUI-0.29.4 on the OOUI board.Nov 6 2018, 8:43 PM
Volker_E edited projects, added OOUI (OOUI-0.29.4); removed OOUI.

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 closed this task as Resolved.Nov 7 2018, 12:26 AM
Volker_E assigned this task to Mooeypoo.
Volker_E triaged this task as Medium priority.
Volker_E removed a project: Patch-For-Review.
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