🚧 UPDATE: format & number #129

[jycouet]

• add tests for numbers
• Add app defaults.
• Add app presets

[changeset-bot]

🦋 Changeset detected

Latest commit: d5f6377

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
svelte-ux	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
svelte-ux	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 17, 2023 2:14pm

[techniq]

@jycouet This is looking great.

Do we need formatNumber & formatNumberAsStyle? Or one could be enough?

I would love to only have a single formatNumber() that handles all use cases. I think renaming formatNumberAsStyle() to formatNumber() is likely the best approach. I believe using d3-format gives us the most control of formatting compared to Intl.NumberFormat, although it could be I haven't explored it deep enough. I've been meaning to add a test suite for this and appreciate you taking the time to do so. I do have a basic format doc page to show some of the format() usage (which calls formatNumberAsStyle() for numbers).

A quick list of use cases off the top of my head

Convenient formatting

Using simple string presets (integer, decimal, currency, etc) to conveniently format a number. This is used by Table, SpringValue, and TweenedValue in Svelte UX, and Axis, Labels, Legend, and TooltipItem in LayerChart (maybe others). Short/rounded metric values are nice, especially for Chart axis labels or summarized dashboard values (not the best name tbh, but the best I could come up with at the time.).

format() is basically a facade to format numbers along with dates (ex. PeriodType.Day) and applying adhoc functions. Updating date formatting to support locales would likely be something to look at soon (based on your needs). It's going to have similar issue with "convenient" presets but also nice to have full format string support for custom formatting, and might be even tricky in practice :(.

Internationalization

Initially I had hopes to just use the browser's default locale (such as what navigator.language) but I know it's not that easy, and likely needs to be user supplied/override capable. It looks like d3-format's formatLocale / formatDefaultLocale was the magic sauce we needed.

At first glance it looks like you need to be explicit with these formats, unlike Intl.NumberFormat() which takes in the locale. Am I overlooking something, or would we need/want to create a map of locales to return the appropriate settings?

This looks great, and i'm on board with both simplifying and supporting non-English locales/Intl.

If it would be helpful to sync up on a video chat, let me know and I can try to free up some time.

[jycouet]

The only drawback of d3-format compared to Intl.NumberFormat is the size. But it's probably ok.

Automatic local (navigator.language) is good and bad. In my case, I don't care about user browser settings, I want to display €.

Main point for me is setting once option, then make use of it. (now writing this, maybe it should be any array of options... and take the first one (or a default one) if not specified)

Great feedback 👍, Yes, let's have a video chat soon.

[techniq]

The only drawback of d3-format compared to Intl.NumberFormat is the size. But it's probably ok.

Agreed. If we can get similar flexibility using Intl.NumberFormat I'm all for using it. It's mostly supporting the presets ('integer', 'decimal', 'currency', 'percent', 'percentRound', and 'metric').

Metric (significant digits) might be tricky. It looks like unit: byte might work, although I'm not sure if this would be using 1024 or 1000 when "rounding". I typically use this to show large summarized currency values (50K, 10M dollars, etc) and often for chart axis labels.

const byteValueNumberFormatter = Intl.NumberFormat("en", {
  notation: "compact",
  style: "unit",
  unit: "byte",
  unitDisplay: "narrow",
});

console.log(byteValueNumberFormatter.format(10));
=> 10B

console.log(byteValueNumberFormatter.format(200000));
=> 200KB

console.log(byteValueNumberFormatter.format(50000000));
=> 50MB

If we think it's worth it (and I think it might), we can try to go through all the presets and find the applicable settings. We might also be able to add a little pre/post processing to get them "just right" (for example take off the "B" (byte) for "metric".

I like the idea of dropping a dependency :). Intl.NumberFormat has great browser support.

Automatic local (navigator.language) is good and bad. In my case, I don't care about user browser settings, I want to display €.

Main point for me is setting once option, then make use of it. (now writing this, maybe it should be any array of options... and take the first one (or a default one) if not specified)

Yeah, I can of suspected that.

Great feedback 👍, Yes, let's have a video chat soon.

Thanks, and sounds good.

[jycouet]

All right @techniq I guess that you can have a look 👍
I left a few comments / TODO / & fail test in purpose for you to comment

[techniq]

I think I'd like to call this Settings instead of Setup (everywhere, not just this instance).

I could see apply an override of settings somewhere in your component tree (since it's just context). For example maybe if you had routing with the locale (/en/..., /fr/..., etc), or you wanted to style a page/section different (frontpage marketing vs app), etc.

It would also be nice to have a <Settings> component like <Theme> (which just makes it easier to set the context)

[jycouet]

All right, I'll update to Settings.

The override thing, you can already do it no?

[techniq]

Is there any risk of having only a single numbers options, or benefit to having options per style/preset?

For example, if fractionDigits: 4, wouldn't all currency be set with 4 (since it doesn't specify a min/max in formatNumber()), or would you want to be able to specify them separately...

{
  formats: {
    numbers: {
      default: {
        locales: 'en',
      },
      currency: {
        currency: 'USD',
        fractionDigits: 2,
      },
      decimal: {
        fractionDigits: 4
      }
    }
  }
}

Just kind of thinking out loud

[jycouet]

Yeah, it's also an options, I like it. I'm hesitant... Adding default right now is a good thing.
Let's see during my next night 😅

[jycouet]

Okay... To be checked with what I just did

[techniq]

@jycouet This is really good! A few more small comments, but I think that will be it.

Also, the Vercel preview build is failing due to createTheme() still be referenced in a handful of places. I think we just need to replace <Theme> with <Settings> and update createTheme({...}) with settings({...}) in these locations.

[techniq]

@jycouet I don't assume we should omit these and have it default to the browser's default locale? These settings work for me, mostly asking for a non-US view :). If not, let's keep them.

[jycouet]

I would keep it like this as default.
Let's see when people have a dedicated use case.

With local I will put 3000€, locally everything will work and Co, but people in Hungary will see 3000 Forint (that's 8€)!!! 😅

[techniq]

Awe, very good point! Thanks!

[techniq]

@jycouet I'm ready to merge if you are. I left a note about whether we should default to en/USD, but if you're happy with that, I am.

[techniq]

Awesome work!