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

[Graphs Screen] Y-Axis Bounds Are Too Small #101

Closed
JRomero0613 opened this issue Aug 8, 2019 · 10 comments
Closed

[Graphs Screen] Y-Axis Bounds Are Too Small #101

JRomero0613 opened this issue Aug 8, 2019 · 10 comments

Comments

@JRomero0613
Copy link

For phetsims/qa#397 the Y-axis bounds are too small for extreme cases such as when the skater is raised very high above the top of the track or when the reference height is raised to the top of the screen and the skater is on the bottom of the track. Another three or four "zoom-out" levels would most likely solve this and make it so no matter the case the energy would still be visible on the graph.

@jessegreenberg
Copy link
Contributor

Sounds good @JRomero0613. I think it would be best to get @arouinfar input on this. @arouinfar can you please recommend the best scales/ranges to support on the energy graph?

@jessegreenberg
Copy link
Contributor

From meeting 10/3/19:

We considered actually limiting the vertical bounds of the skater, but don't like that.

We will find a good range for the graph by dragging the skater and dropping it from heights that seem useful.

@jessegreenberg
Copy link
Contributor

#194 (comment) was another description of this issue.

@arouinfar
Copy link
Contributor

arouinfar commented Mar 16, 2020

@jessegreenberg I'm not entirely sure how the graph range and zoom levels are currently specified. Do the zoom levels need to be equally spaced? Is is possible to specify the zoom ranges with an array, similar to phetsims/circuit-construction-kit-common#536 (comment)?

I think the most zoomed-out range would need to be [-25000 J, 25000 J] and the most zoomed-in range would need to be [-5 J, 5 J]. Equally spacing the zoom levels would not be ideal.

@arouinfar
Copy link
Contributor

@jessegreenberg said:

Jesse Greenberg 9:31 AM
Ah, definitely possible we can make the ranges whatever we want, they don't have to be evenly spaced.

Back to me to make recommendations on the zoom levels.

@arouinfar
Copy link
Contributor

The most zoomed-in range no longer needs to go all they way down to [-5 J, 5 J] due to the changes in #265.

@jessegreenberg let's try these zoom levels.

-25, 25
-50, 50
-100, 100
-200, 200
-500, 500
-1000, 1000
-1500, 1500
-2000, 2000
-2500, 2500  (default zoom level)
-3000, 3000
-4000, 4000
-5000, 5000
-6000, 6000
-7000, 7000
-8000, 8000
-9000, 9000
-10000, 10000
-12500, 12500
-15000, 15000
-17500, 17500
-20000, 20000
-22500, 22500
-25000, 25000

@jessegreenberg
Copy link
Contributor

@arouinfar this was done, can you take a look and review these ranges?

One thing to think about is spacing for grid lines. Each range breaks nicely into 4 so I went with grid lines along the 1/4 for each range.

@arouinfar
Copy link
Contributor

@jessegreenberg this looks really nice! Breaking things into 4 looks good.

After playing with some extreme cases, I think we can eliminate the last two levels of [-22500, 22500] and [-25000, 25000]. Looks like the [-25, 25] level was excluded, but looking at min mass/min g I don't think it's necessary after all.

@arouinfar arouinfar assigned jessegreenberg and unassigned arouinfar May 2, 2020
@jessegreenberg
Copy link
Contributor

Looks like the [-25, 25] level was excluded, but looking at min mass/min g I don't think it's necessary after all.

Hmm, looks like a bug where the zoom button was set to disabled one level too early. But sounds good, ill leave that one out anyway.

I removed the last two ranges. Back for review!

@arouinfar
Copy link
Contributor

Looks good in master. Thanks!

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

3 participants