Skip to content

Commit 79ab941

Browse files
nikoletavnvnikoletavnv
committed
fix(ui5-multi-input): fix arrow left key behavior
Fix comments from the code review
1 parent f5944d6 commit 79ab941

2 files changed

Lines changed: 80 additions & 130 deletions

File tree

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

Lines changed: 78 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,17 @@ const handleTokenDelete = (event) => {
3131

3232
describe("MultiInput Web Component", () => {
3333
it("creates only one token when typing 'ad' and pressing Enter", () => {
34+
const fnOnKeyDown = (event: KeyboardEvent) => {
35+
const inputElement =event.target as HTMLInputElement;
36+
if (event.key === "Enter" && inputElement.value) {
37+
const token = createTokenFromText(inputElement.value);
38+
inputElement.appendChild(token);
39+
inputElement.value = "";
40+
}
41+
};
42+
3443
cy.mount(
35-
<MultiInput showSuggestions={true} showValueHelpIcon={true} id="suggestion-token">
44+
<MultiInput showSuggestions={true} showValueHelpIcon={true} id="suggestion-token" onKeyDown={fnOnKeyDown}>
3645
<SuggestionItem text="Aute"></SuggestionItem>
3746
<SuggestionItem text="ad"></SuggestionItem>
3847
<SuggestionItem text="exercitation"></SuggestionItem>
@@ -44,17 +53,6 @@ describe("MultiInput Web Component", () => {
4453
</MultiInput>
4554
);
4655

47-
cy.get("#suggestion-token").then(multiInput => {
48-
multiInput[0].addEventListener("keydown", (event: KeyboardEvent) => {
49-
const inputElement = multiInput[0] as HTMLInputElement;
50-
if (event.key === "Enter" && inputElement.value) {
51-
const token = createTokenFromText(inputElement.value);
52-
inputElement.appendChild(token);
53-
inputElement.value = "";
54-
}
55-
});
56-
});
57-
5856
cy.get("#suggestion-token")
5957
.shadow()
6058
.find("input")
@@ -212,17 +210,12 @@ describe("MultiInput Web Component", () => {
212210

213211
it("fires value-help-trigger with F4 and Alt/Option + ArrowUp/Down", () => {
214212
cy.mount(
215-
<MultiInput id="multi-with-value-help-icon" showValueHelpIcon={true}></MultiInput>
213+
<MultiInput id="multi-with-value-help-icon" showValueHelpIcon={true} onValueHelpTrigger={cy.stub().as("valueHelpTrigger")}></MultiInput>
216214
);
217215

218216
cy.get("[ui5-multi-input]")
219217
.as("multiInput");
220218

221-
cy.get("@multiInput")
222-
.then($multiInput => {
223-
$multiInput[0].addEventListener("value-help-trigger", cy.stub().as("valueHelpTrigger"));
224-
});
225-
226219
cy.get("@multiInput")
227220
.shadow()
228221
.find(".ui5-input-inner")
@@ -396,20 +389,15 @@ describe("MultiInput tokens", () => {
396389
});
397390

398391
it("Should fire a change event", () => {
392+
const changeSpy = cy.stub().as("changeSpy");
399393
cy.mount(
400-
<MultiInput showSuggestions={true} showValueHelpIcon={true}>
394+
<MultiInput showSuggestions={true} showValueHelpIcon={true} onChange={changeSpy}>
401395
<SuggestionItem text="Aute" />
402396
<SuggestionItem text="ad" />
403397
<SuggestionItem text="exercitation" />
404398
</MultiInput>
405399
);
406400

407-
const changeSpy = cy.stub().as("changeSpy");
408-
409-
cy.get("[ui5-multi-input]").then(multiInput => {
410-
multiInput[0].addEventListener("ui5-change", changeSpy);
411-
});
412-
413401
cy.get("[ui5-multi-input]")
414402
.shadow()
415403
.find("input")
@@ -538,24 +526,22 @@ describe("MultiInput tokens", () => {
538526
});
539527

540528
it("should empty the field when value is cleared in the change handler", () => {
529+
const fnOnChange = (evt: any) => {
530+
(evt.target as HTMLElement).appendChild(createTokenFromText((evt.target as HTMLInputElement).value));
531+
(evt.target as HTMLInputElement).value = "";
532+
};
533+
534+
const fnValueHelpTrigger = (evt: any) => {
535+
(evt.target as ResponsivePopover).open = true;
536+
};
537+
541538
cy.mount(
542-
<MultiInput showSuggestions id="token-unique" showValueHelpIcon>
539+
<MultiInput showSuggestions id="token-unique" showValueHelpIcon onChange={fnOnChange} onValueHelpTrigger={fnValueHelpTrigger}>
543540
<div slot="valueStateMessage" id="value-state-wrapper">Token is already in the list</div>
544541
<SuggestionItem text="Argentina"></SuggestionItem>
545542
</MultiInput>
546543
);
547544

548-
cy.get("[ui5-multi-input]")
549-
.then(multiInput => {
550-
multiInput[0].addEventListener("ui5-value-help-trigger", function (event) {
551-
(event.target as ResponsivePopover).open = true;
552-
});
553-
multiInput[0].addEventListener("ui5-change", (event) => {
554-
(event.target as HTMLElement).appendChild(createTokenFromText((event.target as HTMLInputElement).value));
555-
(event.target as HTMLInputElement).value = "";
556-
});
557-
});
558-
559545
cy.get("[ui5-multi-input]")
560546
.shadow()
561547
.find("[ui5-icon]")
@@ -795,18 +781,13 @@ describe("MultiInput Truncated Token", () => {
795781
});
796782

797783
it("should truncate token when a long token is added", () => {
798-
799784
cy.mount(
800785
<>
801-
<MultiInput id="truncated-token"></MultiInput>
786+
<MultiInput id="truncated-token" onTokenDelete={handleTokenDelete}></MultiInput>
802787
<Button>button</Button>
803788
</>
804789
);
805790

806-
cy.get("[ui5-multi-input]").then(multiInput => {
807-
multiInput[0].addEventListener("ui5-token-delete", handleTokenDelete);
808-
});
809-
810791
cy.get("[ui5-button]").then(button => {
811792
button[0].addEventListener("click", () => {
812793
const longText = "Officia enim ullamco sunt sunt nisi ullamco cillum velit ullamco cillum velit ullamco cillum";
@@ -986,26 +967,23 @@ describe("ARIA attributes", () => {
986967
});
987968

988969
it("announces correct suggestion position when selecting a suggestion with Enter", () => {
970+
const fnKeyDown = (event: KeyboardEvent) => {
971+
const inputElement = event.target as HTMLInputElement;
972+
if (event.key === "Enter" && inputElement.value) {
973+
const token = createTokenFromText(inputElement.value);
974+
inputElement.appendChild(token);
975+
inputElement.value = "";
976+
}
977+
};
978+
989979
cy.mount(
990-
<MultiInput show-suggestions id="suggestion-token">
980+
<MultiInput show-suggestions id="suggestion-token" onKeyDown={fnKeyDown}>
991981
<SuggestionItem text="Aute"></SuggestionItem>
992982
<SuggestionItem text="ad"></SuggestionItem>
993983
<SuggestionItem text="exercitation"></SuggestionItem>
994984
</MultiInput>
995985
);
996986

997-
cy.get("[ui5-multi-input]")
998-
.then(multiInput => {
999-
multiInput[0].addEventListener("keydown", (event: KeyboardEvent) => {
1000-
const inputElement = multiInput[0] as HTMLInputElement;
1001-
if (event.key === "Enter" && inputElement.value) {
1002-
const token = createTokenFromText(inputElement.value);
1003-
inputElement.appendChild(token);
1004-
inputElement.value = "";
1005-
}
1006-
});
1007-
})
1008-
1009987
cy.get("[ui5-multi-input]")
1010988
.realClick();
1011989

@@ -1350,8 +1328,17 @@ describe("Keyboard handling", () => {
13501328
});
13511329

13521330
it("should change input's value when set in selection change event", () => {
1331+
const fnOnKeyDown = (event: KeyboardEvent) => {
1332+
const inputElement = event.target as HTMLInputElement;
1333+
if (event.key === "Enter" && inputElement.value) {
1334+
const token = createTokenFromText(inputElement.value);
1335+
inputElement.appendChild(token);
1336+
inputElement.value = "";
1337+
}
1338+
};
1339+
13531340
cy.mount(
1354-
<MultiInput showSuggestions showValueHelpIcon>
1341+
<MultiInput showSuggestions showValueHelpIcon onKeyDown={fnOnKeyDown}>
13551342
<SuggestionItem text="Aute"></SuggestionItem>
13561343
<SuggestionItem text="ad"></SuggestionItem>
13571344
<SuggestionItem text="exercitation"></SuggestionItem>
@@ -1363,18 +1350,6 @@ describe("Keyboard handling", () => {
13631350
</MultiInput>
13641351
);
13651352

1366-
cy.get("[ui5-multi-input]")
1367-
.then(multiInput => {
1368-
multiInput[0].addEventListener("keydown", (event: KeyboardEvent) => {
1369-
const inputElement = multiInput[0] as HTMLInputElement;
1370-
if (event.key === "Enter" && inputElement.value) {
1371-
const token = createTokenFromText(inputElement.value);
1372-
inputElement.appendChild(token);
1373-
inputElement.value = "";
1374-
}
1375-
});
1376-
})
1377-
13781353
cy.get("[ui5-multi-input]")
13791354
.shadow()
13801355
.find("input")
@@ -1424,42 +1399,38 @@ describe("Keyboard handling", () => {
14241399
});
14251400

14261401
it("should trigger change event on enter", () => {
1402+
const fnOnChange = (event: any) => {
1403+
const target = event.target as HTMLInputElement;
1404+
if (!target.value) {
1405+
return;
1406+
}
1407+
1408+
var isDuplicate = (event.target as MultiInput).tokens.some(function(token) {
1409+
return token.text === (event.target as HTMLInputElement).value
1410+
});
1411+
1412+
if (isDuplicate) {
1413+
(event.target as Input).valueState = "Negative";
1414+
1415+
setTimeout(function () {
1416+
(event.target as Input).valueState = "None";
1417+
}, 200);
1418+
1419+
return;
1420+
}
1421+
1422+
(event.target as HTMLElement).appendChild(createTokenFromText((event.target as HTMLInputElement).value));
1423+
(event.target as HTMLInputElement).value = "";
1424+
};
1425+
14271426
cy.mount(
1428-
<MultiInput showSuggestions id="token-unique" showValueHelpIcon>
1427+
<MultiInput showSuggestions id="token-unique" showValueHelpIcon onChange={fnOnChange}>
14291428
<div slot="valueStateMessage" id="value-state-wrapper">Token is already in the list</div>
14301429
<SuggestionItem text="Argentina" />
14311430
</MultiInput>
14321431
);
14331432

14341433
cy.get("[ui5-multi-input]")
1435-
.then(multiInput => {
1436-
1437-
multiInput[0].addEventListener("ui5-change", function (event) {
1438-
const target = event.target as HTMLInputElement;
1439-
if (!target.value) {
1440-
return;
1441-
}
1442-
1443-
var isDuplicate = (event.target as MultiInput).tokens.some(function(token) {
1444-
return token.text === (event.target as HTMLInputElement).value
1445-
});
1446-
1447-
if (isDuplicate) {
1448-
(event.target as Input).valueState = "Negative";
1449-
1450-
setTimeout(function () {
1451-
(event.target as Input).valueState = "None";
1452-
}, 200);
1453-
1454-
return;
1455-
}
1456-
1457-
(event.target as HTMLElement).appendChild(createTokenFromText((event.target as HTMLInputElement).value));
1458-
(event.target as HTMLInputElement).value = "";
1459-
});
1460-
});
1461-
1462-
cy.get("[ui5-multi-input]")
14631434
.shadow()
14641435
.find("input")
14651436
.as("innerInput");
@@ -1504,21 +1475,16 @@ describe("Keyboard handling", () => {
15041475
});
15051476

15061477
it("should focus last token on ArrowLeft at start of input, keep suggestions open, and not fire change event", () => {
1478+
const changeSpy = cy.stub().as("changeSpy");
1479+
15071480
cy.mount(
1508-
<MultiInput showSuggestions>
1481+
<MultiInput showSuggestions onChange={changeSpy}>
15091482
<Token slot="tokens" text="Amet"></Token>
15101483
<SuggestionItem text="Bulgaria"></SuggestionItem>
15111484
<SuggestionItem text="Brazil"></SuggestionItem>
15121485
</MultiInput>
15131486
);
15141487

1515-
const changeSpy = cy.stub().as("changeSpy");
1516-
1517-
cy.get("[ui5-multi-input]")
1518-
.then(multiInput => {
1519-
multiInput[0].addEventListener("ui5-change", changeSpy);
1520-
});
1521-
15221488
cy.get("[ui5-multi-input]")
15231489
.shadow()
15241490
.find("input")
@@ -1553,21 +1519,16 @@ describe("Keyboard handling", () => {
15531519
});
15541520

15551521
it("should fire change event when returning from tokenizer to input via ArrowRight and pressing Tab", () => {
1522+
const changeSpy = cy.stub().as("changeSpy");
1523+
15561524
cy.mount(
1557-
<MultiInput showSuggestions>
1525+
<MultiInput showSuggestions onChange={changeSpy}>
15581526
<Token slot="tokens" text="Amet"></Token>
15591527
<SuggestionItem text="Bulgaria"></SuggestionItem>
15601528
<SuggestionItem text="Brazil"></SuggestionItem>
15611529
</MultiInput>
15621530
);
15631531

1564-
const changeSpy = cy.stub().as("changeSpy");
1565-
1566-
cy.get("[ui5-multi-input]")
1567-
.then(multiInput => {
1568-
multiInput[0].addEventListener("ui5-change", changeSpy);
1569-
});
1570-
15711532
cy.get("[ui5-multi-input]")
15721533
.shadow()
15731534
.find("input")
@@ -1598,21 +1559,16 @@ describe("Keyboard handling", () => {
15981559
});
15991560

16001561
it("should fire change event when returning from tokenizer to input via Tab and pressing Enter", () => {
1562+
const changeSpy = cy.stub().as("changeSpy");
1563+
16011564
cy.mount(
1602-
<MultiInput showSuggestions noTypeahead>
1565+
<MultiInput showSuggestions noTypeahead onChange={changeSpy}>
16031566
<Token slot="tokens" text="Amet"></Token>
16041567
<SuggestionItem text="Bulgaria"></SuggestionItem>
16051568
<SuggestionItem text="Brazil"></SuggestionItem>
16061569
</MultiInput>
16071570
);
16081571

1609-
const changeSpy = cy.stub().as("changeSpy");
1610-
1611-
cy.get("[ui5-multi-input]")
1612-
.then(multiInput => {
1613-
multiInput[0].addEventListener("ui5-change", changeSpy);
1614-
});
1615-
16161572
cy.get("[ui5-multi-input]")
16171573
.shadow()
16181574
.find("input")

‎packages/main/src/MultiInput.ts‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,6 @@ class MultiInput extends Input implements IFormInputElement {
155155
_skipOpenSuggestions: boolean;
156156
_valueHelpIconPressed: boolean;
157157
_focusInTokenizer: boolean;
158-
_returningFromTokenizer: boolean;
159158

160159
get formValidityMessage() {
161160
return MultiInput.i18nBundle.getText(FORM_MIXED_TEXTFIELD_REQUIRED);
@@ -192,7 +191,6 @@ class MultiInput extends Input implements IFormInputElement {
192191
this._skipOpenSuggestions = false;
193192
this._valueHelpIconPressed = false;
194193
this._focusInTokenizer = false;
195-
this._returningFromTokenizer = false;
196194
}
197195

198196
valueHelpPress() {
@@ -230,9 +228,6 @@ class MultiInput extends Input implements IFormInputElement {
230228
if (!this.contains(e.relatedTarget as HTMLElement) && !this.shadowRoot!.contains(e.relatedTarget as HTMLElement)) {
231229
this.tokenizer._tokens.forEach(token => { token.selected = false; });
232230
}
233-
if (this.shadowRoot!.contains(e.relatedTarget as HTMLElement)) {
234-
this._returningFromTokenizer = true;
235-
}
236231
this._focusInTokenizer = false;
237232
}
238233

@@ -298,7 +293,6 @@ class MultiInput extends Input implements IFormInputElement {
298293
const lastTokenIndex = this.tokens.length - 1;
299294

300295
if (e.target === this.tokens[lastTokenIndex] && this.tokens[lastTokenIndex] === document.activeElement) {
301-
this._returningFromTokenizer = true;
302296
setTimeout(() => {
303297
this.focus();
304298
}, 0);
@@ -376,10 +370,10 @@ class MultiInput extends Input implements IFormInputElement {
376370
*/
377371
_onfocusin(e: FocusEvent) {
378372
const inputDomRef = this.getInputDOMRef();
373+
const wasTokenFocused = e.relatedTarget instanceof HTMLElement && e.relatedTarget.hasAttribute("ui5-token");
379374

380375
if (e.target === inputDomRef) {
381-
if (this._returningFromTokenizer) {
382-
this._returningFromTokenizer = false;
376+
if (wasTokenFocused) {
383377
this.focused = true;
384378
this.open = true;
385379
this._inputIconFocused = false;

0 commit comments

Comments
 (0)