phetsims / equality-explorer

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

Reward dialog obscures challenge solution #104

Closed pixelzoom closed 6 years ago

pixelzoom commented 6 years ago

When 10 challenges are solved, a reward dialog is displayed immediately. E.g.:

screenshot_618

@amanda-phet brought up the concern that the solution (the value of 'x') is obscured in this case. And the student can't see the solution until they close the dialog.

And the dialog is arguably a bit too large, regardless of this problem.

pixelzoom commented 6 years ago

@amanda-phet Is this problem specific to Equality Explorer? Or does it occur in other sims (e.g. Make A Ten) when this reward dialog is used?

pixelzoom commented 6 years ago

We might alleviate this problem (somewhat) by first displaying the smiley face, and waiting until it fades out to display the reward dialog. There wouldn't be "immediate gratification". And more importantly, the smiley face is likely to distract the student from seeing the answer, so this might be a non-solution.

Note to self: This would involve relative easy (but not trivial) logic changes to the scene.scoreProperty listener at line 250 of SolveItSceneNode.

pixelzoom commented 6 years ago

We could make the reward dialog smaller, so that the solution is visible. This also allows you to see more of the stuff that's falling in the background. E.g: with scale:0.5 for the dialog:

screenshot_616

Changes required for this in SolveItSceneNode:

268     rewardDialog = rewardDialog || new RewardDialog( scene.scoreProperty.value, {

          scale: 0.5,
pixelzoom commented 6 years ago

Here's a variation on https://github.com/phetsims/equality-explorer/issues/104#issuecomment-388161852. Dialog scale: 0.75 (so not as small), but dialog is not centered on the screen, it's shifted down.

screenshot_617

Changes required for this in SolveItSceneNode:

268     rewardDialog = rewardDialog || new RewardDialog( scene.scoreProperty.value, {

          scale: 0.75,

          // Display the dialog in a location that does not obscure the challenge solution.
          // See https://github.com/phetsims/equality-explorer/issues/104
          layoutStrategy: function( dialog, simBounds, screenBounds, scale ) {
            var center = simBounds.center.times( 1.0 / scale );
            dialog.centerX = center.x;
            dialog.centerY = center.y + 50; // offset determined empirically
          },
pixelzoom commented 6 years ago

@amanda-phet Let me know if any of the above solutions would address your concerns.

amanda-phet commented 6 years ago

@amanda-phet Is this problem specific to Equality Explorer? Or does it occur in other sims (e.g. Make A Ten) when this reward dialog is used?

This occurs in Make a Ten as well.

I like the third option the best (0.75 and shifted down). It makes me realize that the original dialog design was probably excessively large.

amanda-phet commented 6 years ago

@phet-steele what do you think?

pixelzoom commented 6 years ago

@amanda-phet said:

It makes me realize that the original dialog design was probably excessively large.

The implementation (RewardDialog) is currently used only in Equality Explorer and the vegas demo application. But I think it's OK that it's a bit large -- it's preferable to scale down than scale up.

pixelzoom commented 6 years ago

I asked:

@amanda-phet Is this problem specific to Equality Explorer? Or does it occur in other sims (e.g. Make A Ten) when this reward dialog is used?

@amanda-phet said:

This occurs in Make a Ten as well.

And are there other sims that use a similar reward dialog, and have a similar problem?

pixelzoom commented 6 years ago

The issue of whether other sims have this problem has been moved to https://github.com/phetsims/vegas/issues/72.

I implemented the preferred solution -- scale:0.75 and shifted down. @amanda-phet please review in master. If it looks OK, you can close this issue.

phet-steele commented 6 years ago

@phet-steele what do you think?

To be blunt, I hate the offset. I know it solves the issue, but it is very buggy looking placement. Maybe I'm not used to non-centered dialogs?

I'd like to see the delayed option, where the dialog appears once the smiley starts to fade. I'd imagine you would even still get to hear the "ding" sound for a correct answer. We could even delay it further, right up until when the Next button should appear (but make sure the button doesn't actually appear yet) so that more time is given to noticing the solution. Simply put, I do not think this comment from this issue is true, especially after 10 correct answers: "...the smiley face is likely to distract the student from seeing the answer...".

To me, the problem has not strictly been the lack of being able to see the solution. That's certainly part of the problem, but I've always been more concerned with the lack of knowing you came to the solution at all. I'd much rather the user experience the fanfare of getting the right answer, THEN experience the fanfare of getting 10 correct answers. Games don't skip level completion rewards just because you finished the last level; that's like Mario not getting any points for finally saving the princess....or something. Seeing the reward dialog without FIRST seeing that you got a correct answer may not quite make the connection that you are seeing the dialog BECAUSE of your correct answer. I vividly remember a moment of being startled from the reward dialog because I did not realize I had stumbled across the solution (which is why I revisited this issue), so I'd much rather see the delay option.

Side note: you can see the solution by just dismissing the dialog. There are literally five different ways to dismiss this dialog, none of which remove the solution from being viewed afterward.

Side side note (really this is not important to consider at this time, at all): has there been any talk about games + a11y? I don't have the know-how to deal with that, but it doesn't feel like we should rely on moving the dialog to put the solution in view. We should probably give time for screen readers to communicate the solution before saying anything about the reward dialog (with a delayed dialog). Maybe?

FTR, @pixelzoom isn't shrinking not the best way to do this? Wouldn't we rather change the dimensions of the dialog and keep the scale at "1"? For one, shrinking makes the Close button smaller, let alone everything else in the dialog. The real problem is the size of the dialog, not the size of the dialog's contents. Yes, we'd probably need to do something about the PhET Girl image to make her fit, but that extraneous to the undesirable effect of shrinking the interactable buttons.

pixelzoom commented 6 years ago

To be blunt, I hate the offset. I know it solves the issue, but it is very buggy looking placement. Maybe I'm not used to non-centered dialogs?

I'm not a fan of off-centered dialogs either. But this was the best solution that I could come up with. My gut reaction was that it's a non-problem, but I felt like I should offer some potential solutions.

I'd like to see the delayed option, where the dialog appears once the smiley starts to fade. ...

This is going to be 1-2 hours of work. Imo, it may not solve the problem, and is likely to cause other problems. Delaying the appearance of the dialog any amount of time after the smiley face disappears is going to slow game play. And based on the amount of fiddling I've been asked to do to get the length of the smiley fade to be tolerable, I suspect that the time between when the face disappears and the dialog appears is not going to be spent ruminating on the solution, it's going to be spent wondering "what the heck?"

FTR, @pixelzoom isn't shrinking not the best way to do this? Wouldn't we rather change the dimensions of the dialog? For one, shrinking makes the Close button smaller, let alone everything else in the dialog. The real problem is the size of the dialog, not the size of the dialog's contents. Yes, we'd probably need to do something about the PhET Girl image to make her fit, but that extraneous to the undesirable effect of shrinking the interactable buttons.

It's complicated... It depends on the layout bounds used by the sim, which determines how things are scaled overall. When porting sims from Java, we tend to override the default HTML5 layout bounds and use bounds that are similar to the Java sim. This makes all of the Java layout code port nicely, often with only minor changes.

So... We could change the dimensions of the dialog, and allow the client to specify the font sizes for the buttons. But that will complicate the implementation significantly. Happy to do that, but will require more fiddling by client code. And (imo) the scale: 0.75 looks like about the correct font sizes for this sim. The default font sizes for the buttons are (imo) a tad too big.

phet-steele commented 6 years ago

I'd like to see the delayed option, where the dialog appears once the smiley starts to fade...We could even delay it further, right up until when the Next button should appear.

@pixelzoom @amanda-phet turns out this difference feels significant. IMO, waiting until the fade ends (1.8 sec) is way better than waiting for the fade to start (1 sec). (Tired this myself to check)

@pixelzoom said:

... I suspect that the time between when the face disappears and the dialog appears...

There was never an intent to have there be a gap time between the smiley and the reward, which it sounds like you are implying.

pixelzoom commented 6 years ago

Following up on @phet-steele's point:

FTR, @pixelzoom isn't shrinking not the best way to do this? Wouldn't we rather change the dimensions of the dialog? ...

In commit https://github.com/phetsims/vegas/commit/f2d7b0ac2273bf1d61f7231c1a2ea56a5e320a62, I've reworked RewardDialog so that the defaults are appropriate for standard sim layoutBounds, and added options to support customization by sims that use non-standard layoutBounds. Sims should no longer need to scale this dialog, and scaling has been removed from Equality Explorer.

You can see the defaults in the vegas demo, screenshot below.

(EDIT: I increased the size of the score a bit, and revised the screenshot.)

screenshot_623

pixelzoom commented 6 years ago

Here's the current state of EqEx, no scaling of the dialog, but it is off center.

screenshot_624

pixelzoom commented 6 years ago

@amanda-phet and I discussed via Slack. The goal is to keep the answer visible at all times (temporary occlusion by falling reward objects is OK.) Delaying the appearance of the dialog in order to keep it centered does not achieve that goal - it delays the problem, and the student may still not see the answer because they are distracted by the smiley face. So we're going to stick with shifting the dialog down. I've reduced that shift to minimize the "off center" look, see screenshot below.

@amanda-phet Please review. If there's nothing else to do here, please close.

screenshot_634

phet-steele commented 6 years ago

@pixelzoom can you put a note of the offset somewhere that QA can see so that no one creates an issue thinking it's a bug? (Just in case someone misses this issue)

amanda-phet commented 6 years ago

@phet-steele Where would such a note go, if this issue and the code do not suffice?

phet-steele commented 6 years ago

@phet-steele Where would such a note go, if this issue and the code do not suffice?

Likely in implementation-notes.md, where it would be linked for review in a future QA test task.

pixelzoom commented 6 years ago

I don't see the point in mentioning this in implementation-notes.md. This is a design decision, not unlike the decision about whether or not to show an accordion box's title when it's open. Yes, most of the time a Dialog is centered, but it's not a requirement, it just happens to be the default. And it's not going to be the default for non-modal Dialogs (the specification says they will open wherever they were last closed.)

So I'm not going to do anything additional here. The reason for this specific dialog being shifted down is well documented in the code. And if anyone reports it as an issue, I'll refer them to this issue and close as "not a bug".

Closing.