Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

d.y0 always reset to 0 #333

Closed
dwt opened this issue Oct 16, 2013 · 5 comments
Closed

d.y0 always reset to 0 #333

dwt opened this issue Oct 16, 2013 · 5 comments

Comments

@dwt
Copy link
Contributor

dwt commented Oct 16, 2013

I just discovered that the first renderer that is registered will trigger the creation of a Rickshaw.Graph.Unstacker on the graph, which will then unconditinally reset d.y0 on all the series to zero.

As there seems to be no possibility to stop this, even if you specify unstack: true I guess that this is a bug and a leftover from some older time.

@dwt
Copy link
Contributor Author

dwt commented Oct 16, 2013

This looks especially suspicious as it happens when the renderer is initialized, which happens before it is registered, so the renderer does not even need to be used to trigger this behaviour.

Offending file: https://github.com/shutterstock/rickshaw/blob/master/src/js/Rickshaw.Graph.Unstacker.js
Offending point where this is triggered: https://github.com/shutterstock/rickshaw/blob/master/src/js/Rickshaw.Graph.js#L46

@dchester
Copy link
Contributor

As there seems to be no possibility to stop this, even if you specify unstack: true I guess that this is a bug and a leftover from some older time.

It's true the unstacker is instantiated with the first renderer, but y0 properties aren't actually set then. It's only in the case where you specify unstack: true that y0 properties get set to zero later on.

I agree the implementation is a little bit roundabout, but is there any scenario where the behavior is actually incorrect?

@dwt
Copy link
Contributor Author

dwt commented Oct 28, 2013

Thats precisely the problem. If I specify unstack: true but actually have y0 values in the data, they get overwritten.

That is, only a a renderer that needs this data set to 0 should actually set this for it's data series, and I think it should really do that in the accessor function it is using to get the data into d3 and not change the actual data.

@dchester
Copy link
Contributor

I think I see what you're after -- sounds like you want to do your own stacking. As it is now, unstack: true will not only not stack, but it will also proactively _un_stack, thereby wiping out your previous custom stacking. This is indeed mostly an artifact from a prior time when things were different, but there are some other bits that rely on y0 at least being defined. I'll take a shot at addressing those places, and then I think we can get rid of Rickshaw.Graph.Unstacker altogether.

@dchester dchester reopened this Oct 30, 2013
@dwt
Copy link
Contributor Author

dwt commented Oct 31, 2013

Close to that, we actually did not try to do our own stacking, but instead wanted to draw a region that did not start at the x-axis. :)

Oh, and thanks! :-)

bohan added a commit to bohan/rickshaw that referenced this issue Aug 2, 2022
* Makes hover detail arrow rendering more reliable

Uses the 'CSS triangle' trick to render the left and right arrows on the
hover detail item, rather than using a glyph (whose rendering might
change depending on the font used).

The technique is described here:

http://appendto.com/blog/2013/03/pure-css-triangles-explained/

* Improves hover detail placement when space is tight

The layout error for hover detail elements is now checked for both
left and right alignment.  Then the alignment with the least error is
used.

* Add test case for Legend

* Make Rickshaw.Graph.Legend a real class

This makes it possible to extend the legend to add custom styling
or interactivity. This commit should not change behavior or options.

* Allow Legend subclasses to override the className

* Extract legend rendering into render function that can be called multiple times

* Return rendered line from addLine

* tweak area renderer to play nicely with multi renderer; fixes shutterstock#335; closes shutterstock#336

* update built libs

* Update README.md

* excise Rickshaw.Graph.Unstacker and allow for consumers to pre-stack if they like; fixes shutterstock#333

* Add series configured class name to all scatterplot points

* throw an error if provided element isn't an element

* Added Rickshaw.Graph.Minimap class. The minimap will render exactly what the main graph renders, but will not pay attention to its "window" extents and  thus will always show the whole data set.

* refactor Minimap and rename to Rickshaw.Graph.RangeSlider.Preview; closes shutterstock#273

* add data attributes on axis ticks; closes shutterstock#347

* Update README.md

* tell jshint to warn about leaking globals

* be smarter about rounding months and years; closes shutterstock#344

* Update README.md

* allow specifying custom x and y scales; closes shutterstock#346

* Respect series classes in legend renderer (to allow the color of the swatch to be controlled by css

* invoke stroke factory in renderer parent class; closes shutterstock#337; closes shutterstock#338

* update built libs

* support sending slide callbacks for range sliders; fixes shutterstock#363

* render annotations after adding each one

* make sure to initialize the preview graphs' renderers to match parents for range slider previews; closes shutterstock#364

* fire range slider events with extents in tact

* Rickshaw.Graph.RangeSlider.Preview supports millisecond zooming

Changed the domainScale interpolate function to d3.interpolateNumber
(vs. d3.interpolateRound) so that the RangeSlider Preview Window could
zoom into millisecond levels of precision (before it would only allow a
window of 1 second)

* Fix shutterstock#377: Ensure multi renderer consults it's sub renderers about how they perceive their domain

* Configure range slider from graph, so that width changes affect the slider width too.

* Fix typo

* Ensure Graph creates a copy of the x/yScale before modifying it

x/yScale is coming from the configuration dictionary which may be
referenced by the Graph creator, or shared with other Graphs. We need to
ensure we copy the scale so that our mutations do not change the object
given to us.

* Prevent slider from cutting of series.

This fixes the following bug:

I have 4 series,

[
  [{0, 1}, {1, 1}],
  [{1, 1}, {2, 1}],
  [{2, 1}, {3, 1}],
  [{3, 1}, {4, 1}]
]

I use a slider to select the x-range (2.5, 4)

This means stackedData ends up being

[ [], [], [], [[3,4], [1, 1]] ]

At this point, reading stackedData[0][0] produces undefined, even though
there is data that belongs to the range.

* Recentered detail dot. explicitly set box-sizing.

* refined margin-top

* make jQuery dependent bits compatible with jQuery.noConflict(); fixes shutterstock#396

* Rescope stroke style attribute on y-axis

In mbostock/d3 issue #1016, the scope of the `tick` class on a svg y-axis
element changed to include the entire g object (line and text), not just the
line. The Rickshaw CSS for `.y_ticks .tick` was created in the early commits, and
sets the style to be a 2px width semitransparent stroke.

Due to the change in scoping of the style class, the stroke element applies to
both the line and the text element, creating a shadow on the y-axis text labels.

By changing the scope of the stroke stanza from `.y_ticks .tick` to `.y_ticks
.tick line`, the stroke now applies only to the line, and not to the text
element, this removing the stroke shadow on the text label.

* Extend Range Slider to accept multiple graphs

Extend to allow array of graphs, and handle logic based on if the graph element
is singular or plural.

Also add examples/shared_slider.html as a demo of new functionality

Similar to pullr equest shutterstock#169 (bozdoz)

* Make jQuery known as the global symbol instead of $

To make it actually compile again.

* Define $ variable in correct scope

* Allow for developers to customise hook into jQUI slider events

* Switch back to tabs

* Add start and end scripts to allow use of UMD by Rickshaw

* Allow empty data arrays in series.

* Find correct domain even if the first series in stack is empty.

This can easily happen when series of different length are drawn and the RangeSlider is used to restrict the graph to a range where one of the series is empty.

* Removed trailing comma in example

* provide a supported interface for toggling stacking

* support resizing preview range sliders; closes shutterstock#375; closes shutterstock#417; closes shutterstock#382

* add x magnitude scale and improve barWidth calculation for bars renderer

* use preview range slider in multi renderer example

* refactor lineplot to work with multi renderer; closes shutterstock#413; closes shutterstock#414

* update built libs

* Update README.md

* Add README.md note about $super and minification; closes shutterstock#52

* Only update graph once when toggling a series, not once per series.

* Remove duplicate graph update in smoother.

* Update copyright year

Fix outdated copyright year (update to 2014)
The copyright year was out of date. Copyright notices must reflect the current year. This commit updates the listed year to 2014.

see: http://www.copyright.gov/circs/circ01.pdf for more info

* Better documentation around stacked and unstacked rendering.

* Update example_04.html

* Added method to remove HoverDetail's mouse listeners.

* Show units when displaying minutes.

When time is being displayed in minutes there is no indication of units.

* Update README.md

Per the actual source, one can pass an int to color() and select the nth color from a palette. I simply added this fact to the documentation.

* transform either stroke or color properties on highlight

* override which key to fetch legend swatch color from

* Remove error if no jQuery.

Uncaught ReferenceError: jQuery is not defined at line rickshaw.js:1985(anonymous function)

* Add default value for `renderer`

As much as I understand, `renderer` defaults to `line`. Adding that in the README

* Add homepage, repository and issues links in package.json.

* update built libs

* fixing local time ticks

* Fix typo

* Make calculating interval in bar chart consistent. Fixes shutterstock#461

Sorting object's keys returned to guarantee consistency when iterating over. Keys order in `for .. in` loop is not specified and browsers behave differently here. This results with different invterval value being calculated for different browsers. See last but one section here: http://www.ecma-international.org/ecma-262/5.1/#sec-12.6.4.

* HoverDetail should default to being inactive.

* Add Rickshaw.Graph.Dragzoom

* center x_axis on bar chart

* Rename with .append at the end

* Rename with .prepend at the end

* use prepend/append files for UMD support

prevents downstream issues from cropping up due to invalid JavaScript files.

* RangeSlider: Resize when original graph changes width

* bower ready, with d3 dependency

* fix for 29 Feb

* fixing license warning

* added travis

* upgrade dependancies

* requiring minimally node4 to run tests

jsdom works either with node <0.12 or with node >4 so this means you can install the package at node <4
but you wont be able to run the tests

* add coverage

* Multi-chart slider and colors for individual bars

RangeSlider now works with multiple charts and a single slider, by using an array with the graph variables:

var slider_three = new Rickshaw.Graph.RangeSlider({
    element: document.querySelector('#slider-range'),
    graph: [graph_one, graph_two, graph_three]
});

The bar chart renderer now looks for a proposed color for each individual bar, it will default to the chart-specific color if no color is declared:

var data = [{
  data: [{ 
    x: -1893456000, y: 29389330}, { 
    x: -1577923200, y: 33125803}, { 
    x: -1262304000, y: 37857633, color:'#06f'}, { 
    x: -946771200, y: 41665901, color:'#0cf'}
  ],
    color: 'shutterstock#222'
  }]

* created an example with multiple graphs

* multiple graphs with RangeSlider works

* revert RangeSlider now works with multiple charts and a single slider, by using an array with the graph variables:

632d1fb

* formatting and adding back $ to be able to build

* building css

* Version 1.6.0-rc.0

* Correct check for undefined variable

The code was comparing the result of `typeof` with a variable named `undefined`.

As typeof returns a string it should compare to `'undefined'`

* added example bar chart

* keeping the shared_slider example

* Running `make watchjs` watches src folder

* using npm run watch instead of makefile

* added lint to package.json

* consolodating code to avoid duplication

* Version 1.6.0

* upgraded dependancies
* support for node 4
* support for npm run watch while developing
* RangeSlider can accept multiple graphs

* fixed stacked area chart hover detail bug in firefox

* Fixes issue with negative values between 0 and -1

Probably due to a typo, numbers between 0 and -1 were not formatted as expected.

* Create a locally scoped copy of the loop-index var to prevent accessing out-of-bound index in the onConfigure callback

* added tests for hover detail

* added test for HoverDetail _removeListeners for shutterstock#477

* added tests for palette

* added test for number formatting

* added tests for annotate

* updated coveralls badge link

* Since JS doesn't have block scopes, only function scopes. So, in order to capture
a loop-index variable for the later `onConfigure` function callback, we create an
IIFE

* added test for onConfigure for shutterstock#568 shutterstock#579 shutterstock#306 shutterstock#324 shutterstock#320

added resize in the series example to manually demo onConfigure

* travis relies on built files

* added test for DragZoom initialize error for shutterstock#512

and updated test to use jsdom and lint

* including DragZoom in build

* building if needed before running tests

the tests run on the built code, rather than source code.
building if needed helps ensure the test is running against changes and
so that travis can run on prs

* added tests for mouse events on dragzoom

* added example for dragzoom

* added Rickshaw.version fixes shutterstock#538 fixes shutterstock#224

* running make after version to have updated version in dist

* 1.6.1

* based on series graph

* angular example fixes shutterstock#374 and for shutterstock#477

* example of updating data via angular controller

* instead of deap watch overwriting series data

and adding method setSeries to update data fixes shutterstock#407

* linking to angular-rickshaw directive

* Add ToC, Install section to README

This pull request adds a Table of Contents using `doctoc`, adds an npm-script to easily run it, and adds an install section to the README that more accurately reflects how to use the package. I also moved dependencies up to the Install section, as this is probably where it belongs.

* Edit CONTRIBUTING

Right now, the Contributing file has nothing about READMEs. While shutterstock#588 has been opened to talk about labels, I thought it would be good to have something in the CONTRIBUTING file about labels and issues, in general. This adds that, and should not be too controversial. In particular, I want to field possible labels from those submitting issues. This should cut down on internal time making labels that do not make sense.

* Adding link

* Adding the Contributor Covenant Code of Conduct

* Update License year

Closes shutterstock#589

* Commiting package.lock file

* 1.6.2

* update bower spec for build tools:
* specify js file extension for main property
* pull in rickshaw.css

* 1.6.3

* added test for setSeries

* chore: updated fields in the package.json

        - Check that nothing drastic happened in the `package.json`.
- We've added " JavaScript toolkit for creating interactive real-time graphs" as the description in the `package.json`. We got this from the GitHub repo description.
- Check the `package.json` keywords. We added these from your GitHub topics: "charts", "d3", "graph", "javascript", "rickshaw", "svg".
- Check that the bugs field in the package.json is OK. It doesn't match what we'd expect, which would be https://github.com/shutterstock/rickshaw/issues
- We expected the repository url in the `package.json` to be https://github.com/shutterstock/rickshaw, and it wasn't. Is this intentional?
- If there are more contributors, add them to the Contributors field in the `package.json`.

* fixes shutterstock#604 seperate build into minification and development

* build before test, fail PR on dist change

* Update Table of Contents

* 1.6.4

* Fix positioning of hoverdetail in chrome when zooming

This commit changes the calculation of the current mouse position to use
`MouseEvent.clientX/Y` (mouse position in relation to the viewport) and
`Element.getBoundingClientRect()` (element position in relation to the
viewport).

`event.layerX/Y` is non-standard and seems to return different values
than we are expecting if there is zooming involved in Chrome.

Demo, zoom in and hover the square:
https://jsfiddle.net/6gnpL75j/

An earlier commit reverted the order of layer vs offset and fixed a
positioning bug in Firefox while introducing this new one in Chrome:
shutterstock@c81f037
I looked around for that Firefox positioning bug in v1.6.0 which I found
in: http://code.shutterstock.com/rickshaw/examples/extensions.html
With this PR, both Chrome and Firefox show labels in the correct spot.

More information:
https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/clientX
https://developer.mozilla.org/en-US/docs/Web/API/Element/getBoundingClientRect
https://developer.mozilla.org/en-US/docs/Web/API/UIEvent/layerX

Chromium issue:
https://bugs.chromium.org/p/chromium/issues/detail?id=323518

* wrap style access to prevent unnecessary reflow

* whitespace

* 1.6.5

* fix: check undefined for series opacity

* test: add test for opacity settings for scatterplot renderer

* Update links

* 1.6.6

* not including package-lock since all deps are dev deps

* Add auto height and width to detail.css

* Bump express from 3.3.5 to 4.17.1 in /examples/socket.io

Bumps [express](https://github.com/expressjs/express) from 3.3.5 to 4.17.1.
- [Release notes](https://github.com/expressjs/express/releases)
- [Changelog](https://github.com/expressjs/express/blob/master/History.md)
- [Commits](expressjs/express@3.3.5...4.17.1)

Signed-off-by: dependabot[bot] <support@github.com>

* Bump clean-css from 3.4.28 to 4.2.1

Bumps [clean-css](https://github.com/jakubpawlowicz/clean-css) from 3.4.28 to 4.2.1.
- [Release notes](https://github.com/jakubpawlowicz/clean-css/releases)
- [Changelog](https://github.com/jakubpawlowicz/clean-css/blob/master/History.md)
- [Commits](clean-css/clean-css@v3.4.28...v4.2.1)

Signed-off-by: dependabot[bot] <support@github.com>

* add editorconfig

* clean-css was split into clean-css-cli

* add example of highlight transform for shutterstock#489

* Update Rickshaw.Graph.Axis.Y.js

This change allows the user to select the axis color when creating the axis.
It is useful when creating axis dynamically (adding series with different scales on run time).

* correct formatBase1024KMGTP (shutterstock#620)

* correct formatBase1024KMGTP and support negatives values

fixes shutterstock#601

* fix indentation

* 1.7.0

* check-in screenshots

* npm ignore docs tests etc

* 1.7.1

* remove local copy of vendor/d3 (shutterstock#613)

Removed the local git syncd copy of d3.js since it is already a dependency in
both the package.json and bower.json

Updated examples to use the copy from node_modules to maintain consistency with
the unit tests

* docs: Fix typo (shutterstock#630)

Co-authored-by: Christopher Chambers <chris@peanutcode.com>
Co-authored-by: Robert Buchholz <rbu@goodpoint.de>
Co-authored-by: David Chester <david@fmail.co.uk>
Co-authored-by: David Chester <dchester@shutterstock.com>
Co-authored-by: Martin Häcker <spamfaenger@gmx.de>
Co-authored-by: Mark Derbecker <mark.derbecker@seeq.com>
Co-authored-by: deldrid1 <deldrid1@utk.edu>
Co-authored-by: Noah Gibbs <noah.gibbs@onlive.com>
Co-authored-by: Martin Olsson <martin@minimum.se>
Co-authored-by: arunv <me@arunv.com>
Co-authored-by: Luke Mawbey <lbm@lbm.net.au>
Co-authored-by: Katie McLaughlin <katie@anchor.net.au>
Co-authored-by: Callum Jones <callum@callumj.com>
Co-authored-by: Fernando Jorge Mota <f.j.mota13@gmail.com>
Co-authored-by: Shane Reustle <me@shanereustle.com>
Co-authored-by: msand <msand@abo.fi>
Co-authored-by: Joshua Goldberg <jsgoldberg90@gmail.com>
Co-authored-by: Matthew Ginnard <matthew@runscope.com>
Co-authored-by: Nicolas Racine <nic335@users.noreply.github.com>
Co-authored-by: André Tavares <mail@andretavares.com>
Co-authored-by: Brennan Ashton <bashton@brennanashton.com>
Co-authored-by: hamx0r <jason.haury@gmail.com>
Co-authored-by: Alexander Johansson <alexander@n1s.se>
Co-authored-by: GermanBluefox <dogafox@gmail.com>
Co-authored-by: Gagan Awhad <gagan.a.awhad@gmail.com>
Co-authored-by: Eirik S. Morland <eirik@morland.no>
Co-authored-by: Vivek Kushwaha <vivek.kushwaha@hindustantimes.com>
Co-authored-by: Sebastian Wallin <sebastian.wallin@gmail.com>
Co-authored-by: Michal Ostruszka <michal.ostruszka@gmail.com>
Co-authored-by: Michelle Bu <michelle@stripe.com>
Co-authored-by: Stuart Nelson <stuartnelson3@gmail.com>
Co-authored-by: Mathieu Chataigner <mathieu.chataigner@gmail.com>
Co-authored-by: Mike Atlas <mike@weft.io>
Co-authored-by: Pablo Reyes <reyesoft@gmail.com>
Co-authored-by: funvit <funvit@gmail.com>
Co-authored-by: Wil Moore III <wil.moore@wilmoore.com>
Co-authored-by: cjthompson <chris@thompson-web.org>
Co-authored-by: Jay <jay3686@gmail.com>
Co-authored-by: cesine <cesine@yahoo.com>
Co-authored-by: cesine <cesine@users.noreply.github.com>
Co-authored-by: Thomas Lackemann <tommylackemann@gmail.com>
Co-authored-by: Jay Patel <jpatel@shutterstock.com>
Co-authored-by: Benjamin J DeLong <howaboutben+github@gmail.com>
Co-authored-by: Mathias Rangel Wulff <m@rawu.dk>
Co-authored-by: Doa Jafri <doajafri@gmail.com>
Co-authored-by: Robert Tod <roberttod@gmail.com>
Co-authored-by: Tamas Goga <tamasgoga94@gmail.com>
Co-authored-by: Mokeponi <mokeponi@gmail.com>
Co-authored-by: bhashit parikh <bhashit.parikh@gmail.com>
Co-authored-by: Richard Littauer <richard.littauer@gmail.com>
Co-authored-by: Conan Tzou <conan@asperasoft.com>
Co-authored-by: Roland Schilter <roli@weave.works>
Co-authored-by: Jack Q <qiaobo@outlook.com>
Co-authored-by: Diego Zilioti <zilioti.diego@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: ftortega <francisco.tomas.ortega@gmail.com>
Co-authored-by: Julien Francoz <jfcoz@users.noreply.github.com>
Co-authored-by: Sebastian Murphy <sebastianmurphy@gmail.com>
Co-authored-by: Tim Gates <tim.gates@iress.com>
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

No branches or pull requests

2 participants