search

interactivethings / d3-grid

D3 grid layout
BSD 3-Clause "New" or "Revised" License
77 stars 11 forks source link

Upgrade to d3v4 #9

Open basilesimon opened 7 years ago

basilesimon commented 7 years ago

This pull request upgrade d3-grid code to d3v4.

This is still a work in progress and should not be merged yet

Goals

Upgrade to d3v4 : e2714ad

x = d3.scale.ordinal(), becomes x = d3.scaleOrdinal(),

x.domain(d3.range(_cols)).rangeBands become

var x = d3.scaleBand();
 x.domain(d3.range(_cols)).range([0, size[0]], padding[0], 0);

x.domain(d3.range(_cols)).rangePoints becomes

var x = d3.scalePoint();
x.domain(d3.range(_cols)).range([0, size[0]]);

Trim down the dependencies to modular d3: 838c964

The plugin now only loads d3-scale, instead of the whole of d3.

  "dependencies": {
    "d3-scale": "^1.0.6"
  },

Adopt d3v4 plugin structure: f3015dd

d3-grid.js has been moved to a src/ directory. See guidance on publishing a d3 plugin

gka commented 7 years ago

:+1:

basilesimon commented 7 years ago

Thanks @gka - we're waiting on @herrstucki to merge this in! That said, I've used it successfully in prod for the Times general election results page: https://www.thetimes.co.uk/electionresults (resize to < 768px across)

jstcki commented 7 years ago

@basilesimon not sure if you're still working on it but the package.json was left broken in your last commit and the dependencies are missing. Please make sure the tests actually pass and the build process works before you push 😉 Thanks in advance! 🙇

P.S.: I fixed package.json, so that it's at least parseable again P.P.S.: I've also enabled Travis, so we get test feedback here as well.

basilesimon commented 7 years ago

@herrstucki Ah, this is about as far as I'm going to go. Some tests are still soft-failing (not being return what's expected)... I'd really welcome your help on these at this point.

The build and packaging have been fixed and now working nicely 💯 😀

oller commented 6 years ago

How's this looking guys? Hit a stumbling block on an v3 to v4 upgrade with this handy dependency!

basilesimon commented 6 years ago

@oller The code in this PR is in use in production on my side. I just can't quite get it to pass the tests, hence the PR being still open... I recommend you try it out and give me a shout if you run into trouble, I'll do my best to help 👌

oller commented 6 years ago

Ah great, I'll give it a whirl. Thanks for the prompt response @basilesimon! 👍

oller commented 6 years ago

How are you importing and referencing this lib now @basilesimon, as d3 v4 has done away with d3.layout.grid which this plugin used to extend.

I can see the plugin present in node_modules with the PR code. I've tried both...

import d3grid from "d3-grid"; import d3grid from "d3-grid/index.js";

Both return undefined, the only way I can get some sense back is:

import d3grid from "d3-grid/src/d3-grid.js" but then that errors citing the d3 referenced within is not defined.

I've tried using both yarn and npm to install this PR in their respective formats. I'm on webpack 3.4

Any insights appreciated.

basilesimon commented 6 years ago

@oller that takes a bit of fiddling.

See this folder for example: I've adapted d3-grid.js inside layout.js to make it easier to import, although as you can see there's no Webpack in there...

basilesimon commented 5 years ago

Hey @herrstucki, sorry to bother you again, but I think I need some help to get this over the line.

It would be fantastic to have this plugin properly available. At the moment this is something I personally use all the time, but the implementation described in my above comment is pretty gross.

Also, we now have this fork with a confusing name (cc @finnfiddle).

Maybe it would be simpler if we had one, up-to-date version of this plugin we can all contribute to?