Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
59b5a85
fix(input): no longer remove trailing decimal separator
anveshmekala Jun 8, 2023
4613212
add localized leading decimal zero values
anveshmekala Jun 16, 2023
f241c17
Merge branch 'master' into anveshmekala/7039-fix-calcite-input-decima…
anveshmekala Jun 16, 2023
387db5b
Merge branch 'master' into anveshmekala/7039-fix-calcite-input-decima…
anveshmekala Jun 16, 2023
54b7143
Merge branch 'master' into anveshmekala/7039-fix-calcite-input-decima…
anveshmekala Jun 20, 2023
5d6f507
fix tests
anveshmekala Jun 20, 2023
c6a1e71
fix tests
anveshmekala Jun 21, 2023
cffe278
Merge branch 'master' into anveshmekala/7039-fix-calcite-input-decima…
anveshmekala Jun 21, 2023
1c60d07
cleanup
anveshmekala Jun 21, 2023
eacde2d
Merge branch 'master' into anveshmekala/7039-fix-calcite-input-decima…
anveshmekala Jun 21, 2023
c447239
refactor
anveshmekala Jun 21, 2023
7a0480a
refactor util method
anveshmekala Jun 21, 2023
87f15d7
remove redundant code blocks
anveshmekala Jun 21, 2023
63dff04
refactor util method to handle comma decimal separator
anveshmekala Jun 23, 2023
d06003b
Merge branch 'master' into anveshmekala/7039-fix-calcite-input-decima…
anveshmekala Jun 26, 2023
6751aaa
refactor util methods
anveshmekala Jun 26, 2023
688b98a
update tests
anveshmekala Jun 26, 2023
f7b9f94
remove commented code
anveshmekala Jun 26, 2023
43b3075
add missing type for tests
anveshmekala Jun 26, 2023
cd4a3ac
fix failing spec tests
anveshmekala Jun 27, 2023
98c86d4
use regex
anveshmekala Jun 27, 2023
ce04bab
add more spec tests
anveshmekala Jun 27, 2023
ecbc697
remove unused imports
anveshmekala Jun 27, 2023
fbc3831
Merge branch 'master' into anveshmekala/7039-fix-calcite-input-decima…
anveshmekala Jun 27, 2023
de31f7c
simplify util method logic
anveshmekala Jun 28, 2023
05b771c
Merge branch 'master' into anveshmekala/7039-fix-calcite-input-decima…
anveshmekala Jun 28, 2023
c507400
feedback changes and more e2e tests added
anveshmekala Jun 29, 2023
4a57130
remove unused methods in number spec test
anveshmekala Jun 29, 2023
dfc9311
add new lines in test
anveshmekala Jun 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,35 @@ describe("calcite-input-number", () => {
expect(Number(await element.getProperty("value"))).toBe(195);
});

it("allows deleting exponentail number from decimal and adding trailing zeros", async () => {
const page = await newE2EPage();
await page.setContent(html`<calcite-input-number></calcite-input-number>`);

const calciteInput = await page.find("calcite-input-number");
const input = await page.find("calcite-input-number >>> input");
await calciteInput.callMethod("setFocus");
await page.waitForChanges();
await typeNumberValue(page, "2.100e10");
await page.waitForChanges();
expect(await calciteInput.getProperty("value")).toBe("2.1e10");
expect(await input.getProperty("value")).toBe("2.1e10");

await page.keyboard.press("Backspace");
await page.waitForChanges();
expect(await calciteInput.getProperty("value")).toBe("2.1e1");
expect(await input.getProperty("value")).toBe("2.1e1");

await page.keyboard.press("Backspace");
await page.waitForChanges();
expect(await calciteInput.getProperty("value")).toBe("2.1");
expect(await input.getProperty("value")).toBe("2.1");

await page.keyboard.type("000");
await page.waitForChanges();
expect(await calciteInput.getProperty("value")).toBe("2.1000");
expect(await input.getProperty("value")).toBe("2.1000");
});

it("disallows typing non-numeric characters with shift modifier key down", async () => {
const page = await newE2EPage();
await page.setContent(html`<calcite-input-number></calcite-input-number>`);
Expand Down Expand Up @@ -1114,6 +1143,80 @@ describe("calcite-input-number", () => {
expect(await calciteInput.getProperty("value")).toBe(assertedValue);
expect(await internalLocaleInput.getProperty("value")).toBe(numberStringFormatter.localize(assertedValue));
});

it(`should be able to append values after Backspace for ${locale} locale`, async () => {
const page = await newE2EPage();
await page.setContent(`
<calcite-input-number lang="${locale}"></calcite-input-number>
`);

numberStringFormatter.numberFormatOptions = {
locale,
numberingSystem: "latn",
useGrouping: false
};
const decimalSeparator = numberStringFormatter.decimal;
const calciteInput = await page.find("calcite-input-number");
const input = await page.find("calcite-input-number >>> input");
await calciteInput.callMethod("setFocus");
await typeNumberValue(page, `0${decimalSeparator}0000`);
await page.waitForChanges();
expect(await input.getProperty("value")).toBe(`0${decimalSeparator}0000`);

await page.keyboard.press("Backspace");
await typeNumberValue(page, "1");
await page.waitForChanges();
expect(await input.getProperty("value")).toBe(`0${decimalSeparator}0001`);

await typeNumberValue(page, "01");
await page.waitForChanges();
expect(await input.getProperty("value")).toBe(`0${decimalSeparator}000101`);
});

it(`should keep leading decimal separator while input is focused on Backspace ${locale} locale `, async () => {
const page = await newE2EPage();
await page.setContent(`
<calcite-input-number lang="${locale}"></calcite-input-number>
`);

numberStringFormatter.numberFormatOptions = {
locale,
numberingSystem: "latn",
useGrouping: false
};
const decimalSeparator = numberStringFormatter.decimal;
const calciteInput = await page.find("calcite-input-number");
const input = await page.find("calcite-input-number >>> input");
await calciteInput.callMethod("setFocus");
await typeNumberValue(page, `0${decimalSeparator}01`);
await page.waitForChanges();
expect(await input.getProperty("value")).toBe(`0${decimalSeparator}01`);

await page.keyboard.press("Backspace");
await page.waitForChanges();
expect(await input.getProperty("value")).toBe(`0${decimalSeparator}0`);

await page.keyboard.press("Backspace");
await page.waitForChanges();
expect(await input.getProperty("value")).toBe(`0${decimalSeparator}`);

await typeNumberValue(page, "01");
await page.waitForChanges();
expect(await input.getProperty("value")).toBe(`0${decimalSeparator}01`);
});

it(`should sanitize leading decimal zeros on initial render ${locale} locale`, async () => {
const page = await newE2EPage();
await page.setContent(html`<calcite-input-number value="0.0000" lang="${locale}"></calcite-input-number>`);

numberStringFormatter.numberFormatOptions = {
locale,
numberingSystem: "latn",
useGrouping: false
};
const input = await page.find("calcite-input-number >>> input");
expect(await input.getProperty("value")).toBe("0");
});
});
});

Expand Down Expand Up @@ -1373,7 +1476,7 @@ describe("calcite-input-number", () => {

await page.keyboard.press("Backspace");
await page.waitForChanges();
expect(await element.getProperty("value")).toBe("1");
expect(await element.getProperty("value")).toBe("1.");
expect(calciteInputNumberInput).toHaveReceivedEventTimes(1);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ import {
} from "../../utils/loadable";
import {
connectLocalized,
defaultNumberingSystem,
disconnectLocalized,
LocalizedComponent,
NumberingSystem,
numberStringFormatter
} from "../../utils/locale";
import {
addLocalizedTrailingDecimalZeros,
BigDecimal,
isValidNumber,
parseNumberString,
Expand Down Expand Up @@ -840,12 +840,11 @@ export class InputNumber
useGrouping: this.groupSeparator
};

const sanitizedValue = sanitizeNumberString(
// no need to delocalize a string that ia already in latn numerals
(this.numberingSystem && this.numberingSystem !== "latn") || defaultNumberingSystem !== "latn"
? numberStringFormatter.delocalize(value)
: value
);
const isValueDeleted =
this.previousValue?.length > value.length || this.value?.length > value.length;
const hasTrailingDecimalSeparator = value.charAt(value.length - 1) === ".";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the . should be numberFormatter.decimal here, since it is being done before delocalization

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't think so, if you see that setNumberValue parses delocalized value in inputNumberInputHandler. The only instance when we try to check if there is a trailingDecimalSeparator is when the value is deleted which invokes inputNumberInputHandler method.

const sanitizedValue =
hasTrailingDecimalSeparator && isValueDeleted ? value : sanitizeNumberString(value);

const newValue =
value && !sanitizedValue
Expand All @@ -854,8 +853,21 @@ export class InputNumber
: ""
: sanitizedValue;

const newLocalizedValue = numberStringFormatter.localize(newValue);
this.localizedValue = newLocalizedValue;
let newLocalizedValue = numberStringFormatter.localize(newValue);

if (origin !== "connected" && !hasTrailingDecimalSeparator) {
newLocalizedValue = addLocalizedTrailingDecimalZeros(
newLocalizedValue,
newValue,
numberStringFormatter
);
}

// adds localized trailing decimal separator
this.localizedValue =
hasTrailingDecimalSeparator && isValueDeleted
? `${newLocalizedValue}${numberStringFormatter.decimal}`
: newLocalizedValue;

this.setPreviousNumberValue(previousValue ?? this.value);
this.previousValueOrigin = origin;
Expand Down
105 changes: 104 additions & 1 deletion packages/calcite-components/src/components/input/input.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1055,6 +1055,35 @@ describe("calcite-input", () => {
expect(Number(await element.getProperty("value"))).toBe(195);
});

it("allows deleting exponentail number from decimal and adding trailing zeros", async () => {
const page = await newE2EPage();
await page.setContent(html`<calcite-input type="number"></calcite-input>`);

const calciteInput = await page.find("calcite-input");
const input = await page.find("calcite-input >>> input");
await calciteInput.callMethod("setFocus");
await page.waitForChanges();
await typeNumberValue(page, "2.100e10");
await page.waitForChanges();
expect(await calciteInput.getProperty("value")).toBe("2.1e10");
expect(await input.getProperty("value")).toBe("2.1e10");

await page.keyboard.press("Backspace");
await page.waitForChanges();
expect(await calciteInput.getProperty("value")).toBe("2.1e1");
expect(await input.getProperty("value")).toBe("2.1e1");

await page.keyboard.press("Backspace");
await page.waitForChanges();
expect(await calciteInput.getProperty("value")).toBe("2.1");
expect(await input.getProperty("value")).toBe("2.1");

await page.keyboard.type("000");
await page.waitForChanges();
expect(await calciteInput.getProperty("value")).toBe("2.1000");
expect(await input.getProperty("value")).toBe("2.1000");
});

it("disallows typing any non-numeric characters with shift modifier key down", async () => {
const page = await newE2EPage();
await page.setContent(html`<calcite-input type="number"></calcite-input>`);
Expand Down Expand Up @@ -1291,6 +1320,80 @@ describe("calcite-input", () => {
expect(await calciteInput.getProperty("value")).toBe(assertedValue);
expect(await internalLocaleInput.getProperty("value")).toBe(localizedValue);
});

it(`should be able to append values after Backspace for ${locale} locale`, async () => {
const page = await newE2EPage();
await page.setContent(`
<calcite-input lang="${locale}" type="number"></calcite-input>
`);

numberStringFormatter.numberFormatOptions = {
locale,
numberingSystem: "latn",
useGrouping: false
};
const decimalSeparator = numberStringFormatter.decimal;
const calciteInput = await page.find("calcite-input");
const input = await page.find("calcite-input >>> input");
await calciteInput.callMethod("setFocus");
await typeNumberValue(page, `0${decimalSeparator}0000`);
await page.waitForChanges();
expect(await input.getProperty("value")).toBe(`0${decimalSeparator}0000`);

await page.keyboard.press("Backspace");
await typeNumberValue(page, "1");
await page.waitForChanges();
expect(await input.getProperty("value")).toBe(`0${decimalSeparator}0001`);

await typeNumberValue(page, "01");
await page.waitForChanges();
expect(await input.getProperty("value")).toBe(`0${decimalSeparator}000101`);
});

it(`should keep leading decimal separator while input is focused on Backspace ${locale} locale `, async () => {
const page = await newE2EPage();
await page.setContent(`
<calcite-input lang="${locale}" type="number"></calcite-input>
`);

numberStringFormatter.numberFormatOptions = {
locale,
numberingSystem: "latn",
useGrouping: false
};
const decimalSeparator = numberStringFormatter.decimal;
const calciteInput = await page.find("calcite-input");
const input = await page.find("calcite-input >>> input");
await calciteInput.callMethod("setFocus");
await typeNumberValue(page, `0${decimalSeparator}01`);
await page.waitForChanges();
expect(await input.getProperty("value")).toBe(`0${decimalSeparator}01`);

await page.keyboard.press("Backspace");
await page.waitForChanges();
expect(await input.getProperty("value")).toBe(`0${decimalSeparator}0`);

await page.keyboard.press("Backspace");
await page.waitForChanges();
expect(await input.getProperty("value")).toBe(`0${decimalSeparator}`);

await typeNumberValue(page, "01");
await page.waitForChanges();
expect(await input.getProperty("value")).toBe(`0${decimalSeparator}01`);
});

it(`should sanitize leading decimal zeros on initial render ${locale} locale`, async () => {
const page = await newE2EPage();
await page.setContent(html`<calcite-input value="0.0000" lang="${locale}" type="number"></calcite-input>`);

numberStringFormatter.numberFormatOptions = {
locale,
numberingSystem: "latn",
useGrouping: false
};
const input = await page.find("calcite-input >>> input");
expect(await input.getProperty("value")).toBe("0");
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add an e2e test (doesn't have to be for every locale) for this number and then type three backspaces: 2.100e10.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For context, 2.100e is NaN so I want to make sure the logic still works after deleting the e.

Copy link
Copy Markdown
Contributor Author

@anveshmekala anveshmekala Jun 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2.100e10 is auto-corrected to 2.1e1 by default.

});
});

Expand Down Expand Up @@ -1551,7 +1654,7 @@ describe("calcite-input", () => {

await page.keyboard.press("Backspace");
await page.waitForChanges();
expect(await element.getProperty("value")).toBe("1");
expect(await element.getProperty("value")).toBe("1.");
expect(calciteInputInput).toHaveReceivedEventTimes(1);
});

Expand Down
31 changes: 21 additions & 10 deletions packages/calcite-components/src/components/input/input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@ import {
} from "../../utils/loadable";
import {
connectLocalized,
defaultNumberingSystem,
disconnectLocalized,
LocalizedComponent,
NumberingSystem,
numberStringFormatter
} from "../../utils/locale";

import {
addLocalizedTrailingDecimalZeros,
BigDecimal,
isValidNumber,
parseNumberString,
Expand Down Expand Up @@ -975,13 +975,11 @@ export class Input
signDisplay: "never"
};

const sanitizedValue = sanitizeNumberString(
// no need to delocalize a string that ia already in latn numerals
(this.numberingSystem && this.numberingSystem !== "latn") ||
defaultNumberingSystem !== "latn"
? numberStringFormatter.delocalize(value)
: value
);
const isValueDeleted =
this.previousValue?.length > value.length || this.value?.length > value.length;
const hasTrailingDecimalSeparator = value.charAt(value.length - 1) === ".";
const sanitizedValue =
hasTrailingDecimalSeparator && isValueDeleted ? value : sanitizeNumberString(value);

const newValue =
value && !sanitizedValue
Expand All @@ -990,8 +988,21 @@ export class Input
: ""
: sanitizedValue;

const newLocalizedValue = numberStringFormatter.localize(newValue);
this.localizedValue = newLocalizedValue;
let newLocalizedValue = numberStringFormatter.localize(newValue);

if (origin !== "connected" && !hasTrailingDecimalSeparator) {
newLocalizedValue = addLocalizedTrailingDecimalZeros(
newLocalizedValue,
newValue,
numberStringFormatter
);
}

// adds localized trailing decimal separator
this.localizedValue =
hasTrailingDecimalSeparator && isValueDeleted
? `${newLocalizedValue}${numberStringFormatter.decimal}`
: newLocalizedValue;

this.userChangedValue = origin === "user" && this.value !== newValue;
// don't sanitize the start of negative/decimal numbers, but
Expand Down
Loading