MNTOR-1809 - Add chart into dashboard top banner #3232

codemist · 2023-07-19T13:34:46Z

References:

Jira: MNTOR-1809

Description

This adds the chart UI in the dashboard top banner. It also switches between two states:

This view before scanning:

This view after scanning:

Modal:

Checklist (Definition of Done)

Localization strings (if needed) have been added.
Commits in this PR are minimal and have descriptive commit messages.
I've added or updated the relevant sections in readme and/or code comments
I've added a unit test to test for potential regressions of this bug.
Product Owner accepted the User Story (demo of functionality completed) or waived the privilege.
All acceptance criteria are met.
Jira ticket has been updated (if needed) to match changes made during the development process.
Jira ticket has been updated (if needed) with suggestions for QA when this PR is deployed to stage.

codemist · 2023-07-19T14:20:49Z

src/app/tokens.scss

@@ -234,11 +234,22 @@ $text-body-lg: 400 clamp(16px, 1.5svw, 18px) / 1.5 var(--font-inter), sans-serif
 $text-body-md: 400 clamp(14px, 1.3svw, 16px) / 1.5 var(--font-inter), sans-serif;
 $text-body-sm: 400 clamp(12px, 1.14svw, 14px) / 1.5 var(--font-inter),
  sans-serif;
-$text-body-xs: 400 clamp(10px, 0.975svw, 12px) / 1.5 var(--font-inter),
-  sans-serif;
+$text-body-xs: 400 12px / 1.5 var(--font-inter), sans-serif;


I removed the clamp on this variable, since the font's already so small it was illegible at 10px.

locales-pending/premium.ftl

pdehaan · 2023-07-20T15:36:02Z

Ah, and a general comment since I've been meaning to file this for a while…
Re: donut chart, we probably need a better variety of colors. Having 5 shades of very similar dark purples isn't easy to distinguish in a chart and for a long time I wasn't able to tell that the first two colors were different until I squinted and zoomed in on the chart.

rhelmer · 2023-07-22T04:05:07Z

Ah, and a general comment since I've been meaning to file this for a while… Re: donut chart, we probably need a better variety of colors. Having 5 shades of very similar dark purples isn't easy to distinguish in a chart and for a long time I wasn't able to tell that the first two colors were different until I squinted and zoomed in on the chart.

I agree it's a bit hard to distinguish, if we change it we should be mindful of color insensitive vision (aka "color blindness"), there's an MDN article and explanation about how to use the Accessibility devtools: https://developer.mozilla.org/en-US/docs/Web/Accessibility/Understanding_Colors_and_Luminance

I took a quick look using the accessibility tools and the current chart was legible overall so I wouldn't want to make us worse for some folks while trying to make it better :)

flodolo · 2023-07-25T15:25:32Z

locales-pending/premium.ftl

+modal-active-number-of-exposures-title = About your number of active exposures
+# Variables:
+#   $limit (number) - Number of email addresses included in the plan
+modal-active-number-of-exposures-part-one = This chart includes the total number of times we found each type of data exposed across all data broker profiles and all data breaches for up to { $limit } email addresses that you are currently monitoring.


Please make this a plural string (see issue here)

Fixed in d7844bd.

src/app/(proper_react)/redesign/(authenticated)/user/dashboard/DashboardTopBanner.tsx

locales-pending/premium.ftl

src/app/(proper_react)/redesign/(authenticated)/user/dashboard/Dashboard.test.tsx

src/app/components/client/Chart.tsx

flodolo · 2023-07-27T07:08:39Z

locales/en/data-classes.ftl

@@ -59,6 +59,7 @@ financial-investments = Financial investments
 financial-transactions = Financial transactions
 fitness-levels = Fitness levels
 flights-taken = Flights taken
+full-names = Full name


@codemist

I missed this one in review (not sure if it was there when I reviewed, but that's not relevant).

All categories in this file are plurals. Also, why is this category being added in this PR? Just a coincidence?

@flodolo Me thinks it's related to this: #3244 (comment)
We have a bunch of these breach categories in the new dashboard UI now, so rather than add duplicate strings for localizers to localize, we reused the existing translated strings in locales/*/data-classes.ftl (although yes, that does slightly blur the fact that 99.999% of the values in that file are HIBP specific except this one).

git grep -n "full-names" | cat locales/en/data-classes.ftl:62:full-names = Full name src/app/(proper_react)/redesign/(authenticated)/user/dashboard/Dashboard.stories.tsx:158: { "full-names": 98 }, src/app/functions/server/dashboard.ts:40: fullNames: "full-names",

And yes, good callout that now there is some vague pluralizing where the ID is plural and the value is singular.
Not sure which way to fix that one, or if you think we should remove "full-names" entirely and move it into one of the files in /locales-pending/*.ftl. Plural would be more consistent with the rest of the values in data-classes.ftl, but I'm not clear on if that'd make the dashboard UI less clear, so I defer to UX/content/@codemist.

Reusing strings is a bad idea, especially if we mix them from different sources. Some locales, for example, are translating categories lowercase.

It's less than ideal in terms of localizability, but one way to minimize the impact would be to capitalize the first letter of the category (using a locale-sensitive library) when used in a different context.

codemist commented Jul 19, 2023

View reviewed changes

codemist requested review from flozia and maxxcrawford July 19, 2023 18:51

pdehaan reviewed Jul 20, 2023

View reviewed changes

locales-pending/premium.ftl Outdated Show resolved Hide resolved

locales-pending/premium.ftl Show resolved Hide resolved

rhelmer self-requested a review July 21, 2023 20:46

rhelmer approved these changes Jul 24, 2023

View reviewed changes

codemist marked this pull request as ready for review July 25, 2023 14:58

codemist requested a review from flodolo as a code owner July 25, 2023 14:58

codemist force-pushed the mntor-1809-scan-banner branch from 859516a to 78a953a Compare July 25, 2023 15:06

flodolo approved these changes Jul 25, 2023

View reviewed changes

codemist force-pushed the mntor-1809-scan-banner branch from 4d9e3c2 to 993881d Compare July 25, 2023 17:30

codemist and others added 17 commits July 25, 2023 14:01

MNTOR-1809 - Add chart into dashboard top banner

2a6ae97

clean up flex values

ee4c446

add about exposure num modal

2115741

first pass

9cd205f

add summary metrics for dashboard

876bd1d

add types

7c63c22

todo comment

2330cc5

review comment

f6fadea

review comment

e8c6ca5

map for localized key

b1438b5

inject chart data into chart component

a90adc4

use variable for num email addresses

4efb33c

fix storybook for dashboard

3b27dcc

switch between pre-scan and post-scan views

30862d5

fix breaking test

1d593d6

rm unused vars

1215501

fix l10n lint error and alphatetically organize vars

919f46b

codemist force-pushed the mntor-1809-scan-banner branch from 307b52d to 919f46b Compare July 25, 2023 18:02

add correct data in dashboard top banner desc

014e567

codemist commented Jul 25, 2023

View reviewed changes

src/app/(proper_react)/redesign/(authenticated)/user/dashboard/DashboardTopBanner.tsx Show resolved Hide resolved

codemist added 5 commits July 25, 2023 14:32

rm console log

f8afc68

lint

04dd956

lint

f7aac09

pluralize string

d7844bd

fix stories

58d16f5

pdehaan reviewed Jul 25, 2023

View reviewed changes

locales-pending/premium.ftl Outdated Show resolved Hide resolved

src/app/(proper_react)/redesign/(authenticated)/user/dashboard/Dashboard.test.tsx Show resolved Hide resolved

src/app/components/client/Chart.tsx Show resolved Hide resolved

pdehaan reviewed Jul 25, 2023

View reviewed changes

src/app/components/client/Chart.tsx Show resolved Hide resolved

mansaj and others added 3 commits July 25, 2023 16:42

add total nums (#3252)

d614a31

add special character for ×

c7c4355

arrange colours of chart by odd first, then even

2dce8fc

mansaj approved these changes Jul 26, 2023

View reviewed changes

remove unused var

ebfcd2c

codemist force-pushed the mntor-1809-scan-banner branch from 3ad5937 to ebfcd2c Compare July 26, 2023 16:07

Merge branch 'main' into mntor-1809-scan-banner

e8704b5

codemist merged commit 0951490 into main Jul 26, 2023

codemist deleted the mntor-1809-scan-banner branch July 26, 2023 16:29

flodolo reviewed Jul 27, 2023

View reviewed changes

codemist mentioned this pull request Aug 1, 2023

Remove full names from data classes #3279

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MNTOR-1809 - Add chart into dashboard top banner #3232

MNTOR-1809 - Add chart into dashboard top banner #3232

codemist commented Jul 19, 2023 •

edited

Loading

codemist Jul 19, 2023

pdehaan commented Jul 20, 2023

rhelmer commented Jul 22, 2023

flodolo Jul 25, 2023

codemist Jul 25, 2023

flodolo Jul 27, 2023

pdehaan Jul 27, 2023

flodolo Jul 27, 2023

flodolo Jul 27, 2023

MNTOR-1809 - Add chart into dashboard top banner #3232

MNTOR-1809 - Add chart into dashboard top banner #3232

Conversation

codemist commented Jul 19, 2023 • edited Loading

References:

Description

Checklist (Definition of Done)

codemist Jul 19, 2023

Choose a reason for hiding this comment

pdehaan commented Jul 20, 2023

rhelmer commented Jul 22, 2023

flodolo Jul 25, 2023

Choose a reason for hiding this comment

codemist Jul 25, 2023

Choose a reason for hiding this comment

flodolo Jul 27, 2023

Choose a reason for hiding this comment

pdehaan Jul 27, 2023

Choose a reason for hiding this comment

flodolo Jul 27, 2023

Choose a reason for hiding this comment

flodolo Jul 27, 2023

Choose a reason for hiding this comment

codemist commented Jul 19, 2023 •

edited

Loading