keypad allows you to enter values that are out of range

[pixelzoom]

This is probably not related to #127, since it behaves this way in 1.0.0-dev.8. But the keypad allows you to enter values that are out of range, and are then changed when you press Enter.

Example:

1. Go to "Lab" screen
2. Select "Custom" from combo box.
3. Press edit button next to Mass.
4. Enter "7500", then press Enter button
5. Mass is filled in with "5000".

Example:

1. Go to "Lab" screen
2. Select "Custom" from combo box.
3. Press edit button next to Mass.
4. Enter "0", then press Enter button
5. Mass is filled in with "1".

I realize what's going on here -- Mass must be >= 1 and <= 5000. But you shouldn't be able to enter invalid values, it's confusing to the user.

See the marker editor in Unit Rates, for example. You can't enter '0' for the numerator. In the same way, you shouldn't be able to enter '0' for Mass , or any other value that is out of range.

[andrealin]

The above notifies the user that they are out of range, and does not allow them to enter. @arouinfar and I looked at it and liked it. @pixelzoom , assigning to you to review.

[pixelzoom]

Comments/suggestions:

(1) The "Range" message needs to be localized. Add something like "Range: {{min}} - {{max}} {{units}}" to the strings file.

(2) Consider making the message 'red', and changing the foreground or background of the invalid value to 'red'. Test for colorblindness.

(3) Consider interviewing to see if students have a problem with this. It's a non-standard way of indication an error, and the value and message are about as far apart as they could possibly be.

[pixelzoom]

To support the case whether there are no units, run the string through trim to remove the extra space after the max value. Otherwise the message won't be quite centered in the panel.

I.e.:

// projectile-motion-strings_en.json
  "rangeMessage": {
    "value": "Range: {{min}} - {{max}} {{units}}"
  },

// KeypadLayer
var rangeMessageString = require( 'string!PROJECTILE_MOTION/rangeMessage' );
this.rangeMessage = StringUtils.fillIn( rangeMessageString, {
  min: valueRange.min,
  max: valueRange.max,
  units: unitsString ? unitsString : ''
} ).trim();

[arouinfar]

Thanks for reviewing @pixelzoom!

@andrea-phet I really like suggestions (1) and (2).

There have been a few other changes made, such as #88, #126, #132, #134, so it may be worth doing 1-2 confirmation interviews. What do you think @ariel-phet?

[pixelzoom]

Re:

(2) Consider making the message 'red', and changing the foreground or background of the invalid value to 'red'. Test for colorblindness.

In Unit Rates, we used 'red' foreground (vs background) to indicate an incorrect answer to a question. It also seems more consistent to use red foreground for both the value and message. So I would start with changing the foreground color and see how it looks. Then change the foreground of the value back to 'black' the next time one of the Keypad keys is pressed.

[andrealin]

@pixelzoom how about that? I also checked with @arouinfar already.

[pixelzoom]

When the user presses any Keypad key (numbers, decimal point, backspace), shouldn't the message be hidden and the value fill be restored to 'black'?

[pixelzoom]

For example:

1. Go to Lab screen
2. Select 'Custom'
3. Press edit button for Mass
4. Enter '5001' and press Enter button
5. "Range message" is displayed and '5001' value turns red.

So far, so good. Now here's what I'd expect:

6. Press any key on the Keypad.
7. "Range" message disappears, value turns 'black'.

But what I see is:

6. Hit the backspace until there's only 1 digit left in the value.
7. Value turns 'black'
8. "Range" message remains visible until the Enter button is pressed again.

[pixelzoom]

Thinking about this more.... Perhaps it is appropriate to keep the "Range" message visible until the user hits Enter again -- since they've made a range error, it's useful to see what that range is. But I think the text should turn to 'black' as soon as they begin to edit the value.

[pixelzoom]

More... I don't understand the logic for turning the value 'black' when there's 1 digit. For range 1-5000, if I enter '5001' (for example) and hit the backspace, then '500' is a valid value. For range 10-100, if I enter enter '1', then later press '2', then '12' is valid.

[andrealin]

I was trying to turn the value black when the user presses another digit (because it should turn black with next key press, so that was a workaround), but I see now that there are loopholes.

[andrealin]

Here are new updates incorporated:

[arouinfar]

I like it @andrea-phet! @ariel-phet are you ok with returning "Range"?

[pixelzoom]

Doesn't "Range" have another meaning in this sim's domain? How about using the quantity's name, e.g. "Mass: 1 - 5000 kg". "Drag Coefficient: 0.04 - 1" would likely get scaled down a bit.

[pixelzoom]

"Drag Coefficient: 0.04 - 1" doesn't look too bad:

[pixelzoom]

Mass: 1 - 5000 kg

[arouinfar]

Doesn't "Range" have some other meaning in this sim?

It does @pixelzoom, but @andrea-phet told me that there are two different string keys which have some context to separate the x-distance "Range" from the min-max "Range".

Thanks for the screenshots @pixelzoom! I think "Mass: 1-5000 kg" looks great, but I don't really love the look when the strings get longer. Using the variable name may also be more challenging (length-wise) when it comes to i18n, so I'm leaning towards sticking with "Range".

@ariel-phet can you review and advise?

[ariel-phet]

Thought not completely correct, could also just go with "Drag: 0.04 - 1" since it is explicitly labeled as the drag coefficient in the panel.

My thought was actually just do things like "1 -5000 kg" since the fact it is mass is obvious from the unit label, and for the special case of drag do "Drag: 0.04 - 1"

I am hesitant about using the word Range. What do you think about that solution @arouinfar?

[pixelzoom]

@arouinfar said:

... there are two different string keys which have some context to separate the x-distance "Range" from the min-max "Range".

I wasn't worried about a conflict in string keys. I'm concerned about confusion caused by showing "Range" in the user interface with 2 different semantics. I.e.:

[pixelzoom]

@ariel-phet said:

Thought not completely correct, could also just go with "Drag: 0.04 - 1" since it is explicitly labeled as the drag coefficient in the panel.

Complicates the code by having an optional "abbreviated form" of the label that's used for quantities in the control panel.

[andrealin]

"Domain" instead of "Range?" Kind of interesting vocabulary, but it avoids using two different Ranges.

[ariel-phet]

@arouinfar we could also use "C_d" (as in "C" with "d" as a subscript) since that is a very common abbreviation of drag coefficient.

I think if for drag it came up as "C_d: 0.04 -1" might also work just fine. Your main concern was it looked like an expression. This is a pretty particular use case, and I think it would be OK.

[ariel-phet]

@arouinfar or perhaps the simplest solution:

For drag do "0.04 to 1" so it does not look like an expression.

I really really don't like using the redundant and possibly confusing "Range" or "Domain" prefix when we are just trying to make the drag case cleaner.

[pixelzoom]

Imo, using different labels in the keypad and control panel is fundamentally problematic. It's potentially confusing to the user, it complicates the code, and it's being considered for the wrong reason (to address space constraint, rather than improve user experience). I would consider making the keypad wider (to accommodate identical labels) before resorting to using different labels.

[pixelzoom]

@ariel-phet said:

For drag do "0.04 to 1" so it does not look like an expression.

Not in favor of a solution that is specific to "Drag Coefficient". That is an English-specific solution. Treat all quantities the same.

[ariel-phet]

@arouinfar I think @pixelzoom and I agree that using "to" for all would work fine

"1 to 5000 kg"
"0.04 to 1"

That keeps things compact, consistent, and clear

[pixelzoom]

Also should require minimal code changes. Change this:

  "rangeMessage": {
    "value": "Range: {{min}} - {{max}} {{units}}"
  }

to this:

  "rangeMessage": {
    "value": "{{min}} to {{max}} {{units}}"
  }

[arouinfar]

"1 to 5000 kg"
"0.04 to 1"

Love it @ariel-phet!!

[andrealin]

All good. Assigning to @pixelzoom to review and close.

[pixelzoom]

👍 Closing.