feat(atomic): add formats for new numeric facet #911

ThibodeauJF · 2021-06-21T11:03:23Z

https://coveord.atlassian.net/browse/KIT-751

Upcoming work:

add custom inputs

https://www.figma.com/file/BWSlnVeORhILh79Z2gSf1U/Atomic-v1?node-id=0%3A1
https://docs.google.com/document/d/1Mg-5Iuh88trrulUUYzfQh2npSboMhzvWkXzaMQDsUEs/edit#heading=h.soxpq1m179g6

https://coveord.atlassian.net/browse/KIT-751

ThibodeauJF · 2021-06-21T11:04:49Z

packages/atomic/tsconfig.json

@@ -6,11 +6,11 @@
    "experimentalDecorators": true,
    "lib": [
      "dom",
-      "es2017"
+      "es2020"


Bumped in order to get the latest changes for that issue microsoft/TypeScript#38012
PR adds to ES2020 and not ES2017 microsoft/TypeScript#38013

I could also roll it back and use declaration merging for the NumberFormatOptions

I think it should be good for all modern browser, so...

ThibodeauJF · 2021-06-21T11:05:56Z

packages/atomic/src/components/formats/format-common.ts

+  formatter: NumberFormatter,
+  element: Element
+) => {
+  const event = buildCustomEvent('atomic/numberFormat', formatter);


Any component that will listen to that event can use the formats web component as children

This might pose a small challenge for documentation, to be able to express that component X should be used with component Y (child/parent).

Food for thoughts @dlutzCoveo :)

RIght now only that facet, in the future, possibly some result template component e.g. atomic-result-number

We could leverage doc tags here 🤔

github-actions · 2021-06-21T11:14:28Z

Pull Request Report

PR Title

✅ Title follows the conventional commit spec.

Bundle Size

File	Compression	Old (kb)	New (kb)
dist/browser/headless.js	bundled	535.9	535.9
	minified	220.7	220.7
	gzipped	63	63
dist/browser/headless.esm.js	bundled	519.1	519.1
	minified	283.8	283.8
	gzipped	69	69
dist/browser/case-assist/headless.js	bundled	0.5	0.5
	minified	0.3	0.3
	gzipped	0.2	0.2
dist/browser/case-assist/headless.esm.js	bundled	0.1	0.1
	minified	0	0
	gzipped	0.1	0.1
dist/browser/recommendation/headless.js	bundled	352.2	352.2
	minified	156.4	156.4
	gzipped	45.3	45.3
dist/browser/recommendation/headless.esm.js	bundled	343.1	343.1
	minified	193.4	193.4
	gzipped	49.5	49.5
dist/browser/product-recommendation/headless.js	bundled	347.5	347.5
	minified	154.2	154.2
	gzipped	44.8	44.8
dist/browser/product-recommendation/headless.esm.js	bundled	338.4	338.4
	minified	191.3	191.3
	gzipped	49	49

nbuissoncoveo · 2021-06-21T13:03:46Z

packages/atomic/src/components/facets-v1/atomic-numeric-facet/atomic-numeric-facet.tsx

+    event.preventDefault();
+    event.stopPropagation();


I don't understand the purpose of these two lines.

preventDefault: If invoked when the cancelable attribute value is true, and while executing a listener for the event with passive set to false, signals to the operation that caused event to be dispatched that it needs to be canceled.

It allows us to know when the event has been caught by an handler, this way, if it's not caught, we can display an error saying "The component was not handled as it is not a child of a compatible component"

stopPropagation: When dispatched in a tree, invoking this method prevents event from reaching any objects other than the current object.

Basically prevents the event from bubbling up once it's caught, a safety mesure

packages/atomic/src/components/formats/atomic-format-currency.tsx

packages/atomic/src/components/facets-v1/atomic-numeric-facet/atomic-numeric-facet.tsx

olamothe · 2021-06-21T13:56:10Z

packages/atomic/src/components/formats/atomic-format-currency.tsx

+
+  /**
+   * The currency to use in currency formatting. Possible values are the ISO 4217 currency codes, such as "USD" for the US dollar, "EUR" for the euro, or "CNY" for the Chinese RMB.
+   * See the current [currency & funds code list](https://tc39.es/proposal-unified-intl-numberformat/section6/locales-currencies-tz_proposed_out.html#sec-currency-codes).


I checked that URL but could not see any list... 🤔

updated with link in mozilla doc

olamothe · 2021-06-21T13:57:04Z

packages/atomic/src/components/formats/atomic-format-currency.tsx

+  @Prop() public currency!: string;
+
+  componentWillLoad() {
+    try {


Why could this throw an error ? In what scenario ?

edit: Is it the parseFloat inside format ?

toLocaleString throws errors e.g. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/toLocaleString#checking_for_support_for_locales_and_options_arguments

If you use a wrong, locale, unit, currency, etc.

packages/atomic/src/components/formats/atomic-format-currency.tsx

packages/atomic/src/components/formats/atomic-format-number.tsx

olamothe · 2021-06-21T13:59:30Z

packages/atomic/src/components/formats/atomic-format-number.tsx

+  @Prop() public maximumSignificantDigits?: number;
+
+  componentWillLoad() {
+    try {


Same question as above regarding why this can throw an error.

olamothe · 2021-06-21T14:01:50Z

packages/atomic/src/components/formats/format-common.ts

+  formatter: NumberFormatter,
+  element: Element
+) => {
+  const event = buildCustomEvent('atomic/numberFormat', formatter);


This might pose a small challenge for documentation, to be able to express that component X should be used with component Y (child/parent).

Food for thoughts @dlutzCoveo :)

olamothe · 2021-06-21T14:03:49Z

packages/atomic/tsconfig.json

@@ -6,11 +6,11 @@
    "experimentalDecorators": true,
    "lib": [
      "dom",
-      "es2017"
+      "es2020"


I think it should be good for all modern browser, so...

packages/atomic/src/components.d.ts

packages/atomic/src/components/formats/atomic-format-currency.tsx

packages/atomic/src/components/formats/atomic-format-number.tsx

packages/atomic/src/components/formats/atomic-format-unit.tsx

packages/atomic/src/components/formats/format-common.ts

https://coveord.atlassian.net/browse/KIT-751

feat(atomic): add formats for new numeric facet

e86979f

https://coveord.atlassian.net/browse/KIT-751

ThibodeauJF requested review from btaillon-coveo, nbuissoncoveo, olamothe and samisayegh as code owners June 21, 2021 11:03

ThibodeauJF commented Jun 21, 2021

View reviewed changes

ThibodeauJF requested a review from dlutzCoveo June 21, 2021 11:06

nbuissoncoveo approved these changes Jun 21, 2021

View reviewed changes

nbuissoncoveo reviewed Jun 21, 2021

View reviewed changes

samisayegh approved these changes Jun 21, 2021

View reviewed changes

packages/atomic/src/components/formats/atomic-format-currency.tsx Outdated Show resolved Hide resolved

olamothe approved these changes Jun 21, 2021

View reviewed changes

dlutzCoveo reviewed Jun 21, 2021

View reviewed changes

ThibodeauJF added 2 commits June 21, 2021 13:08

PR fixes

dd1ccc1

https://coveord.atlassian.net/browse/KIT-751

fix string

f5e034e

https://coveord.atlassian.net/browse/KIT-751

ThibodeauJF merged commit aca6a25 into master Jun 21, 2021

ThibodeauJF deleted the KIT-751-3 branch June 21, 2021 19:26

feat(atomic): add formats for new numeric facet #911

feat(atomic): add formats for new numeric facet #911

Uh oh!

Conversation

ThibodeauJF commented Jun 21, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jun 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThibodeauJF Jun 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jun 21, 2021 •

edited

Loading

ThibodeauJF Jun 21, 2021 •

edited

Loading