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

ChartCanvasNode demo looks messed up #26

Closed
pixelzoom opened this issue Mar 14, 2021 · 4 comments
Closed

ChartCanvasNode demo looks messed up #26

pixelzoom opened this issue Mar 14, 2021 · 4 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Mar 14, 2021

@samreid is the bamboo demo for ChartCanvasNode supposed to look like this?

screenshot_912

@samreid
Copy link
Member

samreid commented Mar 14, 2021

Yes, it's a demo for missing data. But we could move that to a separate labeled test so it's not confusing if that would help.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Mar 14, 2021

OK, gotcha. At the very least, that should be documented in DemoChartCanvasNode.js.

And I think that perhaps we should carve out some time to clean up the bamboo demos. Some of these demos are confusing because they aren't really demos of one or two specific things, they are testing a bunch of things. For example, testing SpanNode in DemoLinePlot. New issue?...

@samreid
Copy link
Member

samreid commented Mar 30, 2021

The current documentation is a line comment here:

for ( let x = min; x <= max; x += delta ) {

  // Test holes in the data
  const data = ( Math.abs( x ) < 0.1 && x < 0 ) ? null : new Vector2( x, Math.sin( x * frequency + offset ) );
  dataSet.push( data );
}

I added another note and revised the demo so that only one curve is missing data.

image

It seems the missing data doesn't need to be a separate test because missing data is a feature of ChartCanvasNode.

For example, testing SpanNode in DemoLinePlot.

To me, it makes sense to show the SpanNode in the context of a fuller plot, how do you recommend to organize this? Please open a new issue for that depending on your recommendation.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Apr 5, 2021

Doc looks good, and I think it's a nice change to only have a hole in 1 plot -- it looks less like a bug.

To me, it makes sense to show the SpanNode in the context of a fuller plot, how do you recommend to organize this? Please open a new issue for that depending on your recommendation.

I'm not suggesting that things should not be showing in context. What I'm pointing out is that there is generally too much going on in some of these demos, and some components (like SpanNode) cannot be selected via the combo box. I'll create a new GitHub issue for "clean up demos" (see #28).

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

No branches or pull requests

2 participants