-
Notifications
You must be signed in to change notification settings - Fork 2.9k
ComboBox: Option to use ariaLabel prop as preview text #4540
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
Changes from 7 commits
94c1ad1
ab295d5
a09419a
6d2e481
a548107
6cb5656
b3168ea
bf663ba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| { | ||
| "changes": [ | ||
| { | ||
| "packageName": "office-ui-fabric-react", | ||
| "comment": "Adding the ability to display the ariaLabel instead of the option text. This is necessary for cases where embedded text is used as the option text", | ||
| "type": "patch" | ||
| } | ||
| ], | ||
| "packageName": "office-ui-fabric-react", | ||
| "email": "kysedate@microsoft.com" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -583,18 +583,21 @@ export class ComboBox extends BaseComponent<IComboBoxProps, IComboBoxState> { | |
| if (this.props.autoComplete === 'on') { | ||
|
|
||
| // If autoComplete is on, attempt to find a match where the text of an option starts with the updated value | ||
| const items = currentOptions.map((item, index) => { return { ...item, index }; }).filter((option) => option.itemType !== SelectableOptionMenuItemType.Header && option.itemType !== SelectableOptionMenuItemType.Divider).filter((option) => option.text.toLocaleLowerCase().indexOf(updatedValue) === 0); | ||
| const items = currentOptions.map((item, index) => { return { ...item, index }; }).filter((option) => option.itemType !== SelectableOptionMenuItemType.Header && option.itemType !== SelectableOptionMenuItemType.Divider).filter((option) => this._getPreviewText(option).toLocaleLowerCase().indexOf(updatedValue) === 0); | ||
| if (items.length > 0) { | ||
| // use ariaLabel as the value when the option is set | ||
| const text: string = this._getPreviewText(items[0]); | ||
|
|
||
| // If the user typed out the complete option text, we don't need any suggested display text anymore | ||
| newSuggestedDisplayValue = items[0].text.toLocaleLowerCase() !== updatedValue ? items[0].text : ''; | ||
| newSuggestedDisplayValue = text.toLocaleLowerCase() !== updatedValue ? text : ''; | ||
|
|
||
| // remember the index of the match we found | ||
| newCurrentPendingValueValidIndex = items[0].index; | ||
| } | ||
| } else { | ||
|
|
||
| // If autoComplete is off, attempt to find a match only when the value is exactly equal to the text of an option | ||
| const items = currentOptions.map((item, index) => { return { ...item, index }; }).filter((option) => option.itemType !== SelectableOptionMenuItemType.Header && option.itemType !== SelectableOptionMenuItemType.Divider).filter((option) => option.text.toLocaleLowerCase() === updatedValue); | ||
| const items = currentOptions.map((item, index) => { return { ...item, index }; }).filter((option) => option.itemType !== SelectableOptionMenuItemType.Header && option.itemType !== SelectableOptionMenuItemType.Divider).filter((option) => this._getPreviewText(option).toLocaleLowerCase() === updatedValue); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same comment here |
||
|
|
||
| // if we fould a match remember the index | ||
| if (items.length === 1) { | ||
|
|
@@ -646,7 +649,7 @@ export class ComboBox extends BaseComponent<IComboBoxProps, IComboBoxState> { | |
|
|
||
| // If we found a match, udpdate the state | ||
| if (items.length > 0) { | ||
| this._setPendingInfo(originalUpdatedValue, items[0].index, items[0].text); | ||
| this._setPendingInfo(originalUpdatedValue, items[0].index, this._getPreviewText(items[0])); | ||
| } | ||
|
|
||
| // Schedule a timeout to clear the pending value after the timeout span | ||
|
|
@@ -1040,7 +1043,7 @@ export class ComboBox extends BaseComponent<IComboBoxProps, IComboBoxState> { | |
| onMouseLeave={ this._onOptionMouseLeave } | ||
| role='option' | ||
| aria-selected={ isSelected ? 'true' : 'false' } | ||
| ariaLabel={ item.text } | ||
| ariaLabel={ this._getPreviewText(item) } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about the multiSelect case?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure what you mean..
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the checkbox below seems to be missing the ariaLabel... we should fix that |
||
| disabled={ item.disabled } | ||
| > { <span ref={ isSelected ? this._selectedElement : undefined }> | ||
| { onRenderOption(item, this._onRenderOptionContent) } | ||
|
|
@@ -1307,7 +1310,7 @@ export class ComboBox extends BaseComponent<IComboBoxProps, IComboBoxState> { | |
|
|
||
| if (index >= 0 && index < currentOptions.length) { | ||
| const option = currentOptions[index]; | ||
| this._setPendingInfo(option.text, index, option.text); | ||
| this._setPendingInfo(this._getPreviewText(option), index, this._getPreviewText(option)); | ||
| } else { | ||
| this._clearPendingInfo(); | ||
| } | ||
|
|
@@ -1764,4 +1767,8 @@ export class ComboBox extends BaseComponent<IComboBoxProps, IComboBoxState> { | |
|
|
||
| return retKeys; | ||
| } | ||
|
|
||
| private _getPreviewText(item: IComboBoxOption): string { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this seems like a good place to add a comment explaining why are we doing this |
||
| return item.useAriaLabelAsText && item.ariaLabel ? item.ariaLabel : item.text; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,12 @@ export interface IComboBoxOption extends ISelectableOption { | |
| * the prop comboBoxOptionStyles | ||
| */ | ||
| styles?: Partial<IComboBoxOptionStyles>; | ||
|
|
||
| /** | ||
| * In scenarios where embedded data is used at the text prop, we will use the ariaLabel prop to set the aria-label and preview text. Default to false | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIT: wrap this to multiple lines as well |
||
| * @default false; | ||
| */ | ||
| useAriaLabelAsText?: boolean; | ||
| } | ||
|
|
||
| export interface IComboBoxProps extends ISelectableDroppableTextProps<IComboBox> { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is really long, consider breaking it up across multiple line