feat(date-picker, input-date-picker): add numberingSystem property #5488

benelan · 2022-10-17T02:01:06Z

Related Issue: #4893

Summary

Add numberingSystem property to date-picker and input-date-picker

jcfranco

⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐
⭐🤩🤩🤩🤩⭐🤩⭐⭐⭐🤩⭐🤩🤩🤩🤩⭐🤩⭐⭐🤩⭐🤩🤩🤩🤩🤩⭐🤩🤩🤩⭐🤩⭐⭐⭐🤩⭐🤩🤩🤩🤩⭐🤩⭐
⭐🤩⭐⭐🤩⭐🤩🤩⭐🤩🤩⭐🤩⭐⭐🤩⭐🤩⭐⭐🤩⭐⭐⭐⭐🤩⭐⭐⭐🤩⭐⭐🤩🤩⭐⭐🤩⭐🤩⭐⭐⭐⭐🤩⭐
⭐🤩🤩🤩🤩⭐🤩⭐🤩⭐🤩⭐🤩🤩🤩🤩⭐🤩🤩🤩🤩⭐⭐⭐🤩⭐⭐⭐⭐🤩⭐⭐🤩⭐🤩⭐🤩⭐🤩⭐🤩🤩⭐🤩⭐
⭐🤩⭐⭐🤩⭐🤩⭐⭐⭐🤩⭐🤩⭐⭐🤩⭐🤩⭐⭐🤩⭐⭐🤩⭐⭐⭐⭐⭐🤩⭐⭐🤩⭐⭐🤩🤩⭐🤩⭐⭐🤩⭐⭐⭐
⭐🤩⭐⭐🤩⭐🤩⭐⭐⭐🤩⭐🤩⭐⭐🤩⭐🤩⭐⭐🤩⭐🤩🤩🤩🤩🤩⭐🤩🤩🤩⭐🤩⭐⭐⭐🤩⭐🤩🤩🤩🤩⭐🤩⭐
⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐⭐

jcfranco · 2022-10-20T21:38:27Z

src/components/date-picker-day/date-picker-day.e2e.ts


 describe("calcite-date-picker-day", () => {
  it("can be disabled", async () => {
+    numberStringFormatter.numberFormatOptions = {


Is this needed for the disabled test?

Yeah numberFormatOptions needs to be set at some point with the way the class is currently configured because all of the initialization happens in its setter.

numberFormatOptions is always set in components (or their parents in the case of date-picker-day) with the values of the lang/numberingSystem props.

I could add a constructor to the class to initialize values with the default numberingSystem/lang, but that's just an extra iteration we don't need in the actual components. What do you think?

Or I could just return the localize/delocalize methods' params if numberFormatOptions isn't set, which would effectively assume lang === "en && numberingSystem === "latn". e.g.

localize = (numberString: string) => Object.keys(this._numberFormatOptions).length ? doFormattyStuff() : numberString;

It would be the equivolent outcome as adding a constructor and actually populating the internal properties based on defaults, but without the extra formatter creations.

I went with my second comment ☝️

Now numberStringFormatter.numberFormatOptions only needs to be set when the lang and numberingSystem props are not their default values ("en" and "latn" respectively). This means that for a good portion of our users, we won't be initializing a formatter at all.

That's down from 10+ formatters created/destroyed for each character typed in calcite-input alone 🚀

jcfranco · 2022-10-20T21:38:42Z

src/components/date-picker-day/date-picker-day.tsx

@@ -38,7 +38,7 @@ export class DatePickerDay implements InteractiveComponent {
  //--------------------------------------------------------------------------

  /** Day of the month to be shown. */
-  @Prop() day: number;
+  @Prop() day!: number;


jcfranco · 2022-10-20T21:39:04Z

src/components/date-picker-day/date-picker-day.tsx

-      .split("")
-      .map((i) => this.localeData.numerals[i])
-      .join("");
+    const formattedDay = numberStringFormatter.localize(String(this.day));


jcfranco · 2022-10-20T21:40:19Z

src/components/date-picker-month-header/date-picker-month-header.tsx

  }

  private parseCalendarYear(year: string): string {
    const { localeData } = this;
    const buddhistCalendar = localeData["default-calendar"] === "buddhist";
    const yearOffset = buddhistCalendar ? BUDDHIST_CALENDAR_YEAR_OFFSET : 0;

-    return localizeNumber(parseNumber(year, localeData) - yearOffset, localeData);
+    const parsedYear = Number(numberStringFormatter.delocalize(year)) - yearOffset;
+    return numberStringFormatter.localize(parsedYear.toString());


Nitpick: using template literals can be more concise. Up to you.

Yeah you're right template literals are cleaner here. I use toString() out of habit because I love method chaining lol. Fixing!

jcfranco · 2022-10-20T22:04:18Z

src/components/input-date-picker/input-date-picker.tsx

-  inputInput = (event: CustomEvent<any>): void => {
-    this.input(event.detail.value);
+  startInputInput = (event: CustomEvent<any>): void => {
+    const parsedValue = this.parseNumerals(event.detail.value);


Can the value be obtained from the component instead?

lol I was wondering why it was set up like that, but changing legacy code is scary. Fixing!

jcfranco · 2022-10-20T22:05:57Z

src/utils/date.spec.ts

@@ -124,23 +114,37 @@ describe("nextMonth", () => {
  });
 });

-describe("localizeNumber", () => {
+describe("format number", () => {


src/utils/date.spec.ts

* feat(button): add built-in translations * change watcher order * remove intl default test assertion * cleanup * handle edge case * fix build errors * docs(conventions): update style guide (#5530) * fix(slider): fix slider (single-value) error when clicking range (#5533) #5321 * 1.0.0-next.603 * ci(screener): reenable screener on pushes to master to update baseline (#5528) * ci(screener): renable screener on pushes to master to update baseline * cleanup Co-authored-by: Ben Elan <benelan@users.noreply.github.com> * fix: components should only react to primary button pointer events. (#5519) * fix: components should only react to primary pointer events. * cleanup. add pointerdown * fix test * make sure primary? * review fixes * 1.0.0-next.604 * feat(date-picker, input-date-picker): add numberingSystem property (#5488) * feat(date-picker): add numberingSystem property * docs(date-picker): cleanup lang/numberingSystem selection code * feat(input-date-picker): add numberingSystem property [WIP] * add stories * cleanup * fix spec test * cleanup * test(date-picker-day): fix failing disabled commonTest * merge master * cleanup * review cleanups * skip (de)localization when formatter is not initialized * cleanup and add a spec test * don't init formatter until necessary * final cleanup * replace stepped stories and change locale to lang * test(input-date-picker): add dark theme class to darkThemeRTL story Co-authored-by: Ben Elan <benelan@users.noreply.github.com> * 1.0.0-next.605 * docs: consistent api styling + define card thumbnailPosition (#5518) * docs(card): defined thumbnailPosition and consistent api styling across component * fix typo * remove extraneous dash * add backticks to comps * add b + c component consistency * edits * doc feedback * fix(value-list-item): Change drag handle color. (#5543) * 1.0.0-next.606 * fix: add custom logic for floating-ui positioning across shadow DOM in non-Chromium browsers (#5542) * fix: add custom code for floating-ui positioning across shadow DOM in non-Chrome browsers * tidy up * fix conditional to avoid setting up fix for non-browser environments * provide a way to opt-out of fix in favor of performance * fix: fix jarring positioning when a closed component is first opened (#5484) * fix: fix jarring positioning when a closed component is first opened * uncomment actual fix and switch story to be stepped * tweak story to capture initial positioning * fix typo * add missing setAttr argument * add delay before screenshot test setup * revisit approach to preserve debounced internal repositioning calls and correct positioning * drop unnecessary story * tidy up * fix hydrate build * restore leading option * update test * tweak positioning * drop scale transformto have correct dimensions initially * reposition immediately on componentDidLoad * update tests * revert index.html updates * use transitions to reset positioning * fix test. Co-authored-by: Matt Driscoll <mdriscoll@esri.com> * 1.0.0-next.607 * fix(calcite-loader, calcite-input-message): drop active in favor of hidden (#5537) * fix(calcite-loader, calcite-input-message): drop active in favor of hidden * changes applied to input-message * modify render assertion in the tests * remove redundant hidden prop along with documentation (global attribute) * add the link to mdn hidden attribute * 1.0.0-next.608 * feat(flow-item): Add calciteFlowItemScroll event (#5547) * feat(flow-item): Add scroll event. (#5546) * add a test * 1.0.0-next.609 * ci: adds w3c url for a11y issue filing (#5556) ci: update need info url for a11y * fix(date-picker): display correct date format order in header for zh-CN locale. (#5534) fix(date-picker): display correct date format order in header for zh-CN locale * 1.0.0-next.610 * fix(stepper-item): make sure numberingSystem is rendered on load (#5640) * fix(stepper-item): set numberingSystem in render block * cleanup * cleanup the cleanup Co-authored-by: Ben Elan <benelan@users.noreply.github.com> * 1.0.0-next.611 * feedback changes * remove browser build conditional * fix(slider): dragging range fires input event (#5641) #5449 * fix(tooltip): Prevent opening when closeOnClick is true and referenceElement is clicked quickly (#5643) * fix(tooltip): Stop hover event when closeOnClick is true and click occurs. (#5538) * add test * review cleanup * fix(types): fix type issue caused by unintentionally moving @floating-ui/dom as a dev dependency (#5649) * avoid fetching in hydrate builds * resolve conflicts * clean up * more cleanup Co-authored-by: Ben Elan <belan@esri.com> Co-authored-by: JC Franco <jfranco@esri.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Ben Elan <benelan@users.noreply.github.com> Co-authored-by: Matt Driscoll <mdriscoll@esri.com> Co-authored-by: Kitty Hurley <khurley@esri.com> Co-authored-by: Eliza Khachatryan <eli97736@esri.com> Co-authored-by: Alison Stump <alisonailea@users.noreply.github.com>

benelan and others added 5 commits October 14, 2022 19:06

feat(date-picker): add numberingSystem property

b292585

docs(date-picker): cleanup lang/numberingSystem selection code

8408f06

feat(input-date-picker): add numberingSystem property [WIP]

72618d9

add stories

e228ec7

cleanup

561456e

benelan requested a review from a team as a code owner October 17, 2022 02:01

github-actions bot assigned benelan Oct 17, 2022

github-actions bot added the enhancement Issues tied to a new feature or request. label Oct 17, 2022

benelan and others added 3 commits October 16, 2022 19:58

fix spec test

f49008f

cleanup

8cae4ff

test(date-picker-day): fix failing disabled commonTest

158862a

benelan force-pushed the benelan/4893-date-picker-numbering-system branch from d76baba to 158862a Compare October 17, 2022 22:06

benelan and others added 3 commits October 17, 2022 16:02

merge master

01be7d3

cleanup

2bef53d

Merge branch 'master' into benelan/4893-date-picker-numbering-system

7ec6565

jcfranco approved these changes Oct 20, 2022

View reviewed changes

benelan and others added 7 commits October 20, 2022 18:35

Merge branch 'master' into benelan/4893-date-picker-numbering-system

cad5a3f

review cleanups

11fd935

skip (de)localization when formatter is not initialized

44c8588

cleanup and add a spec test

519c826

don't init formatter until necessary

ddb8230

Merge branch 'master' into benelan/4893-date-picker-numbering-system

748f79d

final cleanup

b96a9d3

benelan added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Oct 21, 2022

replace stepped stories and change locale to lang

c605d33

benelan added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Oct 22, 2022

test(input-date-picker): add dark theme class to darkThemeRTL story

168b73f

benelan added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Oct 22, 2022

benelan merged commit 2e025d8 into master Oct 22, 2022

benelan deleted the benelan/4893-date-picker-numbering-system branch October 22, 2022 01:41

github-actions bot added this to the Sprint 2022/10/24 - 2022/11/04 milestone Oct 22, 2022

benelan mentioned this pull request Oct 26, 2022

fix(date-picker): cached formatter does not conflict with our numberingSystem components #5645

Merged

geospatialem mentioned this pull request Nov 30, 2022

The popup of the input-date-picker and input-time-picker didn't change language after setting the "lang" property. #5855

Closed

benelan mentioned this pull request Dec 29, 2022

fix(input-date-picker): update input value when changing locale #6197

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(date-picker, input-date-picker): add numberingSystem property #5488

feat(date-picker, input-date-picker): add numberingSystem property #5488

benelan commented Oct 17, 2022

jcfranco left a comment

jcfranco Oct 20, 2022

benelan Oct 21, 2022

benelan Oct 21, 2022 •

edited

Loading

benelan Oct 21, 2022

jcfranco Oct 20, 2022

jcfranco Oct 20, 2022

jcfranco Oct 20, 2022

benelan Oct 21, 2022

jcfranco Oct 20, 2022

benelan Oct 21, 2022

jcfranco Oct 20, 2022

feat(date-picker, input-date-picker): add numberingSystem property #5488

feat(date-picker, input-date-picker): add numberingSystem property #5488

Conversation

benelan commented Oct 17, 2022

Summary

jcfranco left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benelan Oct 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benelan Oct 21, 2022 •

edited

Loading