feat(Toggle and ToggleSmall): on Enter key press toggle action is being triggered.

[paschalidi]

Hello you all 👋 🌳 ! Thanks once again for giving me the opportunity to help in this amazing project!

Closes https://github.com/IBM/carbon-components-react/issues/2195

Without using a mouse navigate to the Toggle and ToggleSmall components and press the Enter key to activate the button. It fails to respond. Also note, this issue occurs with and without a screen reader.

Changelog

Adds event onKeyUp to take care explicitly of the key.which===13 (Enter key).
Why onKeyUp
Pressing Space key will also trigger the button to toggle, in my humble opinion Space and Enter press key should behave the same. Therefore I chose onKeyUp.

• on Toggle & ToggleSmall without using a mouse to navigate pressing the Enter key changes the state of the button to its opposite state, from on to off and visa versa.

Hope this makes sense!!
Have a good Tuesday 🕊

[netlify]

Deploy preview for carbon-components-react ready!

Built with commit f764779

https://deploy-preview-2219--carbon-components-react.netlify.com

[emyarod]

thanks for tackling this @paschalidi! we have a set of helper functions for keyboard event handlers here that you can utilize for this PR

[paschalidi]

hey @emyarod thanks for the quick review. Is keys object deprecated or am i free to use??

[emyarod]

the object is fine to use, I believe that comment is just a note about how the event.which property is deprecated. we probably want to use event.key instead?

[asudoh]

Good discussion - For using .key, we need to be careful about IE11, which tends to use different symbols from latest browsers. Even though .which is a deprecated standard, it works very well considering cross-browser support.

[paschalidi]

@emyarod and @asudoh thanks for the feedback. please have another look when you get the time

[asudoh]

LGTM basically, one question looking at your explanation on "why onKeyUp" section - Do you have some explicit reason not choosing onKeyPress? Thanks!

[paschalidi]

Thanks for asking @asudoh.
when you are having on onKeyPress and you press Enter key and you hold it then the toggle will change back and forth many times. What happens currently when you press Space key to change the state of the toggle, is that once you leave the key the state is changing and it is not toggling at the time you have it pressed, my idea is that these two keys, Enter and Space should be have the same behaviour and therefore I chose onKeyUp.

Hope I can phrase this more clearly this time. Let me know if you still have questions please ! 👍

[asudoh]

Cool thank you for explaining @paschalidi!

[emyarod]

thanks for the quick changes @paschalidi! just curious, any particular reason for using onKeyUp over onKeyDown in this scenario? in any case, looks good to me!

[paschalidi]

@emyarod hey happy to help you have this awesome project. thanks for making this possible by making this open sourced. 🏆

I am happy you also asked and being thorough! That means you people take really good care of this code base. Really nice of you!

quoting this time from MDN
keypress

The keypress event is fired when a key that produces a character value is pressed down. Examples of keys that produce a character value are alphabetic, numeric, and punctuation keys. Examples of keys that don't produce a character value are modifier keys such as Alt, Shift, Ctrl, or Meta.
❗️ The keypress event has been deprecated. You may want to use beforeinput or keydown instead.

and
keydown

Unlike the keypress event, the keydown event is fired for all keys, regardless of whether they produce a character value.

So these two are almost the same but the first wont work for all the keys therefore is these days deprecated.
Again the behaviour we would get from having onKeyDown is the same as the onKeyPress. As I try to explain above I believe that we would like to have the same behaviour that Space key has by default. Please look here for more detailed explanation.

Always feel free to disagree if you have an opinion that you would like to raise! Was a good discussion, hope we all learned something! ☮️

[emyarod]

right I understand the desire to avoid the repeated events. my question was originally around the timing of the event firing. however that was based on my initial thought that the default behavior for inputs like checkboxes was to toggle on keydown and this is not actually the case!

[paschalidi]

right I understand the desire to avoid the repeated events. my question was originally around the timing of the event firing. however that was based on my initial thought that the default behavior for inputs like checkboxes was to toggle on keydown and this is not actually the case!

If the default behaviour is onKeyDown I am happy to change it when you both agree that this iswhat we would like to have here. 👍

[emyarod]

the default behavior for native checkbox is to change state on keyup, so we can keep the behavior as is. thanks for your help @paschalidi !

[paschalidi]

Was lots of fun! Thanks @emyarod