Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ComboBox invokes onBlur handler unexpectedly when the user clicks the scroll bar of the dropdown menu #6392

Closed
stanleyymsft opened this issue Sep 17, 2018 · 19 comments · Fixed by #6475 or #11584

Comments

@stanleyymsft
Copy link
Collaborator

stanleyymsft commented Sep 17, 2018

Bug Report

For a ComboBox that is configured with "allowFreeForm" being true, if the user clicks on the scroll bar of the dropdown menu in order to scroll to an option to select, it invokes onBlur handler unexpectedly (technically the user's focus is still on the ComboBox as a whole). In comparison, Fabric Dropdown component doesn't have this issue.

  • Package version(s): 6.61.2
  • Browser and OS versions: Chrome on Windows 10/2018 Server

Priorities and help requested:

Are you willing to submit a PR to fix? No

Requested priority: High

Products/sites affected: SPO

Describe the issue:

Add a ComboBox that is configured with "allowFreeForm" to be true and large number of options so that a scroll bar is shown in the dropdown menu. The user clicks on the arrow to open the dropdown menu. Then the user clicks on the scroll bar in the dropdown and try to drag it down or up.

Actual behavior:

Actual: onBlur handler is invoked unexpectedly.

Expected behavior:

Expected: onBlur handler is not invoked because technically the interaction is still on the ComboBox. (Drowdown control doesn't invoke onBlur in this situation)

@micahgodbolt
Copy link
Member

Have you checked to see if this is still the behavior in our current release? If so, we can try to work out a fix and backport it to 5.x.

@stanleyymsft
Copy link
Collaborator Author

stanleyymsft commented Sep 17, 2018

@micahgodbolt, Sorry, it was my mistake previously. We are actually using version 6.61.2. I updated the bug report above too. Thanks!

@KevinTCoughlin
Copy link
Member

KevinTCoughlin commented Sep 24, 2018

I'm able to repro this in isolation, here's a Codepen: https://codepen.io/kevintcoughlin/pen/gdJdZj?editors=1010

And here's a Codepen with a similar <Dropdown/> not exhibiting the same onBlur behavior: https://codepen.io/kevintcoughlin/pen/vzwVyz?editors=1010

Tagging as a bug given the above repros.

@Vitalius1
Copy link
Contributor

@KevinTCoughlin @stanleyymsft Found a fix for when clicking on Scrollbar not to call the onBlur handler but I noticed that it's called also when clicking the arrow to open the ComboBox. Is this something needs to be fixed too, cause it follows the same logic (technically the user's focus is still on the ComboBox as a whole) as the author of the issue states.

@msft-github-bot
Copy link
Contributor

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

Handy Links:

@stanleyymsft
Copy link
Collaborator Author

Thank you!

@Vitalius1 Vitalius1 reopened this Oct 12, 2018
@Vitalius1
Copy link
Contributor

Reopening as there is still some weird onBlur invocation happening. See details in the discussion with the original author in #6475

@mikewheaton
Copy link
Contributor

@Vitalius1 Are you still working on this one?

@Vitalius1
Copy link
Contributor

@mikewheaton not as of now. Do you think Shield could pick this one up?

@JasonGore
Copy link
Member

JasonGore commented Oct 24, 2018

Ok, I've looked at this for a bit and I think I know what's going on. The _onBlur handler is bound to the Autofill component, which means it can only trap blur events from that subtree. This subtree does not include the Callout component nor any of its elements. That means that onBlur events from the Callout can't be blocked or filtered as ComboBox is currently written.

Additionally, whenever a user simply hovers over a callout element, focus is set on Autofill, which is outside of the hierarchy of the Callout. As a result, an unblockable onBlur event is triggered just by hovering on a Callout element after clicking on the scroll bar. This is what's causing the onBlur event that @stanleyymsft described in #6475. (Unblockable as the blur event comes up the Callout hierarchy, bypassing the _onBlur handler bound to Autofill.)

Unfortunately I'm not sure the best way to approach fixing this. I have some questions like:

  1. Why is Autofill focused just by hovering over elements?
  2. Should _onBlur be at a higher level where it can block blur events from the Callout or is another higher level blur handler required?

I'm hoping @jspurlin can give his thoughts...

@jspurlin
Copy link
Contributor

Not sure why an onBlur is firing on hover, the autofill should already be focused... In general the autofill needs to continue to have focus when hovering over elements so that the user can continue to type in the input. When the user hovers we keep track of which item is hovered so that the user (if they just hover over an items) can press ENTER and it will select the hovered item.

Having onBlur at at full comboBox level where it can handle all the onBlur events sounds good. It looks like the current _onBlur is attempting to do that by checking if a blur is coming from within the the root or within the combBoxMenu...

@JasonGore
Copy link
Member

JasonGore commented Oct 25, 2018

Just to clarify, when you click on the scroll bar to scroll items in the Callout, it grabs focus from the Autofill. Then a hover event over a callout item causes focus to go back to Autofill, which fires an onBlur event (originating from Callout) and bubbling up to the component consumer.

The current _onBlur can only trap events when Autofill loses focus (if the focus moved TO Callout) but not when onBlur events come FROM Callout.

Thanks for the input!

@jspurlin
Copy link
Contributor

jspurlin commented Oct 25, 2018

Ah, I missed the scrollbar aspect of this issue. Yeah, focus should always stay in the input (Autofill) of the ComboBox while the user is interacting with the ComboBox

@JasonGore
Copy link
Member

JasonGore commented Oct 25, 2018

I don't think there's a way to keep focus on Autofill currently when the users clicks on the Callout scrollbar. The way ComboBox is architected now I think Callout would somehow have to be a child of Autofill to make that possible, but I don't think Autofill can take children that way... did you have an idea in mind for that?

@jspurlin
Copy link
Contributor

Nope, but in my mind I think that the ComboBox should be the own driving focus retlated things all up for itself

@micahgodbolt
Copy link
Member

this might be something to look into during the work in #6030 @ecraig12345

@micahgodbolt micahgodbolt added the Status: Blocked Resolution blocked by another issue label Dec 28, 2018
@dzearing
Copy link
Member

@stanleyymsft is this important to resolve? what is the effect of this on SharePoint?

@micahgodbolt
Copy link
Member

micahgodbolt commented Dec 31, 2019

Dropdown works because 'onBlur' is faked. We control 'focus' of the Dropdown with state, and we only fire onBlur if the dropdown was already closed by the event.

Combobox onBlur is normal input focus, and clicking outside of the Combobox causes the input to lose focus.

i.e. i'm not sure if there is a way to keep onBlur from firing when the input in fact loses focus. We can always return focus to the input, but not without an onBlur already being called.

@micahgodbolt micahgodbolt removed the Status: Blocked Resolution blocked by another issue label Dec 31, 2019
msft-github-bot pushed a commit that referenced this issue Jan 3, 2020
…1584)

#### Pull request checklist

- [ ] Addresses an existing issue: Fixes #6392
- [ ] Include a change request file using `$ yarn change`

#### Description of changes

Clicks outside of the input were causing the input to lose focus and fire 'onblur'. With this change clicks to headers, dividers and scrollbars will have no effect, but clicks to menu items work properly.

#### Focus areas to test

(optional)
@msft-github-bot msft-github-bot added Status: Fixed Fixed in some PR and removed Status: In PR labels Jan 3, 2020
@msft-github-bot
Copy link
Contributor

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

Handy links:

@microsoft microsoft locked as resolved and limited conversation to collaborators Feb 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.