Skip to content

Commit a896ded

Browse files
chore: remove redundant statement for autocomplete in Input (#571)
# Motivation There are two reactive statements that define `step` and `autocomplete`. For step, they are identical, so we can remove them. For `autocomplete`, one of the two ignores the prop `autocomplete` for currency input type. I think that the objective is to have this ONLY for type `number`.
1 parent 0c06dcd commit a896ded

File tree

2 files changed

+59
-5
lines changed

2 files changed

+59
-5
lines changed

src/lib/components/Input.svelte

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,6 @@
156156
({ selectionStart, selectionEnd } = inputElement);
157157
};
158158
159-
$: step = inputType === "number" ? (step ?? "any") : undefined;
160-
$: autocomplete =
161-
inputType !== "number" && !currency ? (autocomplete ?? "off") : undefined;
162-
163159
let displayInnerEnd: boolean;
164160
$: displayInnerEnd = nonNullish($$slots["inner-end"]);
165161
</script>

src/tests/lib/components/Input.spec.ts

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ import InputTest from "./InputTest.svelte";
77
import InputValueTest from "./InputValueTest.svelte";
88

99
describe("Input", () => {
10+
type InputType = "icp" | "number" | "text" | "currency";
11+
type AutoComplete = "on" | "off" | undefined;
12+
1013
const props = { name: "name", placeholder: "test.placeholder" };
1114

1215
it("should render an input", () => {
@@ -34,7 +37,7 @@ describe("Input", () => {
3437
container,
3538
}: {
3639
attribute: string;
37-
expected: string;
40+
expected: string | null | undefined;
3841
container: HTMLElement;
3942
}) => {
4043
const input: HTMLInputElement | null = container.querySelector("input");
@@ -553,4 +556,59 @@ describe("Input", () => {
553556

554557
expect(input === document.activeElement).toBe(true);
555558
});
559+
560+
describe.each(["number"])("inputType=%s", (inputType) => {
561+
it("should never set autocomplete", () => {
562+
const { container: container1 } = render(Input, {
563+
props: {
564+
...props,
565+
inputType: inputType as InputType,
566+
autocomplete: "off",
567+
},
568+
});
569+
570+
testGetAttribute({
571+
container: container1,
572+
attribute: "autocomplete",
573+
expected: null,
574+
});
575+
576+
const { container: container2 } = render(Input, {
577+
props: {
578+
...props,
579+
inputType: inputType as InputType,
580+
autocomplete: "on",
581+
},
582+
});
583+
584+
testGetAttribute({
585+
container: container2,
586+
attribute: "autocomplete",
587+
expected: null,
588+
});
589+
});
590+
});
591+
592+
describe.each(["icp", "text", "currency"])("inputType='%s'", (inputType) => {
593+
describe.each([["on"], ["off"], [undefined, "off"]])(
594+
"autocomplete='%s'",
595+
(autocomplete, expected = undefined) => {
596+
it(`should set autocomplete to '${expected}' for inputType='${inputType}' and autocomplete='${autocomplete}'`, () => {
597+
const { container } = render(Input, {
598+
props: {
599+
...props,
600+
inputType: inputType as InputType,
601+
autocomplete: autocomplete as AutoComplete,
602+
},
603+
});
604+
605+
testGetAttribute({
606+
container,
607+
attribute: "autocomplete",
608+
expected: autocomplete ?? expected,
609+
});
610+
});
611+
},
612+
);
613+
});
556614
});

0 commit comments

Comments
 (0)