phetsims / buoyancy

"Buoyancy" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
2 stars 2 forks source link

Add a panel to display % Submerged #112

Closed DianaTavares closed 1 month ago

DianaTavares commented 2 months ago

To add this panel, first #32 needs to be solved.

Preference to show/hide this feature in the Simulation Panel Title: Percent submerged readout Query Parameter: percentSubmergedReadout Default true in Basics, Default false in full Buoyancy

image

Here are the mockups of its position in each screen:

Intro image

Explore image

Shapes image

Bottle Observe how, as this is the only object on the screen, the number is bigger and centered: image

Boat Observe that the name in the panel said "% Boat submerged" because in this case, we want to know the percentage of the boat and not the block.

image

AgustinVallejo commented 2 months ago

The above commit improves on this panel. Still missing:

AgustinVallejo commented 2 months ago

Added to the preferences dialog, noting to change the default to false in DensityBuoyancyCommonQueryParameters before moving on (or move on and remember way down the line, it's okay)

zepumph commented 2 months ago

I was just taking a look at the new and improved ReadoutListAccordionBox. I love it! I really like how much we can factor out with setReadout. I believe we can make things much simpler and more suave with a parametric type. If we have a list that can be typesafe, we may be able to move some of the implementation of setReadout to the supertype.

Basic outline:

I'd just like to think about if this would be helpful. Let's talk about it tomorrow.

zepumph commented 2 months ago
AgustinVallejo commented 2 months ago

@zepumph this question comes up often and I think the answer is always to leave the other components floating there. The same happened multiple times in MSS and Keplers. I would check off that item.

zepumph commented 2 months ago
zepumph commented 2 months ago

First pass of submerged and density panels were added to the intro screen over in https://github.com/phetsims/buoyancy/issues/96 (see https://github.com/phetsims/buoyancy/issues/96#issuecomment-1996002214 for next steps there).

AgustinVallejo commented 2 months ago

Moved the Second Mass button group to the corner of the screen, next to reset all button. Now when there're two masses it isn't obstructed, but the panels are still too close to the buttons for my liking. According to https://github.com/phetsims/buoyancy/issues/32 they should be somewhere else but I'm still discussing with Diana about that position, just wanted them not overlapping for now, and now we know what's the function in charge of positioning that button group.

zepumph commented 2 months ago

Please note that I too am making changes to ReadoutAccordionBox over in https://github.com/phetsims/buoyancy/issues/96 (AV and I have discussed this, but I'm adding a paper trail).

AgustinVallejo commented 2 months ago

This issue has been getting some commits that probably rather belong in https://github.com/phetsims/density-buoyancy-common/issues/105

Assigning this issue back to @DianaTavares to check if the Submerged Accordion Box is ready. In that case, please close.

zepumph commented 2 months ago

@DianaTavares I believe you can still rework on the review in Buoyancy.

DianaTavares commented 2 months ago

Cool! I have some comments:

In the intro:

zepumph commented 2 months ago

I believe that we are having trouble with the new Property for tracking submerged. On CT is seems like we are trying to set it NaN sometimes. Can you take a look?

zepumph commented 1 month ago

@DianaTavares Please note that this feature is not working when gravity is 0. See https://github.com/phetsims/buoyancy/issues/124

AgustinVallejo commented 1 month ago

On our way to close this monster issue. Creating issues for the remaining work:

phet-dev commented 1 month ago

Reopening because there is a TODO marked for this issue.

zepumph commented 1 month ago

Ha, we were so close! Unfortunately both of those TODOs seem good to discuss. @AgustinVallejo let's talk about them.

AgustinVallejo commented 1 month ago

Addressed the colorProperty TODO in the above commit, while improving the way those custom explore options were mapped. Left the other one open since I'd like to discuss: As we're returning that pattern conditionally, I'm not sure if creating a PatternStringProperty right there is the right way to go, or would cause memory leaks.