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

Do we actually want to use Dialog for the Mean Info? #230

Closed
marlitas opened this issue May 8, 2024 · 5 comments
Closed

Do we actually want to use Dialog for the Mean Info? #230

marlitas opened this issue May 8, 2024 · 5 comments

Comments

@marlitas
Copy link
Contributor

marlitas commented May 8, 2024

While working on a comment from #196, I discovered that 1 year ago Marla had implemented the Dialog API incorrectly. This is why we can see strange things in the studio tree where the dialog has both a visibleProperty and isShowingProperty:
image

It is not an expected part of Dialog's API to use a visibleProperty to toggle on and off the appearance of the dialog.

This incorrect implementation allowed us to do things that are not normally part of dialog UX. For example normally when a dialog pops up in our sims, the background darkens, and as soon as you click anywhere outside of the dialog the dialog immediately goes away.

Dialog Implementation:
image

Current Implementation:
image

This discovery is unfortunate because @amanda-phet pointed out that during interviews a student was exploring moving the balls around while seeing the calculations change in real time.

In the end this is a design decision. We can keep the current behavior we have, and I move away from using a Dialog and instead it will act as a Node that is in front of everything in the notepad, or we use the Dialog API as is intended, and lose some of the current behavior, but remain more consistent with PhET dialog UX.

This should be discussed synchronously to avoid confusion.

@amanda-phet
Copy link
Contributor

Design team wants this to be a non-modal dialog! Or at least a non-modal viewable thing.

Questions:

  • Why do we only have modal dialogs? After all this time, is there a good reason for avoiding non-modal dialogs? Why isn't there currently a common code option to make non-modal dialogs?
  • In theory we could switch the "mode" or "view" of the notepad to show the mean calculation, using a toggle, instead of a dialog on top of the notepad. Should we do this? Pedagogically, we don't want to elevate this information to be more visible, so probably not.
  • Should we run this by others to get more thoughts into this implementation?
  • Can we just implement this as a node, and not worry about it looking like a non-modal dialog but not being called a dialog?

@zepumph will take it from here.

@zepumph
Copy link
Member

zepumph commented May 9, 2024

I created phetsims/sun#882 and will report back with a recommendation on how to proceed in this sim.

@zepumph
Copy link
Member

zepumph commented May 13, 2024

Today during Dev meeting, it was recommended that we just use a non Dialog node with a close button for this case. The common issue was closed and we can proceed with a custom Node for this context. Thanks!

@zepumph zepumph assigned jbphet and marlitas and unassigned zepumph May 13, 2024
@marlitas marlitas removed the type:question Further information is requested label May 15, 2024
@marlitas
Copy link
Contributor Author

This is ready for review by @jbphet.

@jbphet
Copy link
Contributor

jbphet commented Jun 7, 2024

Nice. Changes look good, and it seems like a significant improvement. Closing.

@jbphet jbphet closed this as completed Jun 7, 2024
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