search

phetsims / circuit-construction-kit-common

"Circuit Construction Kit: Basics" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
10 stars 10 forks source link

Cut button does not stay disabled in Studio #838

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 Safari

Problem description For https://github.com/phetsims/qa/issues/772: In studio, if I disable the cut button, it will be grayed out initially. If I move to another connection it will be enabled and if I go back to the original connection, that is also enabled. Also, if I disable the cut button and launch the sim, it is enabled. This happens on both screens.

Steps to reproduce

  1. In studio, go to the intro screen and make a simple circuit
  2. Click on a connection so that the cut button appears
  3. In the tree go to view.circuitNode.cutButton.enabledProperty and set it to false--the button will be grayed out
  4. Click on another connection--the button is enabled.
  5. Go back to the original connection--the button is enabled.

Visuals cutbutton

arouinfar commented 2 years ago

Good find @Nancy-Salpepi.

Looks like the model overrides cutButton.enabledProperty each time the button is summoned. Interestingly, the same doesn't happen with the visibleProperty. However, I don't see any real need for a client to control the enabled property of this particular button. If they don't want the student to cut the vertex, they should be using isCuttableProperty or even temporarily setting cutButton.visibleProperty to false (the delete key could still be used to cut the vertex in this case).

@samreid I would be fine with setting cutButton.enabledProperty to phetioReadOnly: true in this case.

This seems like a trivial change, so if we will be sending the client an updated dev version after QA wraps up, I would include this issue. Practically speaking, it's unlikely to cause any harm if we wait to fix it until production, so I wouldn't consider it critical.

@samreid I'm adding this to the client preview milestone, but feel free to bump it to the production milestone if you disagree with my assessment.

samreid commented 2 years ago

The commit sets cutButton to phetioReadOnly:true. I tested that is working as desired in studio. @arouinfar anything else for this issue?

arouinfar commented 2 years ago

Looks good, thanks!