Combobox: Fixes multiselect selected values not being read #11436

joschect · 2019-12-11T22:41:20Z

Pull request checklist

Addresses an existing issue:
Fixes ComboBox with multiple selected values are not read aloud #10718
Fixes [Accessibility][Functional]: On keyboard focus selected value disappears from the editable multiselect combo box. #11195
Fixes [Accessibility] Narrator is not announcing values of "Multi-select uncontrolled" ComboBox #10910
Include a change request file using $ yarn change

Description of changes

This fixes 2 accessibility problems with multiselect combobox.

Selected options in the input would disappear when a user entered into it, this was confusing as it appeared that the current selection is gone. This is not a perfect solution but it does enable the user to have some knowledge that their selected options aren't gone.
That selected options weren't read out. They are now added to the label so the selected options are read.

Focus areas to test

One big potential problem with this change is that it removes a zero width space that may have been used to help prevent bad values from being returned in onChange and fixed issue #3055. I believe it's safe to remove because I tested both locally and added a test to check for it and at no point was an incorrect value returned. Additionally the zero width space was almost always stripped out before the value could be saved so it's unlikely that users interacted/relied on it.

I verified that you can still select/save an empty value if freeform is enabled and that the combobox still interacts with an empty option correctly.

Microsoft Reviewers: Open in CodeFlow

size-auditor · 2019-12-11T23:52:11Z

Asset size changes

Project	Bundle	Baseline Size	New Size	Difference
office-ui-fabric-react	ComboBox	227.852 kB	228.275 kB	423 bytes

Over Tolerance (1024 B) Over Baseline Below Baseline New Removed 1 kB = 1000 B

Baseline commit: 2150212f54760164dd08b3781f80c3ecfad6ca14 (build)

change/office-ui-fabric-react-2019-12-11-14-23-30-combobox-access.json

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx

jspurlin · 2019-12-12T19:27:31Z

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx

    } = this.props;
    const { isOpen, focused, suggestedDisplayValue } = this.state;
    this._currentVisibleValue = this._getVisibleValue();
+    const multiselectPlaceholder = multiSelect
+      ? this._getMultiselectDisplayString(this.state.selectedIndices, this.state.currentOptions, suggestedDisplayValue)
+      : '';


''; [](start = 8, length = 3)

Should this be set to undefined instead of an empty string?

Maybe. I'll change it and see.

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx

jspurlin · 2019-12-12T19:45:20Z

}

Does this fn (and where it's called) still need to be in the code?

Refers to: packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx:2130 in 0c5019f. [](commit_id = 0c5019f, deletion_comment = False)

jspurlin · 2019-12-12T19:48:36Z

This is slightly scary... but I trust your validation :)

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.test.tsx

joschect · 2019-12-12T22:50:46Z

}

Does this fn (and where it's called) still need to be in the code?

Refers to: packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx:2130 in 0c5019f. [](commit_id = 0c5019f, deletion_comment = False)

I'm having a little trouble finding what you're referencing here. Could you point me to it?

jspurlin · 2019-12-12T23:15:10Z

}

Does this fn (and where it's called) still need to be in the code?
Refers to: packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx:2130 in 0c5019f. [](commit_id = 0c5019f, deletion_comment = False)

I'm having a little trouble finding what you're referencing here. Could you point me to it?

(I selected the code and commented, it seems to have just chosen the last line...)

This is referring to the _removeZeroWidthSpaces function

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.classNames.ts

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.types.ts

change/office-ui-fabric-react-2019-12-11-14-23-30-combobox-access.json

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx

…ui-fabric-react into combobox-access

msft-github-bot · 2019-12-18T21:14:25Z

Component Perf Analysis

No significant results to display.

All results

Scenario	Master Ticks	PR Ticks
BaseButton	742	693
BaseButton (experiments)	982	953
DefaultButton	1027	977
DefaultButton (experiments)	1896	1949
DetailsRow	3304	3214
DetailsRow (fast icons)	3310	3326
DetailsRow without styles	3137	3041
DocumentCardTitle with truncation	32600	32240
MenuButton	1284	1301
MenuButton (experiments)	3505	3427
PrimaryButton	1204	1153
PrimaryButton (experiments)	1981	1985
SplitButton	2755	2781
SplitButton (experiments)	6767	6866
Stack	467	473
Stack with Intrinsic children	1152	1090
Stack with Text children	4166	4230
Text	379	383
Toggle	829	858
Toggle (experiments)	2221	2198
button	64	58

msft-github-bot · 2019-12-19T12:33:42Z

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

Handy links:

joschect added 6 commits December 11, 2019 10:20

fix combobox

de32b5d

fix aria-label

fe8d601

add some more tests, minor refactoring

1c107f4

Change files

9967b38

Merge branch 'master' into combobox-access

1ae31d2

minor comment tweak

f96bcde

joschect requested review from jdhuntington, xugao, micahgodbolt, Markionium, jspurlin and marygans December 11, 2019 22:41

msft-github-bot added the Component: Combobox label Dec 11, 2019

msft-github-bot assigned xugao Dec 11, 2019

joschect requested review from cschlechty, Vitalius1, JasonGore and ecraig12345 December 11, 2019 22:43

joschect added 3 commits December 11, 2019 14:53

add missing letter in string'

02f586d

update api

a308703

update snapshots

0c5019f

Vitalius1 reviewed Dec 12, 2019

View reviewed changes

change/office-ui-fabric-react-2019-12-11-14-23-30-combobox-access.json Outdated Show resolved Hide resolved

Vitalius1 reviewed Dec 12, 2019

View reviewed changes

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx Outdated Show resolved Hide resolved

jspurlin reviewed Dec 12, 2019

View reviewed changes

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx Outdated Show resolved Hide resolved

jspurlin reviewed Dec 12, 2019

View reviewed changes

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx Outdated Show resolved Hide resolved

jdhuntington reviewed Dec 12, 2019

View reviewed changes

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.test.tsx Show resolved Hide resolved

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.test.tsx Show resolved Hide resolved

fix comments

bcd9763

khmakoto reviewed Dec 13, 2019

View reviewed changes

joschect added 2 commits December 17, 2019 10:05

minor comment fixes

8de9436

Merge remote-tracking branch 'upstream/master' into combobox-access

07f9ec3

jspurlin approved these changes Dec 17, 2019

View reviewed changes

update snapshots

2747d9f

aneeshack4 approved these changes Dec 17, 2019

View reviewed changes

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx Show resolved Hide resolved

Add comment

6f48cc1

khmakoto approved these changes Dec 17, 2019

View reviewed changes

Update office-ui-fabric-react-2019-12-11-14-23-30-combobox-access.json

8f54cf2

micahgodbolt closed this Dec 18, 2019

micahgodbolt reopened this Dec 18, 2019

joschect added 3 commits December 18, 2019 10:30

Merge remote-tracking branch 'upstream/master' into combobox-access

0034bfa

Merge branch 'combobox-access' of https://github.com/joschect/office-…

7bc97d4

…ui-fabric-react into combobox-access

fix bad variable name

d7e92a1

joschect merged commit ccedc92 into microsoft:master Dec 18, 2019

This was referenced Jan 13, 2020

Combo Box multi select text is not consistent with dropdown #8893

Closed

ComboBox input field is empty while dropdown is visible #7558

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Combobox: Fixes multiselect selected values not being read #11436

Combobox: Fixes multiselect selected values not being read #11436

joschect commented Dec 11, 2019 •

edited by khmakoto

Loading

size-auditor bot commented Dec 11, 2019 •

edited

Loading

jspurlin Dec 12, 2019

joschect Dec 12, 2019

jspurlin commented Dec 12, 2019

jspurlin commented Dec 12, 2019

joschect commented Dec 12, 2019

jspurlin commented Dec 12, 2019 •

edited

Loading

msft-github-bot commented Dec 18, 2019

msft-github-bot commented Dec 19, 2019

Combobox: Fixes multiselect selected values not being read #11436

Combobox: Fixes multiselect selected values not being read #11436

Conversation

joschect commented Dec 11, 2019 • edited by khmakoto Loading

Pull request checklist

Description of changes

Focus areas to test

Microsoft Reviewers: Open in CodeFlow

size-auditor bot commented Dec 11, 2019 • edited Loading

Asset size changes

jspurlin Dec 12, 2019

Choose a reason for hiding this comment

joschect Dec 12, 2019

Choose a reason for hiding this comment

jspurlin commented Dec 12, 2019

jspurlin commented Dec 12, 2019

joschect commented Dec 12, 2019

jspurlin commented Dec 12, 2019 • edited Loading

msft-github-bot commented Dec 18, 2019

Component Perf Analysis

msft-github-bot commented Dec 19, 2019

joschect commented Dec 11, 2019 •

edited by khmakoto

Loading

size-auditor bot commented Dec 11, 2019 •

edited

Loading

jspurlin commented Dec 12, 2019 •

edited

Loading