Skip to content

Commit

Permalink
fix(ui5-combobox): Improve arrow navigation functionality (#3928)
Browse files Browse the repository at this point in the history 
* Enable arrow up on the first item to move the focus to the input

* Include valuestate header in the arrow chain

* On ENTER move the focus to the input, bug fixes

* Fix the big with the undesired closing of the popover when navigating fast through the items + refactoring of the arrow navigation logic

* Restore hash.txt

* Restore hash file

* Add tests

* Add more test, linting

* Do not focus selected suggestion after opening

* Restore visible focus on the input when typing and suggestions are opened

* Merge with master, converts the new tests to be async

* Fix arrow nav when the suggestions popover is closed

* Fix lint errors

* Refactoring

* Add more tests

* Retrigger build

* Fix item announcement

* Fix focus on closing with alt+arr

* Fix double focus on value state header when ESC is pressed and on user input
  • Loading branch information
@ndeshev
ndeshev committed Sep 29, 2021
1 parent 55eb7ba commit fea941a
Show file tree
Hide file tree
Showing 5 changed files with 285 additions and 29 deletions.
141 changes: 113 additions & 28 deletions packages/main/src/ComboBox.js
View file
Expand Up @@ -189,6 +189,14 @@ const metadata = {
type: Boolean,
},

/**
* Indicates whether the visual focus is on the value state header
* @private
*/
_isValueStateFocused: {
type: Boolean,
},

/**
* Sets the accessible aria name of the component.
*
Expand Down Expand Up @@ -390,13 +398,6 @@ class ComboBox extends UI5Element {
}

this._selectMatchingItem();

if (this._isKeyNavigation && this.responsivePopover && this.responsivePopover.opened) {
this.focused = false;
} else if (this.shadowRoot.activeElement) {
this.focused = this.shadowRoot.activeElement.id === "ui5-combobox-input";
}

this._initialRendering = false;
this._isKeyNavigation = false;
}
Expand All @@ -410,15 +411,16 @@ class ComboBox extends UI5Element {

if (this.shouldClosePopover() && !isPhone()) {
this.responsivePopover.close(false, false, true);
this._clearFocus();
this._itemFocused = false;
}

this._itemFocused = false;
this.toggleValueStatePopover(this.shouldOpenValueStateMessagePopover);
this.storeResponsivePopoverWidth();
}

shouldClosePopover() {
return this.responsivePopover.opened && !this.focused && !this._itemFocused;
return this.responsivePopover.opened && !this.focused && !this._itemFocused && !this._isValueStateFocused;
}

_focusin(event) {
Expand Down Expand Up @@ -519,6 +521,8 @@ class ComboBox extends UI5Element {
if (event.target === this.inner) {
// stop the native event, as the semantic "input" would be fired.
event.stopImmediatePropagation();
this.focused = true;
this._isValueStateFocused = false;
}

this._filteredItems = this._filterItems(value);
Expand All @@ -538,8 +542,6 @@ class ComboBox extends UI5Element {
});

this._selectionChanged = false;

item.focused = true;
}
}

Expand Down Expand Up @@ -573,40 +575,62 @@ class ComboBox extends UI5Element {
return;
}

const isOpen = this.open;
const isArrowDown = isDown(event);
const isArrowUp = isUp(event);
const currentItem = this._filteredItems.find(item => {
return this.responsivePopover.opened ? item.focused : item.selected;
return isOpen ? item.focused : item.selected;
});
let indexOfItem = this._filteredItems.indexOf(currentItem);
const indexOfItem = this._filteredItems.indexOf(currentItem);

event.preventDefault();

if ((indexOfItem === 0 && isArrowUp) || (this._filteredItems.length - 1 === indexOfItem && isArrowDown)) {
if ((this.focused === true && isArrowUp && isOpen) || (this._filteredItems.length - 1 === indexOfItem && isArrowDown)) {
return;
}

this._clearFocus();
this._isKeyNavigation = true;

indexOfItem += isArrowDown ? 1 : -1;
indexOfItem = indexOfItem < 0 ? 0 : indexOfItem;
this._filteredItems[indexOfItem].focused = true;
if (isArrowDown) {
this._handleArrowDown(event, indexOfItem);
}

if (this.responsivePopover.opened) {
this.announceSelectedItem(indexOfItem);
if (isArrowUp) {
this._handleArrowUp(event, indexOfItem);
}
}

this.value = this._filteredItems[indexOfItem].isGroupItem ? this.filterValue : this._filteredItems[indexOfItem].text;
_handleItemNavigation(event, indexOfItem, isForward) {
const isOpen = this.open;
const currentItem = this._filteredItems[indexOfItem];
const nextItem = isForward ? this._filteredItems[indexOfItem + 1] : this._filteredItems[indexOfItem - 1];
const isGroupItem = currentItem && currentItem.isGroupItem;

this._isKeyNavigation = true;
this._itemFocused = true;
if ((!isOpen) && ((isGroupItem && !nextItem) || (!isGroupItem && !currentItem))) {
return;
}

this._clearFocus();

if (isOpen) {
this._itemFocused = true;
this.value = isGroupItem ? this.filterValue : currentItem.text;
this.focused = false;
currentItem.focused = true;
} else {
this.focused = true;
this.value = isGroupItem ? nextItem.text : currentItem.text;
currentItem.focused = false;
}

this._isValueStateFocused = false;
this._selectionChanged = true;

if (this._filteredItems[indexOfItem].isGroupItem) {
if (isGroupItem && isOpen) {
return;
}

this._filteredItems[indexOfItem].selected = true;
this._announceSelectedItem(indexOfItem);

// autocomplete
const item = this._autoCompleteValue(this.value);
Expand All @@ -621,6 +645,48 @@ class ComboBox extends UI5Element {
this._fireChangeEvent();
}

_handleArrowDown(event, indexOfItem) {
const isOpen = this.open;

if (this.focused && indexOfItem === -1 && this.hasValueStateText && isOpen) {
this._isValueStateFocused = true;
this.focused = false;
return;
}

indexOfItem = !isOpen && this.hasValueState && indexOfItem === -1 ? 0 : indexOfItem;

this._handleItemNavigation(event, ++indexOfItem, true /* isForward */);
}

_handleArrowUp(event, indexOfItem) {
const isOpen = this.open;

if (indexOfItem === 0 && !this.hasValueStateText) {
this._clearFocus();
this.focused = true;
this._itemFocused = false;
return;
}

if (indexOfItem === 0 && this.hasValueStateText && isOpen) {
this._clearFocus();
this._itemFocused = false;
this._isValueStateFocused = true;
this._filteredItems[0].selected = false;
return;
}

if (this._isValueStateFocused) {
this.focused = true;
this._isValueStateFocused = false;
return;
}

indexOfItem = !isOpen && this.hasValueState && indexOfItem === -1 ? 0 : indexOfItem;
this._handleItemNavigation(event, --indexOfItem, false /* isForward */);
}

_keydown(event) {
const isArrowKey = isDown(event) || isUp(event);
this._autocomplete = !(isBackSpace(event) || isDelete(event));
Expand All @@ -632,16 +698,32 @@ class ComboBox extends UI5Element {
if (isEnter(event)) {
this._fireChangeEvent();
this._closeRespPopover();
this.focused = true;
}

if (isEscape(event) && !this.open) {
this.value = this._lastValue;
if (isEscape(event)) {
this.focused = true;
this.value = !this.open ? this._lastValue : this.value;
this._isValueStateFocused = false;
}

if (isShow(event) && !this.readonly && !this.disabled) {
event.preventDefault();

this._resetFilter();
this._toggleRespPopover();

const selectedItem = this._filteredItems.find(item => {
return item.selected;
});

if (selectedItem && this.open) {
this._itemFocused = true;
selectedItem.focused = true;
this.focused = false;
} else {
this.focused = true;
}
}
}

Expand All @@ -657,6 +739,9 @@ class ComboBox extends UI5Element {
this.filterValue = this._selectedItemText;
}

this._isValueStateFocused = false;
this._clearFocus();

this.responsivePopover.close();
}

Expand Down Expand Up @@ -786,7 +871,7 @@ class ComboBox extends UI5Element {
this._itemFocused = true;
}

announceSelectedItem(indexOfItem) {
_announceSelectedItem(indexOfItem) {
const itemPositionText = this.i18nBundle.getText(LIST_ITEM_POSITION, [indexOfItem + 1], [this._filteredItems.length]);
const itemSelectionText = this.i18nBundle.getText(LIST_ITEM_SELECTED);

Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/ComboBoxPopover.hbs
View file
Expand Up @@ -54,7 +54,7 @@

{{#unless _isPhone}}
{{#if hasValueStateText}}
<div class="ui5-responsive-popover-header {{classes.popoverValueState}}" style="{{styles.suggestionPopoverHeader}}">
<div class="ui5-responsive-popover-header {{classes.popoverValueState}}" ?focused={{_isValueStateFocused}} tabindex="0" style="{{styles.suggestionPopoverHeader}}">
{{> valueStateMessage}}
</div>
{{/if}}
Expand Down
7 changes: 7 additions & 0 deletions packages/main/src/themes/ValueStateMessage.css
View file
Expand Up @@ -36,3 +36,10 @@ ui5-responsive-popover .ui5-valuestatemessage-header {
.ui5-valuestatemessage--information {
background: var(--sapInformationBackground);
}

.ui5-responsive-popover-header[focused] {
outline-width: 0.0625rem;
outline-style: dotted;
outline-offset: -0.125rem;
outline-color: initial;
}
19 changes: 19 additions & 0 deletions packages/main/test/pages/ComboBox.html
View file
Expand Up @@ -99,6 +99,25 @@
<ui5-combobox id="" style="width: 360px;" value-state="Success"></ui5-combobox>
</div>

<div class="demo-section">
<ui5-label id="combo-vs-grouping-label">Items with grouping and value state:</ui5-label>

<ui5-combobox id="value-state-grouping" value-state="Warning">
<ui5-cb-group-item text="A"></ui5-cb-group-item>
<ui5-cb-item text="Argentina"></ui5-cb-item>
<ui5-cb-item text="Australia"></ui5-cb-item>
<ui5-cb-item text="Austria"></ui5-cb-item>
<ui5-cb-group-item text="B"></ui5-cb-group-item>
<ui5-cb-item text="Bahrain"></ui5-cb-item>
<ui5-cb-item text="Belgium"></ui5-cb-item>
<ui5-cb-item text="Brazil"></ui5-cb-item>
<ui5-cb-group-item text="C"></ui5-cb-group-item>
<ui5-cb-item text="Canada"></ui5-cb-item>
<ui5-cb-item text="Chile"></ui5-cb-item>
</ui5-combobox>
</div>


<div class="demo-section">
<ui5-label id="combo-grouping-label">Items with grouping:</ui5-label>
<ui5-combobox filter="StartsWith" id="combo-grouping" style="width: 360px;" aria-label="Select destination:">
Expand Down

0 comments on commit fea941a

Please sign in to comment.