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

Reward dialog obscures challenge solution #183

Closed
pixelzoom opened this issue Aug 13, 2018 · 10 comments
Closed

Reward dialog obscures challenge solution #183

pixelzoom opened this issue Aug 13, 2018 · 10 comments

Comments

@pixelzoom
Copy link
Contributor

Moved here from phetsims/vegas#72 (comment). Arithmetic has this problem. The reward dialog obscures the solution to the final challenge, see example in screenshot below. @amanda-phet's call on whether this needs to be addressed.

39936122-b7db4620-5508-11e8-8c73-02c8f6b986c9

@marlitas
Copy link
Contributor

We should talk through this issue as part of the upcoming region and culture publication.

@amanda-phet
Copy link
Contributor

I think we could just scale down this dialog slightly, but showing the final challenge doesn't seem as helpful to me as seeing the entire board. If I get the dialog and click 'continue' I'm taking back to the level selection screen.

So, my thought is to use the space to the right where the keypad is, and cover it with the dialog. When you click continue, and then go back to the level, the keypad is gone, so that space makes perfect sense to put the dialog.

@amanda-phet
Copy link
Contributor

Discussed 12/7/23 and we like putting the dialog on the right where the keypad used to be. The keypad already disappears, so there is space there.

Keep the continue button so there is a button where they were previously interacting. Seems strange to have two buttons that do the same thing (back button and continue button). Normally we wouldn't want to do this. In this case let's keep it because someone might not know the back button is there.

@amanda-phet
Copy link
Contributor

I'll do a mockup of this for @Luisav1 to implement.

@amanda-phet
Copy link
Contributor

Here is what I think it should look like. I tried to preserve elements of the original dialog, but the stars are smaller and there might be less space between the lines of text.

image

image

To test this feature more efficiently, can you include a query parameter to complete a level faster? :)

@amanda-phet amanda-phet assigned Luisav1 and unassigned amanda-phet Dec 8, 2023
Luisav1 added a commit that referenced this issue Dec 12, 2023
@Luisav1
Copy link
Contributor

Luisav1 commented Dec 12, 2023

I found it easiest to move the reward dialog into the control panel VBox so that it takes up the space of the keypad. This is what it looks like now.
image

@marlitas
Copy link
Contributor

@Luisav1 I looked through the code and I think we can make a couple of changes to make it a bit more performant.

I would recommend creating the LevelCompletedNode outside of the stateWrapper.lazyLink and just adding it as a child to the controlPanelVBox. You can then toggle the visibility of the LevelCompletedNode inside the link dependent on when it should be shown. I would follow the same strategy with the keypad as well, which probably means you'll have to pass that Property through to the LevelCompletedNodeWrapper.

Let me know if you want to pair on this or have any questions.

The layout shouldn't change from what is on main right now though, so I do think this is ready to get feedback from @amanda-phet.

@amanda-phet
Copy link
Contributor

amanda-phet commented Dec 14, 2023

Is there a query parameter to quickly complete a level?

Based on the screenshot provided, I think the design is just fine and I wouldn't change anything.

@amanda-phet
Copy link
Contributor

Seems to be working well. I like that I can access all the buttons and see the completed board. Thanks for making this change!

@amanda-phet amanda-phet removed their assignment Dec 14, 2023
Luisav1 added a commit that referenced this issue Dec 14, 2023
…ing and removing LevelCompletedNode in LevelCompletedNodeWrapper depending on the game state. Hide keypad when showing the level completed. See #183.
@Luisav1
Copy link
Contributor

Luisav1 commented Dec 14, 2023

Thanks for the review @amanda-phet!

@marlitas and I added the LevelCompletedNodeWrapper directly to the controlPanelVBox instead and the LevelCompletedNode is added / removed from LevelCompletedNodeWrapper depending on the game state, which avoids creating multiple LevelCompletedNode instances.

We also now hide the keypad when in the game state is showing the level completed dialog.

Closing.

@Luisav1 Luisav1 closed this as completed Dec 14, 2023
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