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: when clicking on scrollbar do not call onBlur #6475

Merged
merged 2 commits into from
Sep 27, 2018

Conversation

Vitalius1
Copy link
Contributor

@Vitalius1 Vitalius1 commented Sep 26, 2018

Pull request checklist

Description of changes

Adds additional logic to stop the onBlur handler to be invoked when clicking on the scrollbar of the Callout holding the ComboBox items

Microsoft Reviewers: Open in CodeFlow

Copy link
Contributor

@cliffkoh cliffkoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. It would be great to add a unit test.

Copy link
Contributor

@chang47 chang47 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Only one question, has this problem always existed? Or is this something new that was regressed by something else?

@Vitalius1
Copy link
Contributor Author

@chang47 It looks like it always existed... but nobody ever checked the clicking on the scrollbar of the Callout holding the list menu.

@chang47
Copy link
Contributor

chang47 commented Sep 27, 2018

Good to know. Thank you!

Copy link
Member

@KevinTCoughlin KevinTCoughlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A test would be great, but need not block this PR. Perhaps follow-up with one if you have the time or create an issue so we remember.

Solution LGTM, can't think of a way to do it without traversing the parent nodes until first match so 👍

:shipit:

@Vitalius1 Vitalius1 merged commit 16ee7e1 into microsoft:master Sep 27, 2018
@msft-github-bot
Copy link
Contributor

🎉[email protected] has been released which incorporates this pull request.:tada:

Handy Links:

@stanleyymsft
Copy link
Collaborator

stanleyymsft commented Oct 11, 2018

@Vitalius1 , the fix now can prevent onBlur event handler from being invoked when the user clicks on the scroll bar. However, after the user drag the scroll bar to the right position and move the mouse cursor to the option to click and select it, the moment the cursor moves off the scroll bar, the onBlur handler is invoked. At that moment the mouse cursor is on one of the options, so it is technically not an blur gesture. As a result the selection menu will disappear, so the user can't click and select the intended option. I think this is still a little bug left here.

@Vitalius1
Copy link
Contributor Author

Vitalius1 commented Oct 12, 2018

@stanleyymsft I am trying to repro the issue you described when running locally and I don't see the behavior you mentioned. I am able to click on the scrollbar's thumb, on the scrollbar and I can drag the thumb with the mouse and then move the mouse over the items list and select.

I do see the onBlur being invoked though... So maybe you are right there is still something going on there that needs a fix.

Edit: I assume you control the visibility of the list with the onBlur callback and this is why the menu disappears. I will reopen the issue and pass it to the next Shield engineer on duty.

@dzearing
Copy link
Member

@stanleyymsft did you debug in there to understand the bug? Can you provide as much information as possible? Do you see an obvious fix? @jspurlin wrote the ComboBox so if you need more background you can ask him. :)

@Vitalius1 Vitalius1 deleted the v-vibr/ComboBox_onBlurBUG branch November 7, 2018 21:03
@stanleyymsft
Copy link
Collaborator

@dzearing I tested again today. The issue no longer repro and the the scroll worked well when clicking on it or moving from it to options. Thanks!

@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ComboBox invokes onBlur handler unexpectedly when the user clicks the scroll bar of the dropdown menu
7 participants