phetsims / density

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

Block visible property not maintained in Studio #112

Closed Nancy-Salpepi closed 2 years ago

Nancy-Salpepi commented 2 years ago

Test device MacBook Air (m1 chip)

Operating System 12.0.1

Browser safari15

Problem description For https://github.com/phetsims/qa/issues/738 in Studio Wrapper:

On the Mystery Screen, a block will reappear when its visible property is set to false. This happens with all Sets.

Steps to reproduce Here is one example:

  1. In the Studio wrapper go to the mystery screen
  2. Go to density.mysteryScreen.model.blockSets.set1.block1E.visibleProperty and uncheck checkbox to set value to 'false'
  3. Select Set 2
  4. Select Set 1--the 1E block reappears and the checkbox is now checked

Visuals blocksvisibleproperty

jonathanolson commented 2 years ago

@arouinfar thoughts on how this should work? This is how modes control what blocks are showing up (it changed from being created dynamically, because we needed the blocks to always exist for phet-io).

arouinfar commented 2 years ago

Nice catch @Nancy-Salpepi!

@jonathanolson this looks analogous to https://github.com/phetsims/density/issues/104. There needs to be a way to hide individual blocks that isn't overridden by the model when switching between the radio button options.

jonathanolson commented 2 years ago

@arouinfar I'd need more information about how this should work... it seems like there are three settings for some override:

  1. Default (let the sim decide whether it's visible)
  2. Force visible (block is there even if it would be otherwise hidden)
  3. Force invisible (block is hidden even if it would be otherwise there).

Should this be discussed at phet-io meeting?

arouinfar commented 2 years ago

@jonathanolson a block should only be visible if the appropriate radio button option is selected and the block's visibleProperty is true.

For example, Bock 1A is visible if the value of density.mysteryScreen.model.blockSets.blockSetProperty is SET_1 and density.mysteryScreen.model.blockSets.set1.block1A.visibleProperty is true. If the user sets block1A.visibleProperty to false, it would not appear when "Set 1" is selected.

Force visible (block is there even if it would be otherwise hidden)

This should never be possible. A block is visible if (and only if) the appropriate blockSetProperty is selected and its visibleProperty is true. Currently, the model will set the visibleProperty when the blockSetProperty changes, but there should be a way for users to essentially turn off certain blocks so that they never appear.

jonathanolson commented 2 years ago

Implemented discussed changes above. Added the main modeProperty to the first screen, and swapped the visibleProperty so there's a hidden internal one AND a studio-visible one.

@arouinfar can you try it out?

arouinfar commented 2 years ago

@jonathanolson the block visibility is behaving great in master. The only thing that looks a bit strange to me is the modeProperty. I thought the goal of introducing this property was to have it power the blocks RadioButtonGroup rather than using the visibleProperty of the 2nd block.

Changing the value of modeProperty in the Studio interface will change the state of the blocks RadioButtonGroup, but the reverse isn't true. If I change select a radio button in the sim, the modeProperty doesn't update to match.

jonathanolson commented 2 years ago

@arouinfar this was definitely buggy before. I believe it's fixed up in master now, can you verify?

arouinfar commented 2 years ago

Thanks @jonathanolson the modeProperty is now behaving as expected in Studio. Back to you to decide if this should be closed or verified in the next QA cycle.

jonathanolson commented 2 years ago

Thanks! Closing.