diff --git a/common/changes/office-ui-fabric-react/jspurlin-MakeComboBoxesWrap_2018-04-06-19-49.json b/common/changes/office-ui-fabric-react/jspurlin-MakeComboBoxesWrap_2018-04-06-19-49.json new file mode 100644 index 0000000000000..2c574e1c20136 --- /dev/null +++ b/common/changes/office-ui-fabric-react/jspurlin-MakeComboBoxesWrap_2018-04-06-19-49.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "office-ui-fabric-react", + "comment": "ComboBox: Make focus wrap in in the menu (to align with dropdwn and customer feedback)_", + "type": "patch" + } + ], + "packageName": "office-ui-fabric-react", + "email": "jspurlin@microsoft.com" +} \ No newline at end of file diff --git a/packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.test.tsx b/packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.test.tsx index c329c255558fe..2f7cf43124042 100644 --- a/packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.test.tsx +++ b/packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.test.tsx @@ -9,6 +9,7 @@ import { KeyCodes } from '../../Utilities'; import { ComboBox } from './ComboBox'; import { IComboBox, IComboBoxOption } from './ComboBox.types'; +import { SelectableOptionMenuItemType } from '../../utilities/selectableOption/SelectableOption.types'; import { expectOne, expectMissing } from '../../common/testUtilities'; const DEFAULT_OPTIONS: IComboBoxOption[] = [ @@ -22,6 +23,12 @@ const DEFAULT_OPTIONS2: IComboBoxOption[] = [ { key: '2', text: 'Foo' }, { key: '3', text: 'Bar' } ]; +const DEFAULT_OPTIONS3: IComboBoxOption[] = [ + { key: '0', text: 'Zero', itemType: SelectableOptionMenuItemType.Header }, + { key: '1', text: 'One' }, + { key: '2', text: 'Foo' }, + { key: '3', text: 'Bar' } +]; describe('ComboBox', () => { it('Renders ComboBox correctly', () => { @@ -268,6 +275,42 @@ describe('ComboBox', () => { expect(wrapper.find('input').props().value).toEqual('Foo'); }); + it('Can change selected option with keyboard, looping from top to bottom', () => { + const wrapper = mount( + ); + wrapper.find('input').simulate('keydown', { which: KeyCodes.up }); + wrapper.update(); + expect(wrapper.find('input').props().value).toEqual('Bar'); + }); + + it('Can change selected option with keyboard, looping from bottom to top', () => { + const wrapper = mount( + ); + wrapper.find('input').simulate('keydown', { which: KeyCodes.down }); + wrapper.update(); + expect(wrapper.find('input').props().value).toEqual('One'); + }); + + it('Can change selected option with keyboard, looping from top to bottom', () => { + const wrapper = mount( + ); + wrapper.find('input').simulate('keydown', { which: KeyCodes.up }); + wrapper.update(); + expect(wrapper.find('input').props().value).toEqual('Bar'); + }); + it('Cannot insert text while disabled', () => { const wrapper = mount( { title={ title } /> { /> - {isOpen && ( + { isOpen && ( (onRenderContainer as any)({ ...this.props, onRenderList, @@ -368,13 +368,13 @@ export class ComboBox extends BaseComponent { options: this.state.currentOptions.map((item, index) => ({ ...item, index: index })) }, this._onRenderContainer) - )} + ) } { errorMessage &&
- {errorMessage} + { errorMessage }
} @@ -481,7 +481,7 @@ export class ComboBox extends BaseComponent { } displayValues.push(currentPendingValue !== '' ? currentPendingValue : (this._indexWithinBounds(currentOptions, index) ? currentOptions[index].text : '')); } else { - for (let idx = 0; selectedIndices && (idx < selectedIndices.length); idx ++) { + for (let idx = 0; selectedIndices && (idx < selectedIndices.length); idx++) { const index: number = selectedIndices[idx]; displayValues.push(this._indexWithinBounds(currentOptions, index) ? currentOptions[index].text : suggestedDisplayValue); } @@ -933,11 +933,11 @@ export class ComboBox extends BaseComponent { return ( {
{ (onRenderList as any)({ ...props }, this._onRenderList) }
- {onRenderLowerContent(this.props, this._onRenderLowerContent)} + { onRenderLowerContent(this.props, this._onRenderLowerContent) }
); } @@ -967,12 +967,12 @@ export class ComboBox extends BaseComponent { const id = this._id; return (
- {options.map((item) => (onRenderItem as any)(item, this._onRenderItem))} + { options.map((item) => (onRenderItem as any)(item, this._onRenderItem)) }
); } @@ -1002,8 +1002,8 @@ export class ComboBox extends BaseComponent { return (
); } @@ -1039,8 +1039,8 @@ export class ComboBox extends BaseComponent { onMouseMove={ this._onOptionMouseMove.bind(this, item.index) } onMouseLeave={ this._onOptionMouseLeave } role='option' - aria-selected={ isSelected ? 'true' : 'false'} - ariaLabel= {item.text } + aria-selected={ isSelected ? 'true' : 'false' } + ariaLabel={ item.text } disabled={ item.disabled } > { { onRenderOption(item, this._onRenderOptionContent) } @@ -1048,23 +1048,23 @@ export class ComboBox extends BaseComponent { } ) : ( - - {onRenderOption(item, this._onRenderOptionContent)} - - ) + + { onRenderOption(item, this._onRenderOptionContent) } + + ) ); } @@ -1187,7 +1187,7 @@ export class ComboBox extends BaseComponent { private _onRenderOptionContent = (item: IComboBoxOption): JSX.Element => { const optionClassNames = getComboBoxOptionClassNames(this._getCurrentOptionStyles(item)); - return {item.text}; + return { item.text }; } /** @@ -1325,7 +1325,30 @@ export class ComboBox extends BaseComponent { currentOptions } = this.state; - index = this._getNextSelectableIndex(index, searchDirection); + // update index to allow content to wrap + if (searchDirection === SearchDirection.forward && index >= currentOptions.length - 1) { + index = -1; + } else if (searchDirection === SearchDirection.backward && index <= 0) { + index = currentOptions.length; + } + + // get the next "valid" index + const indexUpdate = this._getNextSelectableIndex(index, searchDirection); + + // if the two indicies are equal we didn't move and + // we should attempt to get get the first/last "valid" index to use + // (Note, this takes care of the potential cases where the first/last + // item is not focusable), otherwise use the updated index + if (index === indexUpdate) { + if (searchDirection === SearchDirection.forward) { + index = this._getNextSelectableIndex(-1, searchDirection); + } else if (searchDirection === SearchDirection.backward) { + index = this._getNextSelectableIndex(currentOptions.length, searchDirection); + } + } else { + index = indexUpdate; + } + if (this._indexWithinBounds(currentOptions, index)) { this._setPendingInfoFromIndex(index); }