Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

disposed Dialog blocks interaction with the sim #618

Closed
pixelzoom opened this issue Aug 26, 2020 · 4 comments
Closed

disposed Dialog blocks interaction with the sim #618

pixelzoom opened this issue Aug 26, 2020 · 4 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Aug 26, 2020

There appears to be a new bug in Dialog, possibly introduced when PopupablePanel was factored out. It is currently manifesting in RewardDialog, in the vegas demo. To reproduce:

  1. Run the vegas demo
  2. Go to Reward screen.
  3. Press the "open Reward" button
  4. Press either the "Keep Going" or "New Level" button

The dialog is disposed, but the barrier rectangle is still present and you can't interact with the ScreenView.

The vegas demo uses dispose to dismiss the dialogs, see RewardScreenView.js:

          keepGoingButtonListener: () => {
            console.log( 'Keep Going button' );
            rewardDialog.dispose();
          },
          newLevelButtonListener: () => {
            console.log( 'New Level button' );
            rewardDialog.dispose();
          }

So possibly this problem is specific to dispose, and maybe hide works OK?

Assigning to @jonathanolson to investigate, since he did the PopupablePanel refactor. Sorry I didn't have more time to investigate, or reproduce with a vanilla Dialog.

@pixelzoom pixelzoom changed the title Dialog does not behave correctly when disposed disposed Dialog blocks interaction with the sim Aug 26, 2020
@pixelzoom
Copy link
Contributor Author

Reported again in phetsims/vegas#85. @ariel-phet please prioritize.

@samreid
Copy link
Member

samreid commented Sep 28, 2020

Based on the behavior reported in phetsims/vegas#85, these symptoms may need to block publication. I'll mark accordingly--this can be revised when we know more.

@jonathanolson
Copy link
Contributor

isShowingProperty was being improperly disposed in Dialog. This ALSO resulted in it being disposed BEFORE hide(), so that hide wasn't functioning properly. Should be fixed with the above commit, @samreid can you review/verify?

@samreid
Copy link
Member

samreid commented Oct 1, 2020

Great fix, I reviewed the code change and tested in vegas. Looks great. I presume if this affected any in-progress sims, it would have been identified by QA, so I don't think it needs to be cherry picked anywhere. Closing.

@samreid samreid closed this as completed Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants