phetsims / acid-base-solutions

"Acid-Base Solutions" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/sims/html/acid-base-solutions/latest/acid-base-solutions_en.html
GNU General Public License v3.0
4 stars 7 forks source link

Unable to adjust in larger steps with keyboard nav #231

Closed Nancy-Salpepi closed 1 year ago

Nancy-Salpepi commented 1 year ago

Test device MacBook Air M1 chip

Operating System 13.5

Browser Safari 16.6

Problem description For https://github.com/phetsims/qa/issues/975, the home/end keys (fn +left/right arrows) and the page up/down keys (fn + up/down arrows) do the same thing --they jump to the minimum and maximum values.

Screenshot 2023-09-06 at 4 11 51 PM

Steps to reproduce

  1. On the My Solution screen, tab to the Initial Concentration slider and press the home, end, page up and page down keys
  2. Do the same with the Strength slider.
Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Acid-Base Solutions‬ URL: https://phet-dev.colorado.edu/html/acid-base-solutions/1.3.0-dev.4/phet/acid-base-solutions_all_phet.html Version: 1.3.0-dev.4 2023-09-01 17:25:59 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/16.6 Safari/605.1.15 Language: en-US Window: 1317x706 Pixel Ratio: 2/1 WebGL: WebGL 1.0 GLSL: WebGL GLSL ES 1.0 (1.0) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 30 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}
pixelzoom commented 1 year ago

Reproduced in main. This is because PAGE_KEYBOARD_STEP is currently the same as the maximum range. So home and page will both move to min/max.

The current "keyboard step" values for each slider are shown below. @arouinfar let me know how you'd like these modified.

InitialConcentrationSlider:

// These values are regrettably hardcoded, see https://github.com/phetsims/acid-base-solutions/issues/212
// LogSlider's linearValueRange is [-3,0] in this case, with a tick mark at each integer value, so...
const PAGE_KEYBOARD_STEP = 3; // page up/down will jump to min and max
const KEYBOARD_STEP = 0.5; // up/down, 2 intervals per tick mark
const SHIFT_KEYBOARD_STEP = 0.2; // shift-up/down, 5 intervals per tick mark

StrengthSlider:

// These values are regrettably hardcoded, see https://github.com/phetsims/acid-base-solutions/issues/212
// LogSlider's linearValueRange is [-10,2] in this case, with tick marks only at min and max, so...
const PAGE_KEYBOARD_STEP = 12; // page up/down will jump to min and max
const KEYBOARD_STEP = 1; // up/down interval
const SHIFT_KEYBOARD_STEP = 0.25;  // shift up/down interval
const NUMBER_OF_MIDDLE_THRESHOLDS = 23; // number of sounds when dragging between min and max
pixelzoom commented 1 year ago

@arouinfar How about if we change PAGE_KEYBOARD_STEP as shown below. The other steps seem reasonable.

// InitialConcentrationSlider
const PAGE_KEYBOARD_STEP = 1; // so that there are 3 steps over the range [-3,0]

// StrengthSlider
const PAGE_KEYBOARD_STEP = 4; // so that there are 3 steps over the range [-10, 2]
pixelzoom commented 1 year ago

I went ahead an committed what I suggested in https://github.com/phetsims/acid-base-solutions/issues/231#issuecomment-1709187958. @arouinfar and @Nancy-Salpepi please review. Last one to review can please close, if everything looks OK.

Nancy-Salpepi commented 1 year ago

This change looks good to me!

arouinfar commented 1 year ago

Looks good on main, thanks @pixelzoom!