phetsims / plinko-probability

"Plinko Probability" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
5 stars 7 forks source link

issues with "Out Of Balls!" dialog #72

Closed pixelzoom closed 6 years ago

pixelzoom commented 7 years ago

In the Lab screen, when you reach a maximum number (currently 9999) of balls in any one bin, this ugly dialog pops up:

screenshot_109

And it fails stringTest=long:

screenshot_110

pixelzoom commented 7 years ago

@amanda-phet If you have any requests for how you'd like this dialog to look, let me know. I'm going to take a stab at it. Nothing fancy, I'll just make it look respectable and handle i18n properly.

pixelzoom commented 7 years ago

I find it a little odd that this dialog is triggered by some maximum number of balls being reached in 1 bin, versus some maximum number of balls being dispensed from the hopper. Since it's such a large number, maybe not worth worrying about. But @amanda-phet and @ariel-phet let me know if you have an opinion on this aspect of it.

pixelzoom commented 7 years ago

Also wondering why the Play button is still enabled if we're "out of balls"? If I close the dialog and press the Play button, the dialog immediate pops up again. Shouldn't the Play button be disabled until either the eraser button or Reset All button is pressed?

pixelzoom commented 7 years ago

Revised dialog, ditched the multiline text (harder to read), decreased font size, increased margins:

screenshot_02

stringTest=long:

screenshot_03

pixelzoom commented 7 years ago

Hmmm... This dialog appears only in the Lab screen, where the max is 9999 balls (unlikely to be reached). If doesn't appear in the Intro screen, where the max is 100 balls (likely to be reached), and the Play button is simply disabled when the max is reached.

@amanda-phet Why does one screen need this dialog, while the other does not?

pixelzoom commented 7 years ago

Not sure why this is done in LabScreenView when the dialog is displayed:

157 // sets the play button to active.
158 playPanel.setPlayButtonVisible();

Why would we want to make the Play button active when we've reached the max?

pixelzoom commented 7 years ago

The Play button now disables when the max number of balls is reached, and is enabled only after pressing eraser or Reset All button (similar to Intro screen).

pixelzoom commented 7 years ago

So... The remaining design questions are:

  1. What is the limit that we should be looking for in the Lab screen? In the Intro screen, it's the number of balls dispensed (100). In the Lab screen, it's quite different for some reason - it's the number of balls in any one bin (9999).
  2. Do we even need this dialog? Or is disabling the Play button (as we do in the Intro screen) a sufficient cue?
  3. If we do need this dialog, why doesn't the Intro screen have the same dialog?
  4. Is the revised Dialog acceptable? (screenshot in https://github.com/phetsims/plinko-probability/issues/72#issuecomment-243611253)
pixelzoom commented 7 years ago

I discussed questions 1, 2 and 3 above with @ariel-phet. All of those had been discussed in design meetings, and the current implementation is what is desired.

So @amanda-phet, all I need is the answer to 4: Is the revised Dialog acceptable? ... as shown in https://github.com/phetsims/plinko-probability/issues/72#issuecomment-243611253.

amanda-phet commented 7 years ago
  1. Yes. The smaller text is a nice improvement.
pixelzoom commented 7 years ago

Thanks @amanda-phet. Closing.

jessegreenberg commented 6 years ago

In the above commit I removed a workaround that was necessary because of https://github.com/phetsims/joist/issues/346. @pixelzoom wanted to let you know in case you want to review. I removed the Panel but kept the spacing the same to match the look of the deployed version.

pixelzoom commented 6 years ago

Since this sim is likely to be published before I return from vacation, it's probably best to have @andrea-phet or @jbphet review the changes.

andrealin commented 6 years ago

Looks good, closing.