Skip to content

Comments

[Bug 865378] Add support for multiple Y axises.#1333

Closed
mythmon wants to merge 5 commits intomozilla:masterfrom
mythmon:k-graph-y-axis-865378
Closed

[Bug 865378] Add support for multiple Y axises.#1333
mythmon wants to merge 5 commits intomozilla:masterfrom
mythmon:k-graph-y-axis-865378

Conversation

@mythmon
Copy link
Contributor

@mythmon mythmon commented May 3, 2013

This uses "axis groups". Each axis group's maximum is calculated, and then the entire group is scaled so that the maximum is 1.0. Then the axises and the hover tool tips are adjusted to show the correct numbers. The scaling is adjusted as the zoom is changed and as axises are toggled on and off. I updated the question stats graph to make use of these features

  • initData and makeSeries modified to calculate scales for each axis group, and use them when drawing.
  • Wrote a custom y axis class that makes room for itself around the graph, inserts itself, and supports being added multiple times. It also sets its scale based on a parameter instead of of data (since the data is just from 0 to 1 now).
  • Added a new Hover class, making it respect the scale of series as well, so that the hover tool tips show the original values. I also made the BarHover class an extension of this, for DRYness.

r?

This is kind of hacky, and the more I do this, the more I want to just use d3 directly.

mythmon added 3 commits May 2, 2013 16:31
This uses "axis groups". Each axis group's maximum is calculated, and
then the entire group is scaled so that the maximum is 1.0. Then the
axises and the hover tool tips are adjusted to show the correct numbers.
The scaling is adjusted as the zoom is changed and as acises are toggled
on and off.

This is kind of hacky, and the more I do this, the more I want to just
use d3 directly.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Divs like this are automatically added now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at some point I thought you had all these divs added from js?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not all of them, just some. I tried to do all of them, but ran into some issues and decided not to spend more time on it.

@mythmon
Copy link
Contributor Author

mythmon commented May 3, 2013

Oh, and now you can turn multiple sets on and off, with the assumption that they are different axis groups. ie: They are checkboxes instead of radio buttons.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this sets the max of the axis group to the max of all the series' maxes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I'll add a comment, but can you think of a cleaner way to do this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use _.max. I think something like:

_.max(this.data.series, function(series){ return series.max; });

That should replace this entire for loop?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite, since this is grouping by the name of the axisGroup. Leaving it for now.

@rlr
Copy link
Contributor

rlr commented May 3, 2013

This is intense! I can't see it in action yet, can I?

@mythmon
Copy link
Contributor Author

mythmon commented May 3, 2013

You can see it in action! Check out the history charts, like at /kb/private-browsing-browse-web-without-saving-info/history. The percents and the votes use a different scale.

@mythmon
Copy link
Contributor Author

mythmon commented May 3, 2013

Alphabetized CSS based on feedback ^

@rlr
Copy link
Contributor

rlr commented May 3, 2013

yay it worked! let's :shipit: and move to the next one. r+

@mythmon
Copy link
Contributor Author

mythmon commented May 3, 2013

8da5686

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants