Skip to content

Commit c1abfb0

Browse files
VonahzVonahz
authored
fix(ui5-multi-combobox): fix token deletion when click event is lost due to re-render (#13530)
1 parent d256801 commit c1abfb0

5 files changed

Lines changed: 156 additions & 12 deletions

File tree

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,68 @@ describe("General", () => {
369369
.should("not.exist");
370370
});
371371

372+
it("Should delete token after focus change when tokenizer collapses", () => {
373+
cy.mount(
374+
<MultiComboBox style="width: 250px;">
375+
<MultiComboBoxItem selected={true} text="Albania"></MultiComboBoxItem>
376+
<MultiComboBoxItem selected={true} text="Argentina"></MultiComboBoxItem>
377+
<MultiComboBoxItem selected={true} text="Bulgaria"></MultiComboBoxItem>
378+
<MultiComboBoxItem selected={true} text="England"></MultiComboBoxItem>
379+
</MultiComboBox>
380+
);
381+
382+
cy.get("[ui5-multi-combobox]")
383+
.as("mcb")
384+
.shadow()
385+
.find("[ui5-tokenizer]")
386+
.as("tokenizer")
387+
.invoke('on', 'ui5-token-delete', cy.spy().as('tokenDelete'));
388+
389+
// Verify initial state: 4 tokens
390+
cy.get("@tokenizer")
391+
.find("[ui5-token]")
392+
.should("have.length", 4);
393+
394+
// Click on Albania token to select it (make it focused and selected)
395+
cy.get("@tokenizer")
396+
.find("[ui5-token]")
397+
.first()
398+
.as("albaniaToken")
399+
.realClick();
400+
401+
// Verify Albania token is focused
402+
cy.get("@albaniaToken")
403+
.should("have.attr", "focused");
404+
405+
// Press Arrow Right to move focus to Argentina
406+
cy.realPress("ArrowRight");
407+
408+
// Wait a moment for tokenizer state to settle
409+
cy.wait(100);
410+
411+
// Click delete icon on Albania token
412+
cy.get("@albaniaToken")
413+
.shadow()
414+
.find("[ui5-icon]")
415+
.realClick();
416+
417+
// Verify token-delete event was fired
418+
cy.get("@tokenDelete")
419+
.should("have.been.calledOnce");
420+
421+
// Verify Albania token was removed (3 tokens remaining)
422+
cy.get("@tokenizer")
423+
.find("[ui5-token]")
424+
.should("have.length", 3);
425+
426+
// Verify Albania is no longer the first token
427+
cy.get("@tokenizer")
428+
.find("[ui5-token]")
429+
.first()
430+
.invoke("attr", "text")
431+
.should("not.equal", "Albania");
432+
});
433+
372434
it("Autocomplete (typeahead)", () => {
373435
cy.mount(
374436
<MultiComboBox>

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1794,4 +1794,62 @@ describe("MultiInput Composition", () => {
17941794

17951795
cy.get("@multiinput").should("have.attr", "value", "谢谢");
17961796
});
1797+
1798+
it("should handle token deletion by clicking icon when token is not focused", () => {
1799+
cy.mount(
1800+
<MultiInput style="width: 250px;">
1801+
<Token slot="tokens" text="Albania"></Token>
1802+
<Token slot="tokens" text="Argentina"></Token>
1803+
<Token slot="tokens" text="Bulgaria"></Token>
1804+
<Token slot="tokens" text="England"></Token>
1805+
</MultiInput>
1806+
);
1807+
1808+
cy.get("[ui5-multi-input]")
1809+
.as("multiinput")
1810+
.then(multiInput => {
1811+
multiInput[0].addEventListener("ui5-token-delete", handleTokenDelete);
1812+
});
1813+
1814+
// Verify initial state: 4 tokens
1815+
cy.get("[ui5-token]")
1816+
.should("have.length", 4);
1817+
1818+
// Click the input field to focus it
1819+
cy.get("@multiinput")
1820+
.shadow()
1821+
.find("input")
1822+
.as("input")
1823+
.realClick();
1824+
1825+
// Navigate to Albania token (last token) with ArrowLeft
1826+
cy.realPress("ArrowLeft");
1827+
1828+
// Store reference to Albania token (now focused)
1829+
cy.get("[ui5-token]")
1830+
.eq(3)
1831+
.as("albaniaToken")
1832+
.should("have.attr", "focused");
1833+
1834+
// Move focus away from Albania to Argentina with ArrowRight
1835+
cy.realPress("ArrowRight");
1836+
1837+
// Verify Albania is no longer focused
1838+
cy.get("@albaniaToken")
1839+
.should("not.have.attr", "focused");
1840+
1841+
// Click delete icon on Albania token (which is not focused)
1842+
cy.get("@albaniaToken")
1843+
.shadow()
1844+
.find("[ui5-icon]")
1845+
.realClick();
1846+
1847+
// Verify Albania token was deleted
1848+
cy.get("[ui5-token]")
1849+
.should("have.length", 3);
1850+
1851+
// Verify Albania is no longer present
1852+
cy.get("[ui5-token]")
1853+
.should("not.contain.text", "Albania");
1854+
});
17971855
});

‎packages/main/src/MultiInput.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,9 @@ class MultiInput extends Input implements IFormInputElement {
356356
const insideDOM = this.contains(relatedTarget);
357357
const insideShadowDom = this.shadowRoot!.contains(relatedTarget);
358358

359-
if (!insideDOM && !insideShadowDom) {
359+
const hasTokenToBeDeleted = this.tokenizer._tokens.some(token => token.toBeDeleted);
360+
361+
if (!insideDOM && !insideShadowDom && !hasTokenToBeDeleted) {
360362
this.tokenizer.expanded = false;
361363
}
362364

‎packages/main/src/Token.ts‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ class Token extends UI5Element implements IToken {
124124
* @default false
125125
* @private
126126
*/
127-
@property({ type: Boolean })
127+
@property({ type: Boolean, noAttribute: true })
128128
toBeDeleted = false;
129129

130130
/**
@@ -170,16 +170,20 @@ class Token extends UI5Element implements IToken {
170170
}
171171

172172
_delete() {
173+
// If toBeDeleted is already true, the delete event was already fired in mousedown
174+
if (this.toBeDeleted) {
175+
return;
176+
}
177+
173178
this.toBeDeleted = true;
174179
this.fireDecoratorEvent("delete");
175180
}
176181

177182
_onmousedown(e: MouseEvent) {
178-
const target = e.currentTarget as HTMLElement;
179-
180-
if (target === this.shadowRoot?.querySelector("[ui5-icon]")) {
181-
this.toBeDeleted = true;
182-
}
183+
e.preventDefault(); // Prevent focus changes during deletion
184+
this.toBeDeleted = true;
185+
// Fire the delete event immediately to avoid losing it due to DOM changes
186+
this.fireDecoratorEvent("delete");
183187
}
184188

185189
_keydown(e: KeyboardEvent) {

‎packages/main/src/Tokenizer.ts‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,14 @@ class Tokenizer extends UI5Element implements IFormInputElement {
524524
}
525525

526526
this._scrollToEndIfNeeded();
527-
this._tokenDeleting = false;
527+
528+
// Only reset _tokenDeleting if no token is currently marked for deletion
529+
// This prevents resetting the flag before the actual deletion logic executes
530+
const hasTokenToBeDeleted = this._tokens.some(token => token.toBeDeleted);
531+
532+
if (!hasTokenToBeDeleted) {
533+
this._tokenDeleting = false;
534+
}
528535

529536
// Update lastVisibleToken after rendering is complete to avoid render loops
530537
renderFinished().then(() => {
@@ -578,7 +585,7 @@ class Tokenizer extends UI5Element implements IFormInputElement {
578585
}
579586

580587
_tokenClickDelete(e: CustomEvent<TokenDeleteEventDetail>, token: Token) {
581-
const tokens = this._getVisibleTokens();
588+
const tokens = this._tokens;
582589
const target = e.target as Token;
583590
const deletedTokenIndex = token ? tokens.indexOf(token) : tokens.indexOf(target); // The index of the token that just got deleted
584591
const nextTokenIndex = deletedTokenIndex === tokens.length - 1 ? deletedTokenIndex - 1 : deletedTokenIndex + 1; // The index of the next token that needs to be focused next due to the deletion
@@ -607,7 +614,7 @@ class Tokenizer extends UI5Element implements IFormInputElement {
607614
* @param forwardFocusToPrevious Indicates whether the focus will be forwarded to previous or next token after deletion.
608615
*/
609616
deleteToken(token: Token, forwardFocusToPrevious?: boolean) {
610-
const tokens = this._getVisibleTokens();
617+
const tokens = this._tokens;
611618
const deletedTokenIndex = tokens.indexOf(token);
612619
let nextTokenIndex = (deletedTokenIndex === tokens.length - 1) ? deletedTokenIndex - 1 : deletedTokenIndex + 1;
613620
const notSelectedTokens = tokens.filter(t => !t.selected);
@@ -698,7 +705,10 @@ class Tokenizer extends UI5Element implements IFormInputElement {
698705

699706
handleAfterClose() {
700707
this.open = false;
701-
this._preventCollapse = false;
708+
// Don't reset _preventCollapse if we're in the middle of deleting a token
709+
if (!this._tokenDeleting) {
710+
this._preventCollapse = false;
711+
}
702712
this._focusedElementBeforeOpen = null;
703713
}
704714

@@ -1007,6 +1017,12 @@ class Tokenizer extends UI5Element implements IFormInputElement {
10071017

10081018
_onfocusout(e: FocusEvent) {
10091019
const relatedTarget = e.relatedTarget as HTMLElement;
1020+
const tokenLosingFocus = e.target as Token;
1021+
1022+
// If the token losing focus is being deleted, prevent collapse
1023+
if (tokenLosingFocus?.toBeDeleted) {
1024+
this._preventCollapse = true;
1025+
}
10101026

10111027
this._tokens.forEach(token => {
10121028
token.forcedTabIndex = "-1";
@@ -1021,7 +1037,9 @@ class Tokenizer extends UI5Element implements IFormInputElement {
10211037
this._skipTabIndex = false;
10221038
}
10231039

1024-
if (!this._tokenDeleting && !this._preventCollapse) {
1040+
const hasTokenToBeDeleted = this._tokens.some(token => token.toBeDeleted);
1041+
1042+
if (!this._tokenDeleting && !this._preventCollapse && !hasTokenToBeDeleted) {
10251043
this._preventCollapse = false;
10261044
this.expanded = false;
10271045
}

0 commit comments

Comments
 (0)