Skip to content

Commit 8e00897

Browse files
authored
Merge pull request #2524 from Akshat55/combobox-recursion
fix: Prevent too much recursion bug
2 parents b44ab14 + c23fcaf commit 8e00897

File tree

1 file changed

+48
-37
lines changed

1 file changed

+48
-37
lines changed

src/dropdown/list/dropdown-list.component.ts

Lines changed: 48 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ export class DropdownList implements AbstractDropdownView, AfterViewInit, OnDest
121121
/**
122122
* The list items belonging to the `DropdownList`.
123123
*/
124-
@Input() set items (value: Array<ListItem> | Observable<Array<ListItem>>) {
124+
@Input() set items(value: Array<ListItem> | Observable<Array<ListItem>>) {
125125
if (isObservable(value)) {
126126
if (this._itemsSubscription) {
127127
this._itemsSubscription.unsubscribe();
@@ -286,6 +286,11 @@ export class DropdownList implements AbstractDropdownView, AfterViewInit, OnDest
286286
filterBy(query = "") {
287287
if (query) {
288288
this.displayItems = this.getListItems().filter(item => item.content.toLowerCase().includes(query.toLowerCase()));
289+
// Reset index if items were found
290+
// Prevent selecting index in list that are undefined.
291+
if (this.displayItems) {
292+
this.index = 0;
293+
}
289294
} else {
290295
this.displayItems = this.getListItems();
291296
}
@@ -331,23 +336,26 @@ export class DropdownList implements AbstractDropdownView, AfterViewInit, OnDest
331336
* Returns the `HTMLElement` for the item that is subsequent to the selected item.
332337
*/
333338
getNextElement(): HTMLElement {
334-
if (this.index < this.displayItems.length - 1) {
335-
this.index++;
336-
}
337-
let item = this.displayItems[this.index];
338-
if (item.disabled) {
339-
return this.getNextElement();
339+
// Only return native elements if they are rendered
340+
const elemList = this.listElementList ? this.listElementList.toArray() : [];
341+
if (!elemList.length) {
342+
return null;
340343
}
341344

342-
let elemList = this.listElementList ? this.listElementList.toArray() : [];
343-
344-
// TODO: update to optional chaining after upgrading typescript
345-
// to v3.7+
346-
if (elemList[this.index] && elemList[this.index].nativeElement) {
347-
return elemList[this.index].nativeElement;
348-
} else {
349-
return null;
345+
/**
346+
* Start checking from next index
347+
* Continue looping through the list until a non disabeled element is found or
348+
* end of list is reached
349+
*/
350+
for (let i = this.index + 1; i < elemList.length; i++) {
351+
// If the values in the list are not disabled
352+
if (!this.displayItems[i].disabled) {
353+
this.index = i;
354+
return elemList[i].nativeElement;
355+
}
350356
}
357+
358+
return elemList[this.index].nativeElement;
351359
}
352360

353361
/**
@@ -371,23 +379,26 @@ export class DropdownList implements AbstractDropdownView, AfterViewInit, OnDest
371379
* Returns the `HTMLElement` for the item that precedes the selected item.
372380
*/
373381
getPrevElement(): HTMLElement {
374-
if (this.index > 0) {
375-
this.index--;
376-
}
377-
let item = this.displayItems[this.index];
378-
if (item.disabled) {
379-
return this.getPrevElement();
382+
// Only return native elements if they are rendered
383+
const elemList = this.listElementList ? this.listElementList.toArray() : [];
384+
if (!elemList.length) {
385+
return null;
380386
}
381387

382-
let elemList = this.listElementList ? this.listElementList.toArray() : [];
383-
384-
// TODO: update to optional chaining after upgrading typescript
385-
// to v3.7+
386-
if (elemList[this.index] && elemList[this.index].nativeElement) {
387-
return elemList[this.index].nativeElement;
388-
} else {
389-
return null;
388+
/**
389+
* Start checking from next index
390+
* Continue looping through the list until a non disabeled element is found or
391+
* end of list is reached
392+
*/
393+
for (let i = this.index - 1; i < this.index && i >= 0; i--) {
394+
// If the values in the list are not disabled
395+
if (!this.displayItems[i].disabled) {
396+
this.index = i;
397+
return elemList[i].nativeElement;
398+
}
390399
}
400+
401+
return elemList[this.index].nativeElement;
391402
}
392403

393404
/**
@@ -497,12 +508,12 @@ export class DropdownList implements AbstractDropdownView, AfterViewInit, OnDest
497508
doKeyDown(event: KeyboardEvent, item: ListItem) {
498509
// "Spacebar", "Down", and "Up" are IE specific values
499510
if (event.key === "Enter" || event.key === " " || event.key === "Spacebar") {
500-
if (this.listElementList.some(option => option.nativeElement === event.target)) {
501-
event.preventDefault();
502-
}
503-
if (event.key === "Enter") {
504-
this.doClick(event, item);
505-
}
511+
if (this.listElementList.some(option => option.nativeElement === event.target)) {
512+
event.preventDefault();
513+
}
514+
if (event.key === "Enter") {
515+
this.doClick(event, item);
516+
}
506517
} else if (event.key === "ArrowDown" || event.key === "ArrowUp" || event.key === "Down" || event.key === "Up") {
507518
event.preventDefault();
508519
if (event.key === "ArrowDown" || event.key === "Down") {
@@ -539,7 +550,7 @@ export class DropdownList implements AbstractDropdownView, AfterViewInit, OnDest
539550
event.preventDefault();
540551
if (event.key === "ArrowDown" || event.key === "Down") {
541552
if (this.hasNextElement()) {
542-
this.getNextElement().scrollIntoView({block: "end"});
553+
this.getNextElement().scrollIntoView({ block: "end" });
543554
} else {
544555
this.blurIntent.emit("bottom");
545556
}
@@ -561,7 +572,7 @@ export class DropdownList implements AbstractDropdownView, AfterViewInit, OnDest
561572
*/
562573
doClick(event, item) {
563574
event.preventDefault();
564-
if (!item.disabled) {
575+
if (item && !item.disabled) {
565576
this.list.nativeElement.focus();
566577
if (this.type === "single") {
567578
item.selected = true;

0 commit comments

Comments
 (0)