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

Keyboard input should snap to grid #47

Closed
zepumph opened this issue May 12, 2020 · 10 comments
Closed

Keyboard input should snap to grid #47

zepumph opened this issue May 12, 2020 · 10 comments

Comments

@zepumph
Copy link
Member

zepumph commented May 12, 2020

From #44

@emily-phet I am confirming here that this behavior is only for keyboard, and we don't want this for mouse. Because of this, I felt like it deserved its own issue to think about how to apply singulary to this input modality.

@zepumph
Copy link
Member Author

zepumph commented May 12, 2020

Not sure why this wasn't working. First thought is that downDelta is annotated as being in view coordinates. If that is the case, why?!?!

Index: js/free-objects/view/RatioHalf.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- js/free-objects/view/RatioHalf.js	(revision 3dc94cafabd4a341bacf6b1826275ccf7737e32b)
+++ js/free-objects/view/RatioHalf.js	(date 1589244791833)
@@ -133,9 +133,13 @@
     pointer.addInputListener( dragListener );
 
     // transform and dragBounds set in layout code below
+    const downDelta = 1 / designingProperties.gridBaseUnitProperty.value;
+    debugger;
     const keyboardDragListener = new KeyboardDragListener( {
       positionProperty: positionProperty,
       start: () => { firstInteractionProperty.value = false; },
+      downDelta: downDelta,
+      shifDownDelta: downDelta / 4,
       end: () => this.alertManager.dragEndListener( valueProperty.get() )
     } );
     pointer.addInputListener( keyboardDragListener );

@emily-phet
Copy link

@zepumph Yes, this behavior is intended only for focus-based navigation.

@zepumph
Copy link
Member Author

zepumph commented May 18, 2020

I made each hand a slider above. Now it will snap to each grid line, and using shift will subdevide each grid width by 4. This updates no matter what the grid base-unit is. @emily-phet please review.

@emily-phet
Copy link

@zepumph I think the general snap-to-gridline behavior looks great. I think we'll need the shift behavior to change based on the current ratio, to ensure that success can always be reached. Note, there will likely be an option for teachers / students to change the ratio, so this will need to be anything reasonable with a range of (likely ) 1-10 for the left and 1-10 for the right.

@zepumph
Copy link
Member Author

zepumph commented May 19, 2020

I think we'll need the shift behavior to change based on the current ratio

I agree we will need to make sure that the target ratio is always available, but I worry a bit about changing the behavior of keys as a side-effect of changing the target ratio.

Possible easy solution: Make the shift step be 1/10 of a grid line (10 up arrows when holding shift make a single key press when not holding shift). This is simple to implement, and will give the needed granularity for any target ratio.

Possible harder solution: put this on hold, and then once the keyboard interaction get's a bit more set in stone, come back and determine the best way to alter the step size based on the target ratio.

I prefer the easy solution, to see how far it can take us. Let me know what you would like to do with this issue.

@zepumph zepumph assigned emily-phet and unassigned zepumph May 19, 2020
@emily-phet
Copy link

I like the "shift = 1/10 of a step" idea. I worry that it might introduce some description challenges for the scenario where there are no gridlines or numbers (the qualitative description scenario) but if this is the "easy solution" let's go with it for now and (as you said) see how far it can take us.

zepumph added a commit that referenced this issue May 21, 2020
@zepumph
Copy link
Member Author

zepumph commented May 21, 2020

Implemented above. Let's see how that goes, and revisit as needed.

@emily-phet
Copy link

@zepumph It still seems like 1/4 steps for me, from phettest.

@emily-phet emily-phet assigned zepumph and unassigned emily-phet May 25, 2020
@zepumph
Copy link
Member Author

zepumph commented May 26, 2020

Hmmm. I just tested and it looks to be using 1/10 step for the shift key step on master locally and on phettest. Perhaps it was a caching problem?

@zepumph zepumph removed their assignment May 26, 2020
@emily-phet
Copy link

Looks good!

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

2 participants