Page MenuHomePhabricator

Roughly support stacked area charts
ClosedPublic

Authored by epriestley on May 3 2019, 10:44 PM.

Details

Summary

Ref T13279. This adds support for:

  • Datasets can have types, like "stacked area".
  • Datasets can have multiple functions.
  • Charts can store dataset types and datasets with multiple functions.
  • Adds a "stacked area" dataset.
  • Makes D3 actually draw a stacked area chart.

Lots of rough edges here still, but the result looks slightly more like it's supposed to look.

D3 can do some of this logic itself, like adding up the area stacks on top of one another with d3.stack(). I'm doing it in PHP instead because I think it's a bit easier to debug, and it gives us more options for things like caching or "export to CSV" or "export to API" or rendering a data table under the chart or whatever.

Test Plan

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.May 3 2019, 10:44 PM
epriestley requested review of this revision.May 3 2019, 10:46 PM
amckinley accepted this revision.May 15 2019, 2:03 PM
amckinley added inline comments.
src/applications/fact/chart/PhabricatorChartStackedAreaDataset.php
57

foreach ($missing as $x => $_)?

58–59

This looks like a typo for "before which 'x' is defined". Maybe:

Move the cursor forward to find the point immediately to the left of 'x'.

58–62

I feel like we're going to be tracking down edge cases in a lot of this code for a long time. Did you use the D3 implementation as a reference, or was this just on the fly? Nothing looks like it's obviously going to break, but I wonder if we're doing a lot of work upfront without knowing the limits of the way D3 does things.

(Clearly the right answer is to use PHP to invoke Node.js server-side to evaluate D3's implementation).

64–68

I would change this ordering to match the evaluation order below: "if to the left", "if between", "if to the right".

webroot/rsrc/js/application/fact/Chart.js
55–63

I'm just kinda assuming all these API changes are consistent with the New Hotness™.

This revision is now accepted and ready to land.May 15 2019, 2:03 PM
This revision was automatically updated to reflect the committed changes.