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

Format numbers in Key Metric Widget tiles #7190

Closed
jimmymadon opened this issue Jun 21, 2023 · 7 comments
Closed

Format numbers in Key Metric Widget tiles #7190

jimmymadon opened this issue Jun 21, 2023 · 7 comments
Labels
P0 High priority Type: Enhancement Improvement of an existing feature

Comments

@jimmymadon
Copy link
Collaborator

jimmymadon commented Jun 21, 2023

Feature Description

Larger numbers for total figures in all currently implemented KMW tiles should be formatted as per the Figma mock below.

Expected:
image

Actual:
image


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • All secondary numbers in Key Metrics Widget tiles should be formatted with the appropriate locale-based format (e.g. 1234 in en-US: 1,234)
    • The style should not use the default metric type and generally use the appropriate formatting options to match the design (e.g. integers shouldn't end in .0, etc)
  • This includes (but is not limited to) the "total visitors" figure in both, the "New Visitors" and "Loyal Visitors" KM widgets

Implementation Brief

  • Merge this PR which uses numFmt with the decimal style to format totals in both the widgets mentioned in the AC.

Test Coverage

  • No new tests required.

QA Brief

  • Verify that both, the New Visitors and Loyal Visitors widgets, show the formatted number strings. Storybook stories don't have high enough numbers to test this formatting.

Changelog entry

  • Improve formatting of larger numbers in Key Metric Widget tiles.
@jimmymadon jimmymadon added P1 Medium priority Type: Enhancement Improvement of an existing feature labels Jun 21, 2023
@aaemnnosttv
Copy link
Collaborator

@jimmymadon the format specified by the AC is locale-specific. The number should simply be properly formatted according to the locale, just like any other number.

@jimmymadon
Copy link
Collaborator Author

@aaemnnosttv Updated the AC slightly here and moved it to ACR.

@aaemnnosttv
Copy link
Collaborator

Thanks @jimmymadon – are there any other widget tiles that are in process which could need the same fix here or does it only affect these two? Also are there any that are in the EB that could use a note added to avoid missing this again?

@jimmymadon jimmymadon changed the title Format large numbers in KMW tiles Format numbers in KMW tiles Jun 22, 2023
@jimmymadon
Copy link
Collaborator Author

@aaemnnosttv Thanks - I've added a note on #6247 for @eugene-manuilov as that issue is in CR and added a note in the ACs/IB for #6249. Those are the only two Key Metric widget tile issues which have numbers that are not percentages. We do end up formatting all percentages anyways so those should be fine.

@jimmymadon jimmymadon assigned aaemnnosttv and unassigned jimmymadon Jun 22, 2023
@aaemnnosttv
Copy link
Collaborator

Thanks @jimmymadon – for the AC I think we can word them in a way that is not specific to these two tiles. This essentially applies to all "secondary" numbers in KMW tiles (those aside from metric and change).

  • like other numbers in the plugin

Most other numbers in the plugin are formatted using numFmt which defaults to the "metric" style. If applied here, we'd end up with a number like 1.6K instead of 1,681, so we'll want to make sure that we want to use this alternate style, and that we're a little more explicit about this requirement.

I'll update this to move it forward for the sake of time 👍

@aaemnnosttv aaemnnosttv removed their assignment Jun 22, 2023
@jimmymadon jimmymadon self-assigned this Jun 26, 2023
@jimmymadon jimmymadon removed their assignment Jun 28, 2023
@tofumatt tofumatt self-assigned this Jun 28, 2023
@tofumatt
Copy link
Collaborator

IB ✅

Since this has a PR ready I'll review it as well, so moving this right to CR 🙂

@tofumatt tofumatt changed the title Format numbers in KMW tiles Format numbers in Key Metric Widget tiles Jun 28, 2023
@tofumatt tofumatt removed their assignment Jun 28, 2023
@bethanylang bethanylang added P0 High priority and removed P1 Medium priority labels Jun 29, 2023
@mohitwp mohitwp self-assigned this Jul 3, 2023
@mohitwp
Copy link
Collaborator

mohitwp commented Jul 5, 2023

QA Update ✅

  • Tested on dev environment.
  • Verified under "new Visitor' and "Loyal Visitors" KMW tiles as other tiles don't have that much data or not implmented yet.
  • Verified total visitors numbers are showing formatted number strings.

image

@mohitwp mohitwp removed their assignment Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 High priority Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

5 participants