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

Create a plot that draws data with arrows #34

Closed
jessegreenberg opened this issue Jun 14, 2021 · 5 comments
Closed

Create a plot that draws data with arrows #34

jessegreenberg opened this issue Jun 14, 2021 · 5 comments

Comments

@jessegreenberg
Copy link
Contributor

From phetsims/greenhouse-effect#44, we need a plot that is similar to BarPlot but where the bars have arrow heads. Over slack I checked to see if anyone had a recommendation for how to go about this:

Jesse Greenberg 5:16 PM
I have a plot that looks like this:
image.png
image.png

5:17
Which is currently using BarPlot from Bamboo. But it has been requested that arrow heads be added to each bar, which BarPlot doesn't support. Does that seem like something BarPlot should provide or do you think I should roll my own?

Chris Malley 5:18 PM
Arrow heads are typically added to bars (in PhET sims) only when the value of the bar is off scale. So first, I would push back on the request — it’s non-standard, and likely to confuse. (edited)
5:20
I see that Excel supports “up down arrow” charts, https://www.youtube.com/watch?v=wRvyrznLDiA, so maybe that’s where this request is coming from.
YouTubeYouTube | Karina Adcock
How to make an up and down arrows chart in excel

Jesse Greenberg 5:20 PM
There is another graphic in the sim that looks like this, and the motivation for the request was to make the two look the same.
image.png
image.png

Chris Malley 5:21 PM
I would create UpDownArrowPlot, a new Plot type for bamboo.

Jesse Greenberg 5:21 PM
Arrows for energy flux make sense to me, I can't remember why arrows were needed to represent energy balance.

Chris Malley 5:22 PM
Or call it whatever you think is best. I suggested UpDownArrowPlot because Excel seems to call these “up down arrow charts”.

Jesse Greenberg 5:22 PM
OK, sounds good, thanks!

Chris Malley 5:23 PM
It’s API should be identical to BarPlot (and other Plot classes). That would be more obvious if JavaScript had interfaces. (edited)
5:24
Public methods are obvious. But note the public this.dataSet field.

To summarize, the recommendation was to create a new class that draws the data with arrows, called UpDownArrowPlot. The class should have the same API as all the other plots in bamboo.

@jessegreenberg jessegreenberg self-assigned this Jun 14, 2021
@jessegreenberg
Copy link
Contributor Author

I added an UpDownArrowPlot in the above commit. It is much like BarPlot, but draws the data with ArrowNode instead of Rectangle.

@samreid would you mind reviewing this addition? One thing I noticed was that WebStorm flagged some things in the constructor as duplicated code with BarPlot, which comes from them all having a matching API. Do you think it would be worthwhile to have a supertype like BamboPlot that is extended by BarPlot, UpDownArrowPlot, Lineplot, CanvasLinePlot (and maybe others) that implements these kinds of lines?

image

(Green highlight is what WebStorm flagged as duplicated).

@jessegreenberg
Copy link
Contributor Author

jessegreenberg commented Jun 15, 2021

A better way to add/remove the graphic for each data point was shown in #33. Rather than removing all then adding all back each update, it should just add/remove the new ArrowNodes if necessary. This improvement was made in the following commit.

@samreid
Copy link
Member

samreid commented Jun 15, 2021

The changes look good and I tested them in greenhouse effect. Would you like to add a bamboo demo that shows the UpDownArrowPlot?

I also opened #35 to factor out a BambooPlot parent type.

@jessegreenberg
Copy link
Contributor Author

OK sounds good, thanks for reviewing. I made a simple demo for the BambooDemoScreenView. Anything else for this one?

@jessegreenberg jessegreenberg removed their assignment Jun 15, 2021
@samreid
Copy link
Member

samreid commented Jun 15, 2021

Looks great, thanks! Closing.

@samreid samreid closed this as completed Jun 15, 2021
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