adopt the new classes for plot's big numbers #554

Fil · 2024-01-17T00:26:54Z

the new classes from #547 require less code overall to create big numbers; there are two things to fix, though:

by default the trend is too close to the big number; not sure how to address this cleanly — I'm currently adding a margin-right on .big to fix this.
The link (for open PRs) makes the number blue, and I want to make it black again (or white on dark background)

I've fixed these two issues in 1ab1eb0 but it's not really fancy.

related: #248

closes #582

examples/plot/docs/index.md

Co-authored-by: Mike Bostock <mbostock@gmail.com>

examples/plot/docs/index.md

Co-authored-by: Mike Bostock <mbostock@gmail.com>

mbostock · 2024-01-22T16:58:36Z

examples/google-analytics/docs/index.md

+  <div class="card">
+    <h2>Rolling 28-day Active users</h2>
+    <span class="big">${summary[summary.length-1].active28d.toLocaleString("en-US")}</span>
+    <span class="small">${((v) => html`<span class="${v > 0 ? "green" : v < 0 ? "red" : ""}">${bigNumber(v)} ${v > 0 ? "↗︎" : v < 0 ? "↘︎" : ""}`)(getCompareValue(summary, 'active28d'))}</span>


I think this has too much complexity to be practical. We need to find a way to encapsulate this.

I've put the logic in a function of (value, format).

I don’t see the change here. It looks like you did this for the Plot example but not the Google Analytics one. 😓

Also, I don’t think this really solves the problem I raised because someone still has to find the trend function you wrote — we need to think about how to incorporate a practical solution “in the box” (in the standard library or default CSS).

(And also I want inline expressions for attributes #32 or even better server-side rendering #234…)

examples/plot/docs/index.md

mbostock

I found lots more to clean up. We should run Prettier on this stuff, I guess, if it’s too hard to maintain style manually.

I think the bigger issue is that I want some more encapsulation for the green +42% ↗︎. It feels to me like something people will often want, and the fiddly combination of setting the class dynamically and finding the Unicode arrows seems impractical (given that this use case is very much our bread and butter).

mbostock · 2024-01-23T02:57:13Z

examples/google-analytics/docs/index.md

+  <div class="card">
+    <h2>Rolling 28-day Active users</h2>
+    <span class="big">${summary[summary.length-1].active28d.toLocaleString("en-US")}</span>
+    <span class="small">${((v) => html`<span class="${v > 0 ? "green" : v < 0 ? "red" : ""}">${bigNumber(v)} ${v > 0 ? "↗︎" : v < 0 ? "↘︎" : ""}`)(getCompareValue(summary, 'active28d'))}</span>


I don’t see the change here. It looks like you did this for the Plot example but not the Google Analytics one. 😓

Also, I don’t think this really solves the problem I raised because someone still has to find the trend function you wrote — we need to think about how to incorporate a practical solution “in the box” (in the standard library or default CSS).

(And also I want inline expressions for attributes #32 or even better server-side rendering #234…)

examples/google-analytics/docs/index.md

mbostock · 2024-01-23T03:02:31Z

examples/plot/docs/index.md

@@ -21,6 +20,9 @@ const stars = FileAttachment("data/plot-github-stars.csv").csv({typed: true});
 const lastMonth = d3.utcDay.offset(today, -28);
 const lastWeek = d3.utcDay.offset(today, -7);
 const x = {domain: [start, today]};
+function trend(value, format) {


Suggested change

function trend(value, format) {

function trend(value, format) {

This sort of function should have a default format, or better, allow the format to be optionally specified as a function (and promote to a function when a string). If I wanted it to be reusable, I’d probably also take an options object instead of format, and then allow things like the positive/negative/zero class to be specified, as well as the positive/negative/zero suffix.

examples/plot/docs/index.md

mbostock · 2024-01-23T03:04:39Z

examples/plot/docs/index.md

+  </div>
+  <div class="card">
+    <h2>Opened issues, 28d</h2>
+    <span class="big">${d3.format(",")(d3.sum(issues, (d) => !d.pull_request && d.open >= lastMonth))}</span>


I’m not a big fan of the d3.format(",") repetition here. And I’ve been thinking that d3-format is mostly deprecated anyway in favor of number.toLocaleString.

Co-authored-by: Mike Bostock <mbostock@gmail.com>

- adopt toLocaleString

Fil · 2024-01-25T16:27:35Z

I've added a trend component (for now it is local to each example; should we adopt it as part of stdlib yet?), and simplified things a bit.

mbostock

Is it missing the small class on the trend now? Looks good otherwise.

Fil · 2024-01-25T17:34:06Z

I have not seen a difference with or without small. But I'll add it back.

mbostock · 2024-01-25T18:24:45Z

It’s because the card class already applies small. So not necessary now!

Fil · 2024-01-26T08:27:26Z

Note to myself: good to keep the small class in the trend helper, so that it can be used outside of a card.

Fil requested a review from mbostock January 17, 2024 00:26

mbostock reviewed Jan 17, 2024

View reviewed changes

examples/plot/docs/index.md Outdated Show resolved Hide resolved

mbostock reviewed Jan 17, 2024

View reviewed changes

examples/plot/docs/index.md Outdated Show resolved Hide resolved

mbostock reviewed Jan 17, 2024

View reviewed changes

examples/plot/docs/index.md Outdated Show resolved Hide resolved

mbostock reviewed Jan 17, 2024

View reviewed changes

examples/plot/docs/index.md Outdated Show resolved Hide resolved

Fil mentioned this pull request Jan 17, 2024

Dash.resize → resize #555

Merged

Fil and others added 5 commits January 16, 2024 16:57

adopt the new classes for plot's big numbers

8145753

fix padding and color, the hard way

e611f92

Update examples/plot/docs/index.md

a26fdd0

Co-authored-by: Mike Bostock <mbostock@gmail.com>

Update examples/plot/docs/index.md

54eb832

Co-authored-by: Mike Bostock <mbostock@gmail.com>

quote attributes

55cf5ad

Fil force-pushed the fil/plot-big branch from 4a96b6c to 55cf5ad Compare January 17, 2024 00:57

mbostock reviewed Jan 17, 2024

View reviewed changes

examples/plot/docs/index.md Show resolved Hide resolved

mbostock reviewed Jan 17, 2024

View reviewed changes

examples/plot/docs/index.md Outdated Show resolved Hide resolved

Fil and others added 5 commits January 16, 2024 17:06

link not muted, fix slag

acd987c

Update examples/plot/docs/index.md

b30b82a

Co-authored-by: Mike Bostock <mbostock@gmail.com>

remove redundant client-side html generation

8da625b

a bit of clean-up in the google-analytics example

6537d9d

add a gap class

d335c5a

Fil mentioned this pull request Jan 22, 2024

remove bignumber #583

Closed

Fil added 2 commits January 21, 2024 22:52

Merge branch 'main' into fil/plot-big

0b03f32

fix links

d15e6eb

Fil marked this pull request as ready for review January 22, 2024 06:58

mbostock reviewed Jan 22, 2024

View reviewed changes

Fil added 4 commits January 22, 2024 09:33

remove .gap

65fb83b

fix indentation, close div

edfacf1

trend function

27e6c01

Merge branch 'main' into fil/plot-big

bbd04c8

mbostock reviewed Jan 23, 2024

View reviewed changes

Update examples/google-analytics/docs/index.md

03920a6

Co-authored-by: Mike Bostock <mbostock@gmail.com>

Fil and others added 7 commits January 23, 2024 04:26

Update examples/google-analytics/docs/index.md

ad9164f

Co-authored-by: Mike Bostock <mbostock@gmail.com>

Update examples/google-analytics/docs/index.md

0d9c5bc

Co-authored-by: Mike Bostock <mbostock@gmail.com>

Update examples/google-analytics/docs/index.md

09d3c89

Co-authored-by: Mike Bostock <mbostock@gmail.com>

Update examples/plot/docs/index.md

5f4f6c7

Co-authored-by: Mike Bostock <mbostock@gmail.com>

Update examples/plot/docs/index.md

da25240

Co-authored-by: Mike Bostock <mbostock@gmail.com>

Update examples/plot/docs/index.md

7860c6b

Co-authored-by: Mike Bostock <mbostock@gmail.com>

- trend as a (local) component

1706dc4

- adopt toLocaleString

Merge branch 'main' into fil/plot-big

f3b3f95

mbostock approved these changes Jan 25, 2024

View reviewed changes

small

20ef431

Fil enabled auto-merge (squash) January 25, 2024 17:35

Fil merged commit a1cbe3d into main Jan 25, 2024
2 checks passed

Fil deleted the fil/plot-big branch January 25, 2024 17:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adopt the new classes for plot's big numbers #554

adopt the new classes for plot's big numbers #554

Fil commented Jan 17, 2024 •

edited

Loading

mbostock Jan 22, 2024

Fil Jan 22, 2024

mbostock Jan 23, 2024

mbostock left a comment •

edited

Loading

mbostock Jan 23, 2024

mbostock Jan 23, 2024

mbostock Jan 23, 2024

Fil commented Jan 25, 2024

mbostock left a comment

Fil commented Jan 25, 2024

mbostock commented Jan 25, 2024

Fil commented Jan 26, 2024

	function trend(value, format) {

	function trend(value, format) {

adopt the new classes for plot's big numbers #554

adopt the new classes for plot's big numbers #554

Conversation

Fil commented Jan 17, 2024 • edited Loading

mbostock Jan 22, 2024

Choose a reason for hiding this comment

Fil Jan 22, 2024

Choose a reason for hiding this comment

mbostock Jan 23, 2024

Choose a reason for hiding this comment

mbostock left a comment • edited Loading

Choose a reason for hiding this comment

mbostock Jan 23, 2024

Choose a reason for hiding this comment

mbostock Jan 23, 2024

Choose a reason for hiding this comment

mbostock Jan 23, 2024

Choose a reason for hiding this comment

Fil commented Jan 25, 2024

mbostock left a comment

Choose a reason for hiding this comment

Fil commented Jan 25, 2024

mbostock commented Jan 25, 2024

Fil commented Jan 26, 2024

Fil commented Jan 17, 2024 •

edited

Loading

mbostock left a comment •

edited

Loading