search

phetsims / graphing-quadratics

"Graphing Quadratics" is an educational simulation in HTML5, by PhET Interactive Simulations.
MIT License
1 stars 4 forks source link

Revise the point tool to fit 2 decimal places. #158

Closed amanda-phet closed 3 years ago

amanda-phet commented 3 years ago

Is this a common component? I can't find it in common code anywhere, and doesn't seem to be shared with graphing lines as far as I can tell. (I need a refresher, even though I'm sure we talked about this back when we worked on the sim).

@Nancy-Salpepi brought up some cases where it might be nice to see the same value for the root and the value the point tool is detecting, instead of rounding on the point tool.

image

The only way to make more space for it would be to make the grab area visually smaller. Is this feasible to do? Can the same design also go to other sims using the point tool, or is that not the case since it's not a common component?

ariel-phet commented 3 years ago

@amanda-phet I think you could certainly make the "three bar grab area" narrower horizontally.

  1. If I recall, the reason for making the grab area this size would be to discourage occluding the readout with your finger while moving the tool. However, I am not sure that is of major concern considering the way this tool is used.

  2. The pointer area for the tool is currently the entire tool (including the cross hairs), so I highly doubt reducing the size of the grab area would affect usability, being that the tool is reasonably large and obvious.

Basically it seems like a totally reasonable change (and my instincts would be it is not too cumbersome of a change to make)

pixelzoom commented 3 years ago

Is this a common component? I can't find it in common code anywhere, and doesn't seem to be shared with graphing lines as far as I can tell. (I need a refresher, even though I'm sure we talked about this back when we worked on the sim).

Point tool is not common code. And there are in fact different implementation in graphing-quadratics and graphing-lines, because they have different requirements and work with different underyling models (straight lines vs quadratics). I raised this concern in https://github.com/phetsims/graphing-quadratics/issues/18 during the implementation of graphing-lines. @amanda-phet and @ariel-phet participated in that discussion, and we decided to use different implementation for these 2 sims.

graphing-quadratics and graphing lines DO share the same assert (.ai) file that was used to create the images. From images/license.json:

"notes": "created by Bryce Gruneich, asset file is graphing-lines/assets/point_tool.ai"

@Nancy-Salpepi brought up some cases where it might be nice to see the same value for the root and the value the point tool is detecting, instead of rounding on the point tool. The only way to make more space for it would be to make the grab area visually smaller. Is this feasible to do?

I agree with @ariel-phet's answer to this question in https://github.com/phetsims/graphing-quadratics/issues/158#issuecomment-901377398.

Can the same design also go to other sims using the point tool, or is that not the case since it's not a common component?

Not the case. The only thing shared is the .ai file. But if you're going to change graphing-quadratics, you should carefully consider what to do in graphing-lines.

The point tools in graphing-lines only display integer coordinates. So you'll be making the "display rectangle" much larger than is needed for graphing-lines.

If you don't mind the point tools having a different body size in graphing-quadratics and graphing-lines, then we should copy the .ai file to graphing-quadratics, modify that .ai file, generate new .png files, and make changes only to graphing-quadratics code.

If you want the point tools in graphing-quadratics and graphing-lines to have the same body size, then make the change to graphing-lines .ai file, general new .png files for both sims, and make code changes in both sims.

Finally... If you don't mind ditching the "brushed metal" look of the tools, we could get rid of the .ai files and (programmatically) make these tools the appropriate size for the value that they need to display. Their look would be more similiar to the 3D-looking probes used in various PhET sims. This will be a significant chunck of work, but is imo the best longterm approach. And we'd have to figure out the best way to add the grippy things.

Here's an example of a 3D-looking tool in beers-law-lab. It uses ShadedRectangle, which is in common code.

screenshot_1198
pixelzoom commented 3 years ago

Here's an example of a 3D-looking tool in beers-law-lab. It uses ShadedRectangle, which is in common code.

screenshot_1198
amanda-phet commented 3 years ago

Thanks for all of this background info, @pixelzoom

I’d like to move forward with the improvements to the point tool. I really like the idea of the reusable component, and I prefer the appearance, but it would delay things significantly because Chris is going on vacation soon. Also, factoring out the point tool body later on sounds much easier than doing it now during the RC process. If getting the next RC out is the priority, then @pixelzoom and I think this is the best path forward:

This week: change the body of the point tool in the ai file and make the “window” larger, which would be pretty quick to do (<1 hr)

After the PhET-iO publication: @pixelzoom will factor out the point tool body in common code, making a common point-tool-body-node in code that is more easily reusable (4-6 hours)

Mockup of potential new point tool on the right: image

Waiting for @kathy-phet to confirm that we should move forward with this plan.

kathy-phet commented 3 years ago

I like the new mock-up of the "coded" point tool.

Can you clarify will the point tool always be two decimals, or will it only go to two decimals on special points? If its always two decimal precision, will it be (9.1, 9.1) if it is (9.10, 9.10) point, or will (9,9) be displayed as (9.00, 9.00)? That is will it ever show only 1 decimal or no decimal places if it is precise value?

Oh, and the plan of editting the .ai file now, and doing the body code later sounds OK. @pixelzoom - Do you anticipate a PhET-iO API change with that change?

amanda-phet commented 3 years ago

Can you clarify will the point tool always be two decimals, or will it only go to two decimals on special points? If its always two decimal precision, will it be (9.1, 9.1) if it is (9.10, 9.10) point, or will (9,9) be displayed as (9.00, 9.00)? That is will it ever show only 1 decimal or no decimal places if it is precise value?

It will only go to two decimals when necessary, so 9.1, 9, would be displayed if those were the locations, not 9.10 or 9.00. The point tool currently behaves this way, we just want to expand to hundredths place to match the values of some roots.

pixelzoom commented 3 years ago

@kathy-phet said:

I like the new mock-up of the "coded" point tool.

To clarify... That is not what we're planning to do for 1.2 release. That will be longterm work that will apply to graphing-quadratics, graphing-lines, and graphing-slope-intercept.

Can you clarify will the point tool always be two decimals, or will it only go to two decimals on special points?

We are not proposing any change to current behavior. It will continue to work as it does now - trailing zeros are stripped. This is consistent will all coordinate displays in the sim.

Oh, and the plan of editting the .ai file now, and doing the body code later sounds OK.

👍🏻

@pixelzoom - Do you anticipate a PhET-iO API change with that change?

No PhET-iO change should be necessary.

kathy-phet commented 3 years ago

Yes, I understand the body code would be later, and long term work. Thanks for clarifying on no API changes. If you are both happy to move forward, go for it!

pixelzoom commented 3 years ago

The long-term work to generalize "point tool body" and migrate it to common code is tracked in https://github.com/phetsims/graphing-quadratics/issues/167.

pixelzoom commented 3 years ago

Done, pushed to master and 1.2 branches. @amanda-phet please review in master. Assign back to me for testing in the next RC.

I asked @amanda-phet about whether to maintain a constant font size for the coordinate values, or to scale them to fit the point tool's display area. The former would required a font size change, from 15 to 13. She replied on Slack:

I think the way it’s working now, shrinking to fit, is working fine, so that most of the time the coordinates can be larger.

amanda-phet commented 3 years ago

Something about the gradient was off, so I revised the artwork.

Can you please integrate the newer artwork and assign back to me to check? (sent via slack)

pixelzoom commented 3 years ago

Artwork revised in the above commits, in master and 1.2 branches. @amanda-phet please verify in master.

amanda-phet commented 3 years ago

Looks great- thank you!

pixelzoom commented 3 years ago

Ready for testing in the next RC. To test:

Nancy-Salpepi commented 3 years ago

Point tool works as intended. However, I do want to point out, that values for the roots and what is displayed in the point tool still may not match.

Screen Shot 2021-08-27 at 7 55 28 AM
pixelzoom commented 3 years ago

Right, we're only snapping to x integers, not y. That was to meet the pedagogical goal of being able to plug x back into the equation to solve for y. We did not attempt to solve the case of trying to snap the tool to "special points" like roots. Assigning to @amanda-phet to decide whether there's more to do here. If so, please provide instructions, and we'll need another RC. If not, please close this issue.

amanda-phet commented 3 years ago

Yes, this is still a possibility, but should occur less frequently. The only way around it would be to fit 3 decimal places. On this screen in particular, using the point tool to identify the roots is not an important functionality of the point tool, so we do not need to do anything here.