Enable pan/zoom on categorical scales #3137

Merged
merged 4 commits into from Jan 5, 2017

Projects

None yet

3 participants

@themadcreator
Collaborator

Introduces new interface TransformableScale. This interface abstracts the
necessary API for implementing pan/zoom on various types of scales.

Due to some limitations of d3, for example category scales can't invert screen
space coordinates, we nevertheless allow the scale types to support pan/zoom if
they implement this interface.

Implemented TransformableScale interface on Linear, Log, Time and
Category Scales. The three Quantitative scales have fairly trivial
implementations, but the Category scale requires special handling.

Added an intermediate coordinate space called the Transformation Space to the
Category scale. The conversion from data to screen coordinates is as follows:

Data Space -> _d3Scale -> Transformation Space -> _d3TransformationScale -> Screen Space.

The Transformation Space coordinates are initialized to [0, 1].

Notably, range band calculations are internally computed in Transformation
Space
and transformed to screen space in methods like rangeBand() and
stepWidth().

@themadcreator themadcreator Enable pan/zoom on categorical scales
Introduces new interface `TransformableScale`. This interface abstracts the
necessary API for implementing pan/zoom on various types of scales.

Due to some limitations of d3, for example category scales can't invert screen
space coordinates, we nevertheless allow the scale types to support pan/zoom if
they implement this interface.

Implemented `TransformableScale` interface on `Linear`, `Log`, `Time` and
`Category` Scales. The three `Quantitative` scales have fairly trivial
implementations, but the `Category` scale requires special handling.

Added an intermediate coordinate space called the *Transformation Space* to the
`Category` scale. The conversion from data to screen coordinates is as follows:

Data Space -> _d3Scale -> Transformation Space -> _d3TransformationScale -> Screen Space.

The *Transformation Space* coordinates are initialized to [0, 1].

Notably, range band calculations are internally computed in *Transformation
Space* and transformed to screen space in methods like `rangeBand()` and
`stepWidth()`.
3ebf239
@themadcreator
Collaborator

lol
image

themadcreator added some commits Dec 13, 2016
@themadcreator themadcreator lol unit test
a663a94
@themadcreator themadcreator Hide category axis labels that overflow
Now that category axes can be pan/zoomed, the
labels and ticks can overflow the bounds of
the axis.

This change pulls out the code used in numericAxis
into the base axis class and then is re-used in
categoryAxis.
eda627b
@hellochar

Looks good in general; mainly wondering about the hiding tick marks possible regression

+ this._showAllTickMarks();
+ this._showAllTickLabels();
+ this._hideOverflowingTickLabels();
+ this._hideTickMarksWithoutLabel();
@hellochar
hellochar Jan 5, 2017 Collaborator

Sometimes if the space for a tick label is too small, it won't get drawn: https://jsfiddle.net/hellochar/t8j5d7vt/ Will these "non-labelled tick-marks" get removed now?

@themadcreator
themadcreator Jan 5, 2017 Collaborator

This won't be an issue because those labels aren't hidden with the CSS visibility rule. Instead the SVGTypewriter just doesn't draw the text because there isn't enough space. But the label is still technically "visible".

+ }
+
+ public getTransformationDomain() {
+ return this.domain() as [number, number];
@hellochar
hellochar Jan 5, 2017 Collaborator

nit: might make sense to have this.domain() return a [number, number] type directly

@themadcreator
themadcreator Jan 5, 2017 edited Collaborator

Scale's this.domain returns D[] where D is the templated domain type. It should probably return [D, D], but I don't want to redo all the other instances.

@@ -256,6 +257,24 @@ export class QuantitativeScale<D> extends Scale<D, number> {
}
}
+ public magnify(magnifyAmount: number, centerValue: number) {
@hellochar
hellochar Jan 5, 2017 Collaborator

Would it make sense for the Plottable.Scale class to declare abstract methods for magnify/translate/scaleTransformation/getTransformationDomain ? And then make quantitativeScale abstract, to pass down scaleTransformation and getTransformationDomain?

@CalvinFernandez
CalvinFernandez Jan 5, 2017 Collaborator

yes also wondering this

@themadcreator
themadcreator Jan 5, 2017 edited Collaborator

Not all Plottable.Scales are transformable (e.g. color scales). So, I added the TransformableScale interface to the particular scales that implement it. Quantitative scales implements this interface and are essentially abstract. However, Plottable doesn't use true abstract types, but instead implements error-throwing methods, so i followed that pattern.

+ return this._d3TransformationScale(untransformed);
+ }
+
+ public magnify(magnifyAmount: number, centerValue: number) {
@hellochar
hellochar Jan 5, 2017 Collaborator

Is this the "single source of truth" for panning/zooming a scale? If so, PanZoomInteraction should use these

@themadcreator
themadcreator Jan 5, 2017 Collaborator

Yeah, it is and PanZoomInteraction does use these.

@hellochar

Tested this branch on my jsfiddle and it was fine; looks good!

src/interactions/panZoomInteraction.ts
+ * runtime interface typechecking, so we have to explicitly list all classes
+ * that implement the interface.
+ */
+ export function isTransformable(scale: any): scale is TransformableScale {
@hellochar
hellochar Jan 5, 2017 Collaborator

Do we use this method anywhere?

@themadcreator
themadcreator Jan 5, 2017 Collaborator

We need it in the internal library to detect which scales can be pan/zoomed.

src/interactions/panZoomInteraction.ts
+ * pan/zoom if they implement this interface. See `Category`'s
+ * `_d3TransformationScale` for more info.
+ */
+ export interface TransformableScale {
@hellochar
hellochar Jan 5, 2017 Collaborator

Ohhh my god Github hid this whole file from me... Okay this makes a lot more sense. Imo this interface belongs in Plottable.Scales instead of Plottable.Interactions (and you can type TransformableScale = Plottable.Scales.TransformableScale in this file to prevent needing to type it out everywhere)

@themadcreator
themadcreator Jan 5, 2017 Collaborator

Sure, I'll move it.

@themadcreator themadcreator Move TransformableScale interface to Plottable.Scales
96aa826
@themadcreator themadcreator merged commit 6150006 into develop Jan 5, 2017

3 checks passed

cla/palantir CLA signed via membership in "palantir"
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@themadcreator themadcreator deleted the bd/pan-zoom-any-scale branch Jan 5, 2017
@hellochar hellochar modified the milestone: v2.6.0 Jan 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment