Skip to content

Dropdown: do not select first option automatically unless it's selected using keyboard navigation#10755

Merged
msft-github-bot merged 8 commits into
microsoft:masterfrom
xugao:dropdown-fix
Oct 9, 2019
Merged

Dropdown: do not select first option automatically unless it's selected using keyboard navigation#10755
msft-github-bot merged 8 commits into
microsoft:masterfrom
xugao:dropdown-fix

Conversation

@xugao

@xugao xugao commented Oct 9, 2019

Copy link
Copy Markdown
Contributor

Pull request checklist

Description of changes

In case of keyboard navigation to dropdown, per aria guidance, dropdown should select first option by default on focus. But we do not want to do this if user clicked and opened the dropdown.
Before:
image
After:
image

Microsoft Reviewers: Open in CodeFlow

@xugao xugao changed the title Dropdown: do not select first option automatically unless it's selected using keyboard navigation per aria Dropdown: do not select first option automatically unless it's selected using keyboard navigation Oct 9, 2019
@cliffkoh
cliffkoh requested a review from dzearing October 9, 2019 05:13
@size-auditor

size-auditor Bot commented Oct 9, 2019

Copy link
Copy Markdown

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react Dropdown 212.376 kB 212.566 kB ExceedsBaseline     190 bytes

ExceedsTolerance Over Tolerance (1024 B) ExceedsBaseline Over Baseline BelowBaseline Below Baseline New New Deleted  Removed 1 kB = 1000 B

Baseline commit: 25b3c4b429e592c8d61208f11e808c4d1cbc82a7 (build)

@msft-github-bot

msft-github-bot commented Oct 9, 2019

Copy link
Copy Markdown
Contributor

Component Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Status
BaseButton 954 892
BaseButton (experiments) 1162 1188
DefaultButton 1187 1165
DefaultButton (experiments) 2196 2290
DetailsRow 3617 3733
DetailsRow (fast icons) 3708 3676
DetailsRow without styles 3417 3420
DocumentCardTitle with truncation 34713 34944
MenuButton 1511 1494
MenuButton (experiments) 3977 3950
PrimaryButton 1372 1386
PrimaryButton (experiments) 2308 2435
SplitButton 3229 3280
SplitButton (experiments) 8072 7929
Stack 526 533
Stack with Intrinsic children 1289 1280
Stack with Text children 4793 4855
Text 422 426
Toggle 1012 968
Toggle (experiments) 2691 2584
button 71 67

@dzearing

dzearing commented Oct 9, 2019

Copy link
Copy Markdown
Member

Looks like screener caught a regression; take a look at the failed screener result and check the problem state. :)

@xugao

xugao commented Oct 9, 2019

Copy link
Copy Markdown
Contributor Author

@dzearing

Looks like screener caught a regression; take a look at the failed screener result and check the problem state. :)

The changes look expected. according to the wiki, i need admin to approve these screener test failures. Could you help double check the changes and approve if look good? :)

P.S. the way github lets you reply a comment feels unintuitive to me . let me know if im doing this wrong :)

@msft-github-bot

Copy link
Copy Markdown
Contributor

Hello @cliffkoh!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msft-github-bot) and give me an instruction to get started! Learn more here.

@khmakoto khmakoto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Made one small suggestion but otherwise LGTM 👍

Comment thread packages/office-ui-fabric-react/src/components/Dropdown/Dropdown.base.tsx Outdated
@xugao
xugao requested a review from joschect October 9, 2019 22:52
@msft-github-bot
msft-github-bot merged commit 31396ed into microsoft:master Oct 9, 2019
@msft-github-bot

Copy link
Copy Markdown
Contributor

🎉office-ui-fabric-react@v7.48.0 has been released which incorporates this pull request.:tada:

Handy links:

pKalaga pushed a commit to pKalaga/fluentui that referenced this pull request Oct 16, 2019
…ed using keyboard navigation (microsoft#10755)* do not select first option by default for non-aria case* update snapshots* code cleanup* yarn change* add Aria doc link inline* resolve comment* simplify to use one class vars rather than two* resolve comments
pKalaga pushed a commit to pKalaga/fluentui that referenced this pull request Oct 16, 2019
unless it's selected using keyboard navigation (microsoft#10755)

* do not select first option by default for non-aria case
* update snapshots
* code cleanup
* yarn change
* add Aria doc link inline
* simplify to use one class vars rather than two
joschect pushed a commit that referenced this pull request Oct 17, 2019
unless it's selected using keyboard navigation (#10755)

* do not select first option by default for non-aria case
* update snapshots
* code cleanup
* yarn change
* add Aria doc link inline
* simplify to use one class vars rather than two
@mbest

mbest commented Mar 26, 2020

Copy link
Copy Markdown
Member

What is the "aria guidance" referenced above? It seems quite strange that tabbing through an item should change it.

@xugao
xugao deleted the dropdown-fix branch October 17, 2020 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dropdown calls onChange before callout is dismissed Dropdown with placeholder - first option automatically selected on focus.

7 participants