Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support format_d3_locale in the metrics YAML #5974

Merged
merged 3 commits into from
Nov 13, 2024

Conversation

begelundmuller
Copy link
Contributor

@begelundmuller begelundmuller commented Oct 24, 2024

Enables passing locale config to D3. For details about the available properties, see: https://d3js.org/d3-format#formatLocale.

Example measure:

measures:
  - name: b
    expression: count(*)
    format_d3: "$.2f"
    format_d3_locale:
        currency: ["£", ""]

The D3 locale properties are returned in the MetricsViewSpec.MeasureV2 type for frontend consumption (cc @djbarnwal).

@begelundmuller begelundmuller self-assigned this Oct 24, 2024
Copy link
Member

@djbarnwal djbarnwal left a comment

Choose a reason for hiding this comment

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

Can we have more stricter type definitions?

@@ -2505,6 +2505,8 @@ export interface NumericHistogramBinsBin {
count?: number;
}

export type MetricsViewSpecMeasureV2FormatD3Locale = { [key: string]: any };
Copy link
Member

Choose a reason for hiding this comment

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

Can we type it more strongly?

Copy link
Member

Choose a reason for hiding this comment

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

Probably something along the lines of

interface FormatLocaleDefinition {
  decimal?: string; // Character for the decimal point (e.g., '.')
  thousands?: string; // Character for thousands separator (e.g., ',')
  grouping?: number[]; // Array defining group sizes (e.g., [3] for groups of 3 digits)
  currency?: [string, string]; // Array for prefix and suffix currency symbols (e.g., ['$', ''])
  numerals?: string[]; // Array of numeral representations, if different from 0-9
  percent?: string; // Symbol used for percentage (e.g., '%')
  minus?: string; // Symbol for minus (negative) values (e.g., '-')
  nan?: string; // Representation of "Not a Number" (e.g., 'NaN')
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can copy D3's spec and validation on the runtime side if you really think it's worth it, but I would prefer to just leave the validation and reporting of any errors in the locale to D3. The error will show up in the UI instead of the reconcile logs since it happens at runtime, but since this is an advanced feature and unlikely to act differently in production, I think that should be okay. Apart from the duplicated logic, it also saves us from needing to stay in sync with any additional options D3 adds.

@begelundmuller begelundmuller merged commit 6c18b46 into main Nov 13, 2024
11 checks passed
@begelundmuller begelundmuller deleted the begelundmuller/d3-format-locale branch November 13, 2024 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants