palantir / plottable

:bar_chart: A library of modular chart components built on D3
http://plottablejs.org/
MIT License
2.96k stars 224 forks source link

Don't short-circuit `_includedValuesProvider` for unanchored components #3408

Closed themadcreator closed 6 years ago

themadcreator commented 6 years ago

The lines that were removed prevented _includedValuesProvider from working when a component was not yet anchored. This may be a minor performance optimization but it is not necessary.

It prevents extents calculations from working. Furthermore, the calculations in this method depend only on data space, not pixel space, so anchoring is irrelevant.

blueprint-bot commented 6 years ago

Only ignore anchored state when asking for transformationdomain

Demo: quicktests | fiddle

hellochar commented 6 years ago

Why didn't the first commit work? Can we fix the unit tests instead of introducing this ignoreAttachState arg (which feels a bit hacky)?