phetsims / joist

Joist is the main framework for PhET Interactive Simulations. Joist creates and displays the simulation content, home screen, navigation bar, About dialog, enables switching between tabs, and other framework-related features.
MIT License
8 stars 6 forks source link

The "One or More Query Parameters have invalid values" error dialog should be more specific and helpful #923

Closed samreid closed 1 year ago

samreid commented 1 year ago

From https://github.com/phetsims/center-and-variability/issues/148#issuecomment-1545878197, @catherinecarter said:

I tried to say ?maxKicks=22, and I got a nice error dialog box letting me know 22 was not an allowable number of max kicks. My only recommendation is to have the error dialog say how many kicks the default is (currently it tells me default values will be used, but doesn't tell me how many this is, or what values are allowed).

Is there value in also saying, "You can change the max number of kicks in the preferences menu." Or something like that to point users to where they can change this and see possible values that are valid?

The listed authors for QueryParametersWarningDialog.ts are @chrisklus and @pixelzoom. Can you please comment on questions like:

samreid commented 1 year ago

@catherinecarter commented:

I don't think the error dialog should block publication. [for CAV 1.0]

pixelzoom commented 1 year ago

@samreid said:

... The listed authors for QueryParametersWarningDialog.ts are @chrisklus and @pixelzoom. Can you please comment on questions like: ...

QueryParametersWarningDialog was intentionally barebones/simple. From the design issue:

@arouinfar in https://github.com/phetsims/joist/issues/593#issuecomment-578907663:

I believe the conclusion in PhET-iO meeting was to keep the dialog simple and shift some responsibility to the user.

@amanda-phet in https://github.com/phetsims/joist/issues/593#issuecomment-578917716:

If the decision is to keep the dialog simple, this certainly works. It's not the most user-friendly, but I think that isn't necessarily the goal right now. My understanding is that we want to create a more robust solution later and will put more effort in to making it user friendly at that time. If that is all correct, then this looks like a fine solution for now.

We intentionally went "simple" because there is a recognized need for a QueryStringMachine 2.0. Rather than putting effort into QueryParametersWarningDialog, PhET should be advancing QueryStringMachine 2.0. So I recommend that we do nothing and close this issue.

samreid commented 1 year ago

The recommendation in the preceding comment sounds good to me, @catherinecarter what do you think?

catherinecarter commented 1 year ago

Sure - sounds like this has come up in the past, and the solution is to keep as is and put effort into other, more valuable, items. Thanks for weighing in, @pixelzoom, and for providing more information about the history of this discussion. Ok with me to close.