dringtech / ifg-charts

Visualisation component library for IfG
MIT License
1 stars 1 forks source link

Fixing start/endpoints of chart #9

Open philipnye opened 7 months ago

philipnye commented 7 months ago

Currently the chart can be drawn from a point before the start date supplied to the chart/to a point after the end date supplied to the chart. This is done to ensure 'sensible' gridlines/axis labels are used but causes two issues:

timeline (6)

timeline (7) - top
philipnye commented 7 months ago

I wonder - is it possible to use D3 to choose the gridlines and axis labels, but then revert to using the original start and end dates supplied to the chart? It's already the case that there isn't always a gridline at the point where the chart starts and ends. Would doing this introduce other problems?

gilesdring commented 7 months ago

There is a very simple fix to this: remove the call to the nice() method of the xScale. If you're happy that the bar start / end are provided by either the first start / last end of the timeline or explicitly provided I can make this fix.

philipnye commented 7 months ago

@gilesdring Yes, the scripts that I've written will always return both a (finite) start/end date for all bars and a start/end date for the chart as a whole. Just to check we're on the same page - this won't affect the choice of gridlines? I.e. Gridlines would be October (on 1 October) rather than 15 October, say?

gilesdring commented 7 months ago

It shouldn't do - all that will happen is that the range won't be rounded to a nice value - so if the earliest timestamp was 2:23 am on 15th Oct (for some reason) that's where the chart would start, unless manually overridden. I think the ticks will still fall on sensible points. Can run some testing to validate this assumption.

gilesdring commented 6 months ago

Confirmed, and removed in code. Will be in next release.

philipnye commented 4 months ago

@gilesdring We've tested this and it's looking good, thanks - definitely preferable to what we had before. We wonder if a further refinement is possible:

I could probably find the budget for another hour or two if you think a fix would be doable in that time. If it's a more complicated ask, I think we'd live with what we have.

gilesdring commented 2 months ago

@philipnye thinking about this in generic terms, as it would need to be based on flags passed to the library, which has no knowledge of what it is presenting (!), would something like this work...

round: none | start | end | both

Let me know if this sounds workable. If so, it's a change to this codebase then further integration with Drupal, including setting the new property, which would likely need @graham73may to make some changes (I've no real insight to the database integration)

gilesdring commented 2 months ago

Hm. On second thoughts there, and having investigated, there does not seem to be a nice which only works on start or end. More digging required. I think the interface would have to be something like described.

philipnye commented 2 months ago

@gilesdring Thanks for giving this some thought. Sounds like there not be a particularly quick solution out there.

Given the limited time we have available + the fact it'd need some work on the Drupal side too, I suggest we park this - the current solution works very well, and this was just striving for perfection. Shout if you think I'm being too pessimistic on timing though.

cc @graham73may