Skip to content

Commit

Permalink
chore: remove redundant statement for autocomplete in Input (#571)
Browse files Browse the repository at this point in the history
# 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`.
  • Loading branch information
AntonioVentilii authored Jan 30, 2025
1 parent 0c06dcd commit a896ded
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 5 deletions.
4 changes: 0 additions & 4 deletions src/lib/components/Input.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,6 @@
({ selectionStart, selectionEnd } = inputElement);
};
$: step = inputType === "number" ? (step ?? "any") : undefined;
$: autocomplete =
inputType !== "number" && !currency ? (autocomplete ?? "off") : undefined;
let displayInnerEnd: boolean;
$: displayInnerEnd = nonNullish($$slots["inner-end"]);
</script>
Expand Down
60 changes: 59 additions & 1 deletion src/tests/lib/components/Input.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import InputTest from "./InputTest.svelte";
import InputValueTest from "./InputValueTest.svelte";

describe("Input", () => {
type InputType = "icp" | "number" | "text" | "currency";
type AutoComplete = "on" | "off" | undefined;

const props = { name: "name", placeholder: "test.placeholder" };

it("should render an input", () => {
Expand Down Expand Up @@ -34,7 +37,7 @@ describe("Input", () => {
container,
}: {
attribute: string;
expected: string;
expected: string | null | undefined;
container: HTMLElement;
}) => {
const input: HTMLInputElement | null = container.querySelector("input");
Expand Down Expand Up @@ -553,4 +556,59 @@ describe("Input", () => {

expect(input === document.activeElement).toBe(true);
});

describe.each(["number"])("inputType=%s", (inputType) => {
it("should never set autocomplete", () => {
const { container: container1 } = render(Input, {
props: {
...props,
inputType: inputType as InputType,
autocomplete: "off",
},
});

testGetAttribute({
container: container1,
attribute: "autocomplete",
expected: null,
});

const { container: container2 } = render(Input, {
props: {
...props,
inputType: inputType as InputType,
autocomplete: "on",
},
});

testGetAttribute({
container: container2,
attribute: "autocomplete",
expected: null,
});
});
});

describe.each(["icp", "text", "currency"])("inputType='%s'", (inputType) => {
describe.each([["on"], ["off"], [undefined, "off"]])(
"autocomplete='%s'",
(autocomplete, expected = undefined) => {
it(`should set autocomplete to '${expected}' for inputType='${inputType}' and autocomplete='${autocomplete}'`, () => {
const { container } = render(Input, {
props: {
...props,
inputType: inputType as InputType,
autocomplete: autocomplete as AutoComplete,
},
});

testGetAttribute({
container,
attribute: "autocomplete",
expected: autocomplete ?? expected,
});
});
},
);
});
});

0 comments on commit a896ded

Please sign in to comment.