Provide a general summary of the issue here Based on the code belo

[RAC] Checkbox should not be toggled with 'Enter' key about react-spectrum HOT 2 OPEN

sookmax commented on June 20, 2024

[RAC] Checkbox should not be toggled with 'Enter' key

from react-spectrum.

Comments (2)

snowystinger commented on June 20, 2024 1

Your analysis looks correct. I'm also not too sure about the fix for it, but this seems like a reasonable starting place. Thank you for the very detailed issue!

from react-spectrum.

sookmax commented on June 20, 2024

I've looked into this a little more and realized the way RAC Checkbox's <label> and <input> are laid out is fundamentally different than that of React Spectrum's.

To be more specific, it looks like <label> handles all the Checkbox interactions while <input> is hidden away visually in a RAC Checkbox, whereas the situation for a React Spectrum Checkbox is quite the opposite—<input> is styled in a way that it sits on top of <label> and other elements that are part of a Checkbox and handles all the interaction itself.

So because of this duality of the roles of <label> and <input>, I now think adding checks for HTMLLabelElement wherever HTMLInputElement is checked in usePress() is a valid way to fix this issue.

Specifically in these functions:

react-spectrum/packages/@react-aria/interactions/src/usePress.ts

Lines 767 to 781 in d80999e

    
           function isValidKeyboardEvent(event: KeyboardEvent, currentTarget: Element): boolean { 
        
             const {key, code} = event; 
        
             const element = currentTarget as HTMLElement; 
        
             const role = element.getAttribute('role'); 
        
             // Accessibility for keyboards. Space and Enter only. 
        
             // "Spacebar" is for IE 11 
        
             return ( 
        
               (key === 'Enter' || key === ' ' || key === 'Spacebar' || code === 'Space') && 
        
               !((element instanceof getOwnerWindow(element).HTMLInputElement && !isValidInputKey(element, key)) || 
        
                 element instanceof getOwnerWindow(element).HTMLTextAreaElement || 
        
                 element.isContentEditable) && 
        
               // Links should only trigger with Enter key 
        
               !((role === 'link' || (!role && isHTMLAnchorLink(element))) && key !== 'Enter') 
        
             ); 
        
           }

react-spectrum/packages/@react-aria/interactions/src/usePress.ts

Lines 876 to 890 in d80999e

    
           function shouldPreventDefaultKeyboard(target: Element, key: string) { 
        
             if (target instanceof HTMLInputElement) { 
        
               return !isValidInputKey(target, key); 
        
             } 
        
             if (target instanceof HTMLButtonElement) { 
        
               return target.type !== 'submit' && target.type !== 'reset'; 
        
             } 
        
             if (isHTMLAnchorLink(target)) { 
        
               return false; 
        
             } 
        
             return true; 
        
           }

react-spectrum/packages/@react-aria/interactions/src/usePress.ts

Lines 904 to 909 in d80999e

    
           function isValidInputKey(target: HTMLInputElement, key: string) { 
        
             // Only space should toggle checkboxes and radios, not enter. 
        
             return target.type === 'checkbox' || target.type === 'radio' 
        
               ? key === ' ' 
        
               : nonTextInputTypes.has(target.type); 
        
           }

Lastly, I think we shouldn't pass pressProps, which is baked-in in inputProps returned from useToggle() to the visually hidden <input> element in a RAC Checkbox since pressProps contains interaction-related event handlers, and said <input> in the RAC Checkbox is not supposed to be interacted with (not sure how to efficiently filter out those props though..)

from react-spectrum.

[RAC] Checkbox should not be toggled with 'Enter' key about react-spectrum HOT 2 OPEN

Comments (2)

Related Issues (20)

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent

	function isValidKeyboardEvent(event: KeyboardEvent, currentTarget: Element): boolean {
	const {key, code} = event;
	const element = currentTarget as HTMLElement;
	const role = element.getAttribute('role');
	// Accessibility for keyboards. Space and Enter only.
	// "Spacebar" is for IE 11
	return (
	(key === 'Enter' \|\| key === ' ' \|\| key === 'Spacebar' \|\| code === 'Space') &&
	!((element instanceof getOwnerWindow(element).HTMLInputElement && !isValidInputKey(element, key)) \|\|
	element instanceof getOwnerWindow(element).HTMLTextAreaElement \|\|
	element.isContentEditable) &&
	// Links should only trigger with Enter key
	!((role === 'link' \|\| (!role && isHTMLAnchorLink(element))) && key !== 'Enter')
	);
	}

	function shouldPreventDefaultKeyboard(target: Element, key: string) {
	if (target instanceof HTMLInputElement) {
	return !isValidInputKey(target, key);
	}

	if (target instanceof HTMLButtonElement) {
	return target.type !== 'submit' && target.type !== 'reset';
	}

	if (isHTMLAnchorLink(target)) {
	return false;
	}

	return true;
	}

	function isValidInputKey(target: HTMLInputElement, key: string) {
	// Only space should toggle checkboxes and radios, not enter.
	return target.type === 'checkbox' \|\| target.type === 'radio'
	? key === ' '
	: nonTextInputTypes.has(target.type);
	}