Picker getTextFromItem changes dismiss behavior

[mattmazzola]

Environment Information

• 7.52.0
• Version 77.0.3865.120 (Official Build) (64-bit)

Please provide a reproduction of the bug in a codepen:

Tried to reproduce here, but it doesn't work properly
https://codesandbox.io/s/picker-calling-onchange-incorrectly-dpwr0

Created video to demonstrate and walk through bug
https://microsoft.sharepoint.com/:v:/t/convsysresearch/ERMRSnt0UNRFuJ0h5uKdS8cBhXbKyC5ATXYvVWj28kOfSA?e=V1aiQf

Actual behavior:

First item of suggestions list is highlighted and selected by default when Picker is dismissed

Expected behavior:

Should not add tag. When dismissing the picker the user intention is not do do anything with picker anymore so it should not do anything. Especially when the user clicks a cancel button.

As stated in video there are two bugs:

1. getTextFromItems should not change dismiss / onChange behaviors.
   It seems like it would just be called turing some map operation to translate items into text, the fact that it can change component behavior is very strange and scary. Makes you wonder what other behaviors / bugs exist only because you set one seemingly innocent attribute differently.

2. onDismiss should still be called when dismissing the picker directly to another Office Fabric component. It shouldn't matter what component you click on. This is also very weird.

Priorities and help requested:

Are you willing to submit a PR to fix? (Yes, No)

Requested priority: (Blocking, High, Normal, Low)

Products/sites affected: Conversation Learner

[marygans]

Hi @mattmazzola thank you for submitting this issue!
The functionality of having the suggested item picked by default after navigating outside of the picker is by design. Please refer to this closed issue for more details surrounding why it is by design as well as a onDismiss callBack solution.
Thanks!

[mattmazzola]

I don't understand your reply. Either I have the wrong onDismiss solution and didn't implement it correctly or you didn't watch the video as the item is picked even when using the "onDismiss soluton". The onDismiss function isn't called when it should be. Bug 2 listed.

What is the onDismiss solution you were referring to?

Both the links you provided went to the same issue which I didn't see any code snippets. I looked in the codepen's and didn't find it there either.
Can you link me directly to the code for solution so I can try it?

I had been using this:

const onDismissPicker = (event?: any) => {
    if (event && typeof event.preventDefault === 'function') {
      event.preventDefault()
    }
  }

Which doesn't work for all cases as shown because the function is not invoked.

[joschect]

@mattmazzola This is definitely an interesting scenario!

First, here is a codepen which does show the suggestions. I think something might have not been working with the filter but I didn't debug too much.

The reason why the second scenario doesn't behave as expected is that some javascript events don't respect preventDefault. One of which I was surprised to find out, happens to be focus. Another example of one that doesn't actually support that method are scroll events. So if you inspect your onDismissPicker function you'll see that event.defaultPrevented is never set to true when a focus event causes the dismiss. There is no way to fix this bug directly since it's a flawed assumption that preventDefault will handle all event cases. I believe that the only way to fix is to allow onDismiss to return a boolean which would ignore the event type and allow correct prevention of the standard behavior. I'll start working on that today.

As for the 3rd scenario, where you don't have getTextFromItem, that's because the picker doesn't automatically select an item when it doesn't have text to match, I'm not sure if this is by design or a subtle bug but we can definitely investigate if this needs to get fixed.

[mattmazzola]

Thank you for looking into this.

Let me make sure I understand what you are saying.

1. Dismiss by clicking empty space
   The reason it does NOT select the top tag is because the event that triggers dismiss is of type "click" which does respect preventDefault()

2. Dismiss by clicking button
   The reason is DOES select top tag is because the event that triggers dismiss is of type "focus" which does NOT respect preventDefault()
   Then the onChange callback fires anyways

This would seem like a browser bug to me. Are there any docs on why different events are treated differently so I can watch out for this in the future?

Can you elaborate on this thought more?

the picker doesn't automatically select an item when it doesn't have text to match

I thought getTextFromItem was for display purposes. Since we were just recreating default behavior of using item.name Is it for some other matching purpose to match filter text?

[joschect]

@mattmazzola Your understanding of the preventDefault issue is correct. As far as I know it is not a browser bug and is expected behavior, albeit not very well documented. I'm a little surprised but wikipedia seems to have a good list as to what's cancelable and what's not.

getTextFromItem is mostly for display purposes, but I'm not sure if it was being used as way of validating that there is a "correct" choice so the user couldn't accidentally select something that wasn't properly formed. I'm leaning towards it being a bug but I still need to verify.

[msft-github-bot]

🎉This issue was addressed in #14302, which has now been successfully released as [email protected].:tada:

Handy links:

• Release Notes
• Official NPM Release
• Test the release with Developer Demo Site