Skip to content

Commit a1e763d

Browse files
ndeshevndeshev
authored
fix(ui5-combobox): fire selection-change event on item selection (#12036)
fixes: #12003, #10239. ui5-selection-change event is now also fired when: - an item is selected after backspace/delete input interaction - an item selection is cleared, either by clicking the clear icon or by pressing backspace/delete on an empty input
1 parent 1cc86a5 commit a1e763d

File tree

3 files changed

+328
-10
lines changed

3 files changed

+328
-10
lines changed

‎packages/main/cypress/specs/ComboBox.cy.tsx

Lines changed: 273 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,3 +458,276 @@ describe("Accessibility", () => {
458458
});
459459
});
460460
});
461+
462+
describe("Selection-change event", () => {
463+
it("should fire selection-change event when item is selected", () => {
464+
cy.mount(
465+
<ComboBox>
466+
<ComboBoxItem text="Argentina"></ComboBoxItem>
467+
<ComboBoxItem text="Bulgaria"></ComboBoxItem>
468+
<ComboBoxItem text="Canada"></ComboBoxItem>
469+
</ComboBox>
470+
);
471+
472+
cy.get("[ui5-combobox]")
473+
.as("combo")
474+
.invoke('on', 'ui5-selection-change', cy.spy().as('selectionChangeSpy'));
475+
476+
cy.get("@combo")
477+
.shadow()
478+
.find("input")
479+
.as("input");
480+
481+
// Type to select an item
482+
cy.get("@input")
483+
.focus()
484+
.realType("Bulgaria");
485+
486+
cy.get("@selectionChangeSpy")
487+
.should("have.been.calledOnce");
488+
489+
cy.get("@selectionChangeSpy")
490+
.should("have.been.calledWith", Cypress.sinon.match.has("detail", Cypress.sinon.match.has("item")));
491+
});
492+
493+
it("should fire selection-change event when item is selected after backspace", () => {
494+
cy.mount(
495+
<ComboBox>
496+
<ComboBoxItem text="Argentina"></ComboBoxItem>
497+
<ComboBoxItem text="Bulgaria"></ComboBoxItem>
498+
<ComboBoxItem text="Canada"></ComboBoxItem>
499+
</ComboBox>
500+
);
501+
502+
cy.get("[ui5-combobox]")
503+
.as("combo")
504+
.invoke('on', 'ui5-selection-change', cy.spy().as('selectionChangeSpy'));
505+
506+
cy.get("@combo")
507+
.shadow()
508+
.find("input")
509+
.as("input");
510+
511+
// Type partial text, then backspace, then complete selection
512+
cy.get("@input")
513+
.focus()
514+
.realType("Bulg")
515+
.realPress("Backspace")
516+
517+
cy.get("@selectionChangeSpy")
518+
.should("have.been.called");
519+
520+
cy.get("@selectionChangeSpy").should("have.been.calledWithMatch", Cypress.sinon.match(event => {
521+
return event.detail.item === null;
522+
}));
523+
});
524+
525+
it("should fire selection-change event when item is selected after delete key", () => {
526+
cy.mount(
527+
<ComboBox>
528+
<ComboBoxItem text="Argentina"></ComboBoxItem>
529+
<ComboBoxItem text="Bulgaria"></ComboBoxItem>
530+
<ComboBoxItem text="Canada"></ComboBoxItem>
531+
</ComboBox>
532+
);
533+
534+
cy.get("[ui5-combobox]")
535+
.as("combo")
536+
.invoke('on', 'ui5-selection-change', cy.spy().as('selectionChangeSpy'));
537+
538+
cy.get("@combo")
539+
.shadow()
540+
.find("input")
541+
.as("input");
542+
543+
// Type text, position cursor, use delete key, then complete selection
544+
cy.get("@input")
545+
.focus()
546+
.realType("Bulgxaria")
547+
.realPress(["Home"])
548+
.realPress(["ArrowRight", "ArrowRight", "ArrowRight", "ArrowRight"])
549+
.realPress("Delete")
550+
.realPress("End");
551+
552+
cy.get("@selectionChangeSpy")
553+
.should("have.been.called");
554+
555+
cy.get("@selectionChangeSpy")
556+
.should("have.been.calledWith", Cypress.sinon.match.has("detail", Cypress.sinon.match.has("item")));
557+
});
558+
559+
it("should fire selection-change event when selection is cleared by typing non-matching text", () => {
560+
cy.mount(
561+
<ComboBox value="Bulgaria">
562+
<ComboBoxItem text="Argentina"></ComboBoxItem>
563+
<ComboBoxItem text="Bulgaria" selected></ComboBoxItem>
564+
<ComboBoxItem text="Canada"></ComboBoxItem>
565+
</ComboBox>
566+
);
567+
568+
cy.get("[ui5-combobox]")
569+
.as("combo")
570+
.invoke('on', 'ui5-selection-change', cy.spy().as('selectionChangeSpy'));
571+
572+
cy.get("@combo")
573+
.shadow()
574+
.find("input")
575+
.as("input");
576+
577+
// Clear existing value and type non-matching text
578+
cy.get("@input")
579+
.focus()
580+
.clear()
581+
.realType("NonExistentCountry");
582+
583+
cy.get("@selectionChangeSpy")
584+
.should("have.been.called");
585+
586+
cy.get("@selectionChangeSpy")
587+
.should("have.been.calledWith", Cypress.sinon.match.has("detail", Cypress.sinon.match.has("item")));
588+
});
589+
590+
it("should fire selection-change event when clear icon is clicked", () => {
591+
cy.mount(
592+
<ComboBox value="Bulgaria" showClearIcon>
593+
<ComboBoxItem text="Argentina"></ComboBoxItem>
594+
<ComboBoxItem text="Bulgaria" selected></ComboBoxItem>
595+
<ComboBoxItem text="Canada"></ComboBoxItem>
596+
</ComboBox>
597+
);
598+
599+
cy.get("[ui5-combobox]")
600+
.as("combo")
601+
.invoke('on', 'ui5-selection-change', cy.spy().as('selectionChangeSpy'));
602+
603+
// Click the clear icon
604+
cy.get("@combo")
605+
.shadow()
606+
.find(".ui5-input-clear-icon-wrapper")
607+
.click();
608+
609+
cy.get("@selectionChangeSpy")
610+
.should("have.been.calledOnce");
611+
612+
cy.get("@selectionChangeSpy")
613+
.should("have.been.calledWith", Cypress.sinon.match.has("detail", Cypress.sinon.match.has("item")));
614+
});
615+
616+
it("should fire selection-change event when item is clicked from dropdown", () => {
617+
cy.mount(
618+
<ComboBox>
619+
<ComboBoxItem text="Argentina"></ComboBoxItem>
620+
<ComboBoxItem text="Bulgaria"></ComboBoxItem>
621+
<ComboBoxItem text="Canada"></ComboBoxItem>
622+
</ComboBox>
623+
);
624+
625+
cy.get("[ui5-combobox]")
626+
.as("combo")
627+
.invoke('on', 'ui5-selection-change', cy.spy().as('selectionChangeSpy'));
628+
629+
// Open dropdown and click an item
630+
cy.get("@combo")
631+
.shadow()
632+
.find("ui5-icon")
633+
.click();
634+
635+
cy.get("@combo")
636+
.find("ui5-cb-item")
637+
.eq(1)
638+
.click();
639+
640+
cy.get("@selectionChangeSpy")
641+
.should("have.been.calledOnce");
642+
643+
cy.get("@selectionChangeSpy")
644+
.should("have.been.calledWith", Cypress.sinon.match.has("detail", Cypress.sinon.match.has("item")));
645+
});
646+
647+
it("should fire selection-change event when navigating with arrow keys and pressing Enter", () => {
648+
cy.mount(
649+
<ComboBox>
650+
<ComboBoxItem text="Argentina"></ComboBoxItem>
651+
<ComboBoxItem text="Bulgaria"></ComboBoxItem>
652+
<ComboBoxItem text="Canada"></ComboBoxItem>
653+
</ComboBox>
654+
);
655+
656+
cy.get("[ui5-combobox]")
657+
.as("combo")
658+
.invoke('on', 'ui5-selection-change', cy.spy().as('selectionChangeSpy'));
659+
660+
cy.get("@combo")
661+
.shadow()
662+
.find("input")
663+
.as("input");
664+
665+
// Open dropdown with F4, navigate with arrow keys, and select with Enter
666+
cy.get("@input")
667+
.focus()
668+
.realPress("F4")
669+
.realPress("ArrowDown")
670+
.realPress("Enter");
671+
672+
cy.get("@selectionChangeSpy")
673+
.should("have.been.calledWith", Cypress.sinon.match.has("detail", Cypress.sinon.match.has("item")));
674+
});
675+
676+
it("should fire selection-change event with correct item data", () => {
677+
cy.mount(
678+
<ComboBox>
679+
<ComboBoxItem text="Argentina"></ComboBoxItem>
680+
<ComboBoxItem text="Bulgaria"></ComboBoxItem>
681+
<ComboBoxItem text="Canada"></ComboBoxItem>
682+
</ComboBox>
683+
);
684+
685+
cy.get("[ui5-combobox]")
686+
.as("combo")
687+
.invoke('on', 'ui5-selection-change', cy.spy().as('selectionChangeSpy'));
688+
689+
cy.get("@combo")
690+
.shadow()
691+
.find("input")
692+
.focus()
693+
.realType("Bulgaria");
694+
695+
cy.get("@selectionChangeSpy").should("have.been.calledWithMatch", Cypress.sinon.match(event => {
696+
return event.detail.item.text === "Bulgaria";
697+
}));
698+
});
699+
700+
it("should work correctly with grouped items", () => {
701+
cy.mount(
702+
<ComboBox>
703+
<ComboBoxItemGroup headerText="Europe">
704+
<ComboBoxItem text="Bulgaria"></ComboBoxItem>
705+
<ComboBoxItem text="Germany"></ComboBoxItem>
706+
</ComboBoxItemGroup>
707+
<ComboBoxItemGroup headerText="Americas">
708+
<ComboBoxItem text="Argentina"></ComboBoxItem>
709+
<ComboBoxItem text="Canada"></ComboBoxItem>
710+
</ComboBoxItemGroup>
711+
</ComboBox>
712+
);
713+
714+
cy.get("[ui5-combobox]")
715+
.as("combo");
716+
717+
cy.get("ui5-combobox")
718+
.as("combo")
719+
.invoke('on', 'ui5-selection-change', cy.spy().as('selectionChangeSpy'));
720+
721+
cy.get("@combo")
722+
.shadow()
723+
.find("input")
724+
.focus()
725+
.realType("Bulgaria");
726+
727+
cy.get("@selectionChangeSpy")
728+
.should("have.been.calledOnce");
729+
730+
cy.get("@selectionChangeSpy")
731+
.should("have.been.calledWith", Cypress.sinon.match.has("detail", Cypress.sinon.match.has("item")));
732+
});
733+
});

‎packages/main/src/ComboBox.ts

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ enum ValueStateIconMapping {
118118
}
119119

120120
type ComboBoxSelectionChangeEventDetail = {
121-
item: ComboBoxItem,
121+
item: ComboBoxItem | null,
122122
};
123123

124124
/**
@@ -706,7 +706,7 @@ class ComboBox extends UI5Element implements IFormInputElement {
706706

707707
// autocomplete
708708
if (shouldAutocomplete && !isAndroid()) {
709-
this._handleTypeAhead(value, value, true);
709+
this._handleTypeAhead(value, value);
710710
}
711711

712712
this.fireDecoratorEvent("input");
@@ -845,25 +845,19 @@ class ComboBox extends UI5Element implements IFormInputElement {
845845
return;
846846
}
847847
// autocomplete
848-
this._handleTypeAhead(this.value, this.open ? this._userTypedValue : "", false);
848+
this._handleTypeAhead(this.value, this.open ? this._userTypedValue : "");
849849

850850
this.fireDecoratorEvent("input");
851851
}
852852

853-
_handleTypeAhead(value: string, filterValue: string, checkForGroupItem: boolean) {
853+
_handleTypeAhead(value: string, filterValue: string) {
854854
const item = this._getFirstMatchingItem(value);
855855

856856
if (!item) {
857857
return;
858858
}
859859

860860
this._applyAtomicValueAndSelection(item, filterValue);
861-
862-
if (value !== "" && !item.selected && (!checkForGroupItem || !item.isGroupItem)) {
863-
this.fireDecoratorEvent("selection-change", {
864-
item: item as ComboBoxItem,
865-
});
866-
}
867861
}
868862

869863
_handleArrowDown(e: KeyboardEvent, indexOfItem: number) {
@@ -1157,6 +1151,21 @@ class ComboBox extends UI5Element implements IFormInputElement {
11571151
const currentlyFocusedItem = this.items.find(item => item.focused);
11581152
const shouldSelectionBeCleared = currentlyFocusedItem && currentlyFocusedItem.isGroupItem;
11591153
let itemToBeSelected: IComboBoxItem | undefined;
1154+
let previouslySelectedItem: IComboBoxItem | undefined;
1155+
1156+
// Find previously selected item
1157+
this._filteredItems.forEach(item => {
1158+
if (!isInstanceOfComboBoxItemGroup(item)) {
1159+
if (item.selected) {
1160+
previouslySelectedItem = item;
1161+
}
1162+
} else {
1163+
const selectedGroupItem = item.items?.find(i => i.selected);
1164+
if (selectedGroupItem) {
1165+
previouslySelectedItem = selectedGroupItem;
1166+
}
1167+
}
1168+
});
11601169

11611170
this._filteredItems.forEach(item => {
11621171
if (!shouldSelectionBeCleared && !itemToBeSelected) {
@@ -1176,6 +1185,21 @@ class ComboBox extends UI5Element implements IFormInputElement {
11761185

11771186
return item;
11781187
});
1188+
1189+
// Fire selection-change event only when selection actually changes
1190+
if (previouslySelectedItem !== itemToBeSelected) {
1191+
if (itemToBeSelected) {
1192+
// New item selected
1193+
this.fireDecoratorEvent("selection-change", {
1194+
item: itemToBeSelected as ComboBoxItem,
1195+
});
1196+
} else if (previouslySelectedItem) {
1197+
// Selection cleared - fire event with 'null'
1198+
this.fireDecoratorEvent("selection-change", {
1199+
item: null,
1200+
});
1201+
}
1202+
}
11791203
}
11801204

11811205
_fireChangeEvent() {

0 commit comments

Comments
 (0)