Gesture features for micro:bit #271
Conversation
xnkevinnguyen
requested review from
abmahdy,
adclements,
iennae,
isadorasophia,
markAtMicrosoft,
nasadigital,
sagarmanchanda and
smmatte
as code owners
|
Can we make the dropdown the same length as the "send gesture" button |
|
Some other things:
|
xnkevinnguyen
changed the title
Width is changed |
When selected, clicking enter will trigger the gestures |
| <div className="AccelerometerBar"> | ||
| <br /> | ||
| {/* Implement Gestures Here */} | ||
| <Dropdown options={GESTURES} onSelect={props.onSelectGestures} /> | ||
| <SensorButton | ||
| label={GESTURE_BUTTON_MESSAGE} | ||
| onMouseDown={() => { | ||
| if (props.onSendGesture) { | ||
| props.onSendGesture(true); | ||
| } | ||
| }} | ||
| onMouseUp={() => { | ||
| if (props.onSendGesture) { | ||
| props.onSendGesture(false); | ||
| } | ||
| }} | ||
| onKeyDown={(e: React.KeyboardEvent) => { | ||
| handleOnKeyDown(e, props.onSendGesture); | ||
| }} | ||
| onKeyUp={(e: React.KeyboardEvent) => { | ||
| handleOnKeyUp(e, props.onSendGesture); | ||
| }} | ||
| type="gesture" | ||
| /> | ||
| <ThreeDimensionSlider | ||
| axisProperties={MOTION_SENSOR_PROPERTIES} | ||
| onUpdateValue={props.onUpdateValue} |
There was a problem hiding this comment.
I feel the current UI is a bit confusing. After adjusting the sliders in the bottom, it is a bit unclear whether I need to press the "Send Gesture" button. It is also not clear whether after picking some gesture you need to adjust the sliders as well or not (E.g. it is unclear whether picking a gesture and clicking the send gesture button is enough or if you also need to adjust the sliders).
What do you think?
There was a problem hiding this comment.
That is some good feedback, do you think if we make the instruction clearer it would help?
Maybe something like: "Select a gesture then press the button to send the gestures or you could change acceleration manually with the sliders."
There was a problem hiding this comment.
To be honest, it's very tricky and I am not super familiar with UX design. I would consult someone who would be more familiar like the design interns or Katelynn.
If I had to come up with something (keeping in mind our timeline), I think maybe separating the gestures from the accelerometer sliders would clear up things. Up to you.
There was a problem hiding this comment.
Ok understandable. What I'm thinking of doing with the time constraints is separate in the same modal the manual acceleration and sending gestures. Separating the 2 sensors would require extra work and svgs that we don't currently have.
| onMouseDown={() => { | ||
| if (props.onSendGesture) { | ||
| props.onSendGesture(true); | ||
| } | ||
| }} |
There was a problem hiding this comment.
You can replace this with:
onMouseDown={() => props.onSendGesture?.(true)}
There was a problem hiding this comment.
It seems like this functions optional chaining with the typescript version we have is not compatible. This could be added when we update the typescript version. Since this doesn't affect user experience, I will not include this change.
There was a problem hiding this comment.
function optional chaining is available with TS 3.7.0
Description:
Send gestures via dropdown from the webview to typescript side and then typescript side to python
[AB#34024] and AB#34128
.
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Type of change
Please delete options that are not relevant.
Limitations:
Please describe limitations of this PR
Testing:
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist:
npm run formatand passes the checks innpm run check