Add the ability to round bar index scale

[ericsoco]

I noticed that ResponsiveBar charts with many, narrow bars were varying in width. The svg container was the correct width, but the bars within it were offset from the left/right edges.

Screencap:

After some digging, I narrowed it down to the use of d3-scale.scaleBand.rangeRound() within Bar's common.js::getIndexedScale(). This diff puts this nicing behavior behind a nice prop, which defaults to true to maintain current functionality unless a developer opts out with nice=false.

The above test with nice=false:

I didn't add anything to the Bar demo site; I can if you like, but this prop feels niche / edge-case enough that it may not need to be surfaced in the demo site.

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 785cec5:

Sandbox	Source
nivo	Configuration
nivo-website	Configuration

[plouc]

@ericsoco seems interesting to have the option, however I'm not sure about the naming, having nice as a prop on the top level component is hard to figure out I think, maybe something like roundIndexScale? This should also be added to the TypeScript definitions, ideally I'd like to have everything documented, but the way the website works is not documented, so I can definitely understand if you don't want to :) It would also be nice to add a unit test for it.

[wyze]

I agree about the prop name. Something like indexScale={{ nice?: boolean, round?: boolean }} to be more flexible to accept future options if needed.

[ericsoco]

@plouc -- I can look into added to TS defs and a unit test; can also take a look at adding to the website if it's not too complex!

Also, I realize now that nice is slightly different semantically than rangeRound (docs), so agreed on naming; can take @wyze's approach and refactor to indexScale={round}. I'll leave out "nice" for now, can add in the future if useful.

[plouc]

Thank you @ericsoco! Yes @wyze approach is more flexible and align with the valueScale we already have.

[plouc]

@ericsoco Thank you for the adjustments, maybe what we suggested was confusing, the property should be indexScale, to be consistent with valueScale (we cannot really use x/y scale for names as the chart supports different layouts), but the scale type is a band scale.

[ericsoco]

No worries, I can make these changes now (and fix the failing lint).

[wyze]

Also need to update the canvas implementation to pass the value through to the generate bars methods or it will crash. I made the mistake with valueScale :O.

nivo/packages/bar/src/BarCanvas.js

Line 58 in 484235f

valueScale,

[ericsoco]

[wyze]

Code looks really good! https://dlqum.sse.codesandbox.io/bar/

Only minor change is I think we should change the website values to default to the default values of the component.

[ericsoco]

@wyze oof good catch! Fixed.

[wyze]

Looks great! Thanks. https://z5c1z.sse.codesandbox.io/bar/

[plouc]

LGTM

[plouc]

Thank you @ericsoco!

[ericsoco]

Thank you @plouc for the lovely library, and @wyze as well 😄

[jamesdh]

@ericsoco I'm looking back at this issue and I see your bar chart uses time series data for its X-axis but I don't see any way to do so without converting their value to a string (which causes every bar to have a tick). Can any one of you guys clarify how that was done?

[ericsoco]

@jamesdh I have string values for x-axis formatted as YYYY-MM, and I'm passing xFormat: 'time:%Y-%m' to ResponsiveBar.

I don't have a tick on every bar because I'm not using Nivo to render the x-axis labels / ticks, this is a hybrid component that functions as a slider (using Material-UI) with a bar chart overlaid.

I think what I just told you is probably not helpful for your use case...sorry!