-
Notifications
You must be signed in to change notification settings - Fork 6
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
Alt input for HeaterCoolerNode #853
Comments
… on blur for alt input, see #853
We can use the event from
We could add another key that sets it back to zero, but I don't think that is necessary. @pixelzoom what do you think of this change? |
Thanks @jessegreenberg. This seems to be working nicely. With keyboard input, the slider says where you put it, and snaps back to zero when focus is lost (blur). But I do think that having a 0 (and Home?) shortcut would make HeaterCoolerNode more usable. A typical interaction using the mouse: I apply heat, overshoot the temperature, release the slider to quick turn of heat, then apply cool to get closer to the desired themperature, then release the slider to quickly turn off cooling. Having to loose keyboard focus to shut off heat/cool makes this type of interaction inconvenient/confusing with the keyboard. @arouinfar what do you think? |
I agree @pixelzoom. I think it would be very helpful to have a shortcut key to instantly turn off the HeaterCoolerNode. Users may want to heat/cool to a specific temperature without overshooting. Similarly, users may want to cut the heat before the lid explodes, which can happen very quickly. Being able to use a single key press to turn off the HeaterCoolerNode would improve the user experience. As for the key itself, I think 0 makes a lot of sense and is consistent with FaucetNode. Home would presumably take the slider to its minimum value, which corresponds to maximum cooling. |
OK, I added a listener to the '0' key for this. We probably need custom keyboard help content for HeaterCoolerNode then? @arouinfar what should that look like? |
Here is a patch with something to get started, but the language should probably be improved. Subject: [PATCH] Update usages of KeyboardListener and KeyboardDragListener after changes from https://github.com/phetsims/scenery/issues/1570
---
Index: js/keyboard/help/SliderControlsKeyboardHelpSection.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/keyboard/help/SliderControlsKeyboardHelpSection.ts b/js/keyboard/help/SliderControlsKeyboardHelpSection.ts
--- a/js/keyboard/help/SliderControlsKeyboardHelpSection.ts (revision 61197c7b28debdf6c92bc1fb55e358e40a037204)
+++ b/js/keyboard/help/SliderControlsKeyboardHelpSection.ts (date 1715121293164)
@@ -11,7 +11,7 @@
import Enumeration from '../../../../phet-core/js/Enumeration.js';
import EnumerationValue from '../../../../phet-core/js/EnumerationValue.js';
import optionize from '../../../../phet-core/js/optionize.js';
-import { HBox } from '../../../../scenery/js/imports.js';
+import { HBox, Node } from '../../../../scenery/js/imports.js';
import sceneryPhet from '../../sceneryPhet.js';
import SceneryPhetStrings from '../../SceneryPhetStrings.js';
import TextKeyNode from '../TextKeyNode.js';
@@ -22,7 +22,7 @@
import assertMutuallyExclusiveOptions from '../../../../phet-core/js/assertMutuallyExclusiveOptions.js';
// Configurations of arrow keys that can be displayed for 'Move between items in a group'
-class ArrowKeyIconDisplay extends EnumerationValue {
+export class ArrowKeyIconDisplay extends EnumerationValue {
public static readonly UP_DOWN = new ArrowKeyIconDisplay();
public static readonly LEFT_RIGHT = new ArrowKeyIconDisplay();
public static readonly BOTH = new ArrowKeyIconDisplay();
@@ -64,6 +64,8 @@
// Determines whether this keyboard help section will have the "Adjust in Larger Steps" row.
includeLargerStepsRow?: boolean;
+
+ additionalRows?: KeyboardHelpSectionRow[];
};
export type SliderControlsKeyboardHelpSectionOptions = SelfOptions & KeyboardHelpSectionOptions;
@@ -94,7 +96,8 @@
adjustInLargerStepsStringProperty: SceneryPhetStrings.keyboardHelpDialog.adjustInLargerStepsStringProperty,
includeSmallerStepsRow: true,
- includeLargerStepsRow: true
+ includeLargerStepsRow: true,
+ additionalRows: []
}, providedOptions );
let adjustSliderStringProperty = options.adjustSliderStringProperty;
@@ -224,7 +227,8 @@
...( options.includeSmallerStepsRow ? [ adjustSliderInSmallerStepsRow ] : [] ),
...( options.includeLargerStepsRow ? [ adjustInLargerStepsRow ] : [] ),
jumpToMinimumRow,
- jumpToMaximumRow
+ jumpToMaximumRow,
+ ...options.additionalRows
];
super( options.headingStringProperty, content, options );
Index: js/demo/keyboard/KeyboardScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/demo/keyboard/KeyboardScreenView.ts b/js/demo/keyboard/KeyboardScreenView.ts
--- a/js/demo/keyboard/KeyboardScreenView.ts (revision 61197c7b28debdf6c92bc1fb55e358e40a037204)
+++ b/js/demo/keyboard/KeyboardScreenView.ts (date 1715121106324)
@@ -18,6 +18,7 @@
import demoKeyboardHelpIconFactory from './demoKeyboardHelpIconFactory.js';
import demoFaucetControlsKeyboardHelpSection from './demoFaucetControlsKeyboardHelpSection.js';
import demoSpinnerControlsKeyboardHelpSection from './demoSpinnerControlsKeyboardHelpSection.js';
+import demoHeaterCoolerControlsKeyboardHelpSection from './demoHeaterCoolerControlsKeyboardHelpSection.js';
type SelfOptions = EmptySelfOptions;
type KeyboardScreenViewOptions = SelfOptions & PickRequired<DemosScreenViewOptions, 'tandem'>;
@@ -35,6 +36,7 @@
{ label: 'BasicActionsKeyboardHelpSection', createNode: demoBasicActionsKeyboardHelpSection },
{ label: 'ComboBoxKeyboardHelpSection', createNode: demoComboBoxKeyboardHelpSection },
{ label: 'FaucetControlsKeyboardHelpSection', createNode: demoFaucetControlsKeyboardHelpSection },
+ { label: 'HeaterCoolerControlsKeyboardHelpSection', createNode: demoHeaterCoolerControlsKeyboardHelpSection },
{ label: 'KeyboardHelpIconFactory', createNode: demoKeyboardHelpIconFactory },
{ label: 'KeyboardHelpSection', createNode: demoKeyboardHelpSection },
{ label: 'KeyNode', createNode: demoKeyNode },
|
Suggested changes to #853 (comment): Jump to minimum => Maximum cool [Home] |
@jessegreenberg I reviewed the alt input behavior of HeaterCoolerNode in Gas Properties and Energy Forms and Changes. The zero key to turn off the HeaterCoolerNode is a useful shortcut. EFAC supports the To reproduce in EFAC on main:
As for the keyboard help content, I like @pixelzoom's suggestions in #853 (comment). I don't have any other change requests. |
Thanks @arouinfar, in 43a25d2 the snap to zero on blur only happens if In c4f3651 I added the new keyboard help content. @arouinfar confirmed it looks OK over slack. @pixelzoom can you please review this? In particular the new string keys and changes to SliderControlsKeyboardHelpSection? |
Over slack @pixelzoom recommended renaming these files to be consistent with #714 HeaterCoolerControlsKeyboardHelpSection -> HeatCoolControlsKeyboardHelpSection So those renames were done in the above commits. |
All looks good. See screenshot below for HeatCoolControlsKeyboardHelpSection used in Gas Properties. Back to @jessegreenberg. Feel free to close if there's all work is done. |
OK, thank you for reviewing. Closing. |
@arouinfar, I have questions on this closed issue.
The reason I ask is that I have recently been reading up on WCAG Success Criteria for keyboard accessibility, specifically 2.1.4 Character Key Shortcuts. Since we do not yet have a way to re-map or turn off shortcuts, single character keyboard shortcuts must require focus. I think this is how we typically design single-character-key shortcuts, though I may not be aware of all shortcuts. |
@terracoda Answers to your questions below. You can test drive by running Gas Properties, or by running the demo that @jessegreenberg provided.
No. As stated in the keyboard help, Home is "Maximum cool" (the minimum slider value, the same as other sliders). Zero is "Off", which for a heater/coooler is 0 value, at the center of the slider range. End is "Maximum heat" (the maximum slider value, the same as other sliders)
Only when the control has focus. |
Needed for phetsims/gas-properties#213 ...
@arouinfar and I reviewed the current behavior of HeaterCoolerNode. The current behavior with keyboard input needs work, it's not really usable.
With the mouse, you can drag the slider and hold it in one place to heat/cool. When the mouse is released, the slider snaps back to zero.
With the keyboard, the slider snaps back to zero as soon as you release the up/down arrow key. So it's impossible to set the slider to a specific value and "hold" it there. What should probably happen is that the arrow keys are used to set the slider to a specific value, and "0" (and Home? and blur?) makes the slider snap back to zero.
I don't have any info on the history behind the current behavior, whether it was intentionally designed, etc. I suspec that there has been no design, and we're just seeing the default Slider behavior. @jessegreenberg do you have any recollection? How would we specify different behavior for the slider's DragListener vs KeyboardDragListener?
Here's the relevant code, in HeaterCoolerFront.ts. I wonder if it's as simple as only calling
setSliderToZero
for DragListener, and whetherendDrag
can tell whether it was called by DragListener or KeyboardDragListener.The text was updated successfully, but these errors were encountered: