[change] Text and View onClick handling

Makes `onClick` part of the stable props API. In the future this will be used
to implement `onPress` in the Touchables/Pressables. Special handling of click
for keyboards is performed in `createElement`. At the moment, `Text` still
includes the `onPress` prop, which will only be called if `onClick` is not also
being used. In the future `Text` (in React Native) should remove the Touchable
props from its API.
This commit is contained in:
Nicolas Gallagher
2020-03-31 14:06:01 -07:00
parent 66751502a3
commit 204c432f66
7 changed files with 58 additions and 70 deletions
+9 -14
View File
@@ -35,6 +35,7 @@ const Text = forwardRef<TextProps, *>((props, ref) => {
nativeID, nativeID,
numberOfLines, numberOfLines,
onBlur, onBlur,
onClick,
onContextMenu, onContextMenu,
onFocus, onFocus,
onLayout, onLayout,
@@ -128,19 +129,14 @@ const Text = forwardRef<TextProps, *>((props, ref) => {
onStartShouldSetResponderCapture onStartShouldSetResponderCapture
}); });
function createEnterHandler(fn) { function handleClick(e) {
return e => { if (onClick != null) {
if (e.keyCode === 13) { onClick(e);
fn && fn(e); }
} if (onClick == null && onPress != null) {
};
}
function createPressHandler(fn) {
return e => {
e.stopPropagation(); e.stopPropagation();
fn && fn(e); onPress(e);
}; }
} }
const component = hasTextAncestor ? 'span' : 'div'; const component = hasTextAncestor ? 'span' : 'div';
@@ -159,14 +155,13 @@ const Text = forwardRef<TextProps, *>((props, ref) => {
lang, lang,
nativeID, nativeID,
onBlur, onBlur,
onClick: handleClick,
onContextMenu, onContextMenu,
onFocus, onFocus,
ref: setRef, ref: setRef,
style, style,
testID, testID,
// unstable // unstable
onClick: onPress != null ? createPressHandler(onPress) : null,
onKeyDown: onPress != null ? createEnterHandler(onPress) : null,
onMouseDown, onMouseDown,
onMouseEnter, onMouseEnter,
onMouseLeave, onMouseLeave,
+2 -4
View File
@@ -103,6 +103,8 @@ export type TextProps = {
nativeID?: ?string, nativeID?: ?string,
numberOfLines?: ?number, numberOfLines?: ?number,
onBlur?: (e: any) => void, onBlur?: (e: any) => void,
onClick?: (e: any) => void,
onContextMenu?: (e: any) => void,
onFocus?: (e: any) => void, onFocus?: (e: any) => void,
onLayout?: (e: LayoutEvent) => void, onLayout?: (e: LayoutEvent) => void,
onPress?: (e: any) => void, onPress?: (e: any) => void,
@@ -126,10 +128,6 @@ export type TextProps = {
style?: GenericStyleProp<TextStyle>, style?: GenericStyleProp<TextStyle>,
testID?: ?string, testID?: ?string,
// unstable // unstable
onContextMenu?: (e: any) => void,
onKeyDown?: (e: any) => void,
onKeyPress?: (e: any) => void,
onKeyUp?: (e: any) => void,
onMouseDown?: (e: any) => void, onMouseDown?: (e: any) => void,
onMouseEnter?: (e: any) => void, onMouseEnter?: (e: any) => void,
onMouseLeave?: (e: any) => void, onMouseLeave?: (e: any) => void,
+8 -10
View File
@@ -44,11 +44,13 @@ const View = forwardRef<ViewProps, *>((props, ref) => {
accessibilityRole, accessibilityRole,
accessibilityState, accessibilityState,
accessibilityValue, accessibilityValue,
accessible,
forwardedRef, forwardedRef,
hitSlop, hitSlop,
importantForAccessibility, importantForAccessibility,
nativeID, nativeID,
onBlur, onBlur,
onClick,
onContextMenu, onContextMenu,
onFocus, onFocus,
onLayout, onLayout,
@@ -71,12 +73,7 @@ const View = forwardRef<ViewProps, *>((props, ref) => {
pointerEvents, pointerEvents,
testID, testID,
// unstable // unstable
onClick,
onClickCapture,
onScroll,
onWheel,
onKeyDown, onKeyDown,
onKeyPress,
onKeyUp, onKeyUp,
onMouseDown, onMouseDown,
onMouseEnter, onMouseEnter,
@@ -85,6 +82,7 @@ const View = forwardRef<ViewProps, *>((props, ref) => {
onMouseOver, onMouseOver,
onMouseOut, onMouseOut,
onMouseUp, onMouseUp,
onScroll,
onTouchCancel, onTouchCancel,
onTouchCancelCapture, onTouchCancelCapture,
onTouchEnd, onTouchEnd,
@@ -93,6 +91,7 @@ const View = forwardRef<ViewProps, *>((props, ref) => {
onTouchMoveCapture, onTouchMoveCapture,
onTouchStart, onTouchStart,
onTouchStartCapture, onTouchStartCapture,
onWheel,
href, href,
itemID, itemID,
itemRef, itemRef,
@@ -159,11 +158,13 @@ const View = forwardRef<ViewProps, *>((props, ref) => {
accessibilityRole, accessibilityRole,
accessibilityState, accessibilityState,
accessibilityValue, accessibilityValue,
accessible,
children, children,
classList, classList,
importantForAccessibility, importantForAccessibility,
nativeID, nativeID,
onBlur, onBlur,
onClick,
onContextMenu, onContextMenu,
onFocus, onFocus,
pointerEvents, pointerEvents,
@@ -171,12 +172,7 @@ const View = forwardRef<ViewProps, *>((props, ref) => {
style, style,
testID, testID,
// unstable // unstable
onClick,
onClickCapture,
onScroll,
onWheel,
onKeyDown, onKeyDown,
onKeyPress,
onKeyUp, onKeyUp,
onMouseDown, onMouseDown,
onMouseEnter, onMouseEnter,
@@ -185,6 +181,7 @@ const View = forwardRef<ViewProps, *>((props, ref) => {
onMouseOver, onMouseOver,
onMouseOut, onMouseOut,
onMouseUp, onMouseUp,
onScroll,
onTouchCancel, onTouchCancel,
onTouchCancelCapture, onTouchCancelCapture,
onTouchEnd, onTouchEnd,
@@ -193,6 +190,7 @@ const View = forwardRef<ViewProps, *>((props, ref) => {
onTouchMoveCapture, onTouchMoveCapture,
onTouchStart, onTouchStart,
onTouchStartCapture, onTouchStartCapture,
onWheel,
href, href,
itemID, itemID,
itemRef, itemRef,
+4 -6
View File
@@ -98,6 +98,8 @@ export type ViewProps = {
importantForAccessibility?: 'auto' | 'yes' | 'no' | 'no-hide-descendants', importantForAccessibility?: 'auto' | 'yes' | 'no' | 'no-hide-descendants',
nativeID?: ?string, nativeID?: ?string,
onBlur?: (e: any) => void, onBlur?: (e: any) => void,
onClick?: (e: any) => void,
onContextMenu?: (e: any) => void,
onFocus?: (e: any) => void, onFocus?: (e: any) => void,
onLayout?: (e: LayoutEvent) => void, onLayout?: (e: LayoutEvent) => void,
onMoveShouldSetResponder?: (e: any) => boolean, onMoveShouldSetResponder?: (e: any) => boolean,
@@ -120,13 +122,7 @@ export type ViewProps = {
style?: GenericStyleProp<ViewStyle>, style?: GenericStyleProp<ViewStyle>,
testID?: ?string, testID?: ?string,
// unstable // unstable
onClick?: (e: any) => void,
onClickCapture?: (e: any) => void,
onContextMenu?: (e: any) => void,
onScroll?: (e: any) => void,
onWheel?: (e: any) => void,
onKeyDown?: (e: any) => void, onKeyDown?: (e: any) => void,
onKeyPress?: (e: any) => void,
onKeyUp?: (e: any) => void, onKeyUp?: (e: any) => void,
onMouseDown?: (e: any) => void, onMouseDown?: (e: any) => void,
onMouseEnter?: (e: any) => void, onMouseEnter?: (e: any) => void,
@@ -135,6 +131,7 @@ export type ViewProps = {
onMouseOver?: (e: any) => void, onMouseOver?: (e: any) => void,
onMouseOut?: (e: any) => void, onMouseOut?: (e: any) => void,
onMouseUp?: (e: any) => void, onMouseUp?: (e: any) => void,
onScroll?: (e: any) => void,
onTouchCancel?: (e: any) => void, onTouchCancel?: (e: any) => void,
onTouchCancelCapture?: (e: any) => void, onTouchCancelCapture?: (e: any) => void,
onTouchEnd?: (e: any) => void, onTouchEnd?: (e: any) => void,
@@ -143,6 +140,7 @@ export type ViewProps = {
onTouchMoveCapture?: (e: any) => void, onTouchMoveCapture?: (e: any) => void,
onTouchStart?: (e: any) => void, onTouchStart?: (e: any) => void,
onTouchStartCapture?: (e: any) => void, onTouchStartCapture?: (e: any) => void,
onWheel?: (e: any) => void,
href?: ?string, href?: ?string,
itemID?: ?string, itemID?: ?string,
itemRef?: ?string, itemRef?: ?string,
@@ -25,7 +25,7 @@ describe('modules/createElement', () => {
}); });
const testRole = ({ accessibilityRole, disabled }) => { const testRole = ({ accessibilityRole, disabled }) => {
[{ key: 'Enter', which: 13 }, { key: 'Space', which: 32 }].forEach(({ key, which }) => { [{ key: 'Enter' }, { key: ' ' }].forEach(({ key }) => {
test(`"onClick" is ${disabled ? 'not ' : ''}called when "${key}" key is pressed`, () => { test(`"onClick" is ${disabled ? 'not ' : ''}called when "${key}" key is pressed`, () => {
const onClick = jest.fn(); const onClick = jest.fn();
const component = shallow( const component = shallow(
@@ -35,7 +35,7 @@ describe('modules/createElement', () => {
isDefaultPrevented() {}, isDefaultPrevented() {},
nativeEvent: {}, nativeEvent: {},
preventDefault() {}, preventDefault() {},
which key
}); });
expect(onClick).toHaveBeenCalledTimes(disabled ? 0 : 1); expect(onClick).toHaveBeenCalledTimes(disabled ? 0 : 1);
}); });
@@ -11,39 +11,15 @@ import AccessibilityUtil from '../../modules/AccessibilityUtil';
import createDOMProps from '../../modules/createDOMProps'; import createDOMProps from '../../modules/createDOMProps';
import React from 'react'; import React from 'react';
const adjustProps = domProps => {
const { onClick, role } = domProps;
const isButtonLikeRole = AccessibilityUtil.buttonLikeRoles[role];
const isDisabled = AccessibilityUtil.isDisabled(domProps);
// Button-like roles should not trigger 'onClick' if they are disabled.
if (isButtonLikeRole && isDisabled && domProps.onClick != null) {
domProps.onClick = undefined;
}
// Button-like roles should trigger 'onClick' if SPACE or ENTER keys are pressed.
if (isButtonLikeRole && !isDisabled) {
domProps.onKeyPress = function(e) {
if (!e.isDefaultPrevented() && (e.which === 13 || e.which === 32)) {
e.preventDefault();
if (onClick) {
onClick(e);
}
}
};
}
};
const createElement = (component, props, ...children) => { const createElement = (component, props, ...children) => {
// use equivalent platform elements where possible // Use equivalent platform elements where possible.
let accessibilityComponent; let accessibilityComponent;
if (component && component.constructor === String) { if (component && component.constructor === String) {
accessibilityComponent = AccessibilityUtil.propsToAccessibilityComponent(props); accessibilityComponent = AccessibilityUtil.propsToAccessibilityComponent(props);
} }
const Component = accessibilityComponent || component; const Component = accessibilityComponent || component;
const domProps = createDOMProps(Component, props); const domProps = createDOMProps(Component, props);
adjustProps(domProps);
return React.createElement(Component, domProps, ...children); return React.createElement(Component, domProps, ...children);
}; };
@@ -67,6 +67,7 @@ const createDOMProps = (component, props, styleResolver) => {
accessibilityRelationship, accessibilityRelationship,
accessibilityState, accessibilityState,
accessibilityValue, accessibilityValue,
accessible,
classList, classList,
disabled: providedDisabled, disabled: providedDisabled,
importantForAccessibility, importantForAccessibility,
@@ -75,7 +76,6 @@ const createDOMProps = (component, props, styleResolver) => {
style: providedStyle, style: providedStyle,
testID, testID,
/* eslint-disable */ /* eslint-disable */
accessible,
accessibilityRole, accessibilityRole,
/* eslint-enable */ /* eslint-enable */
unstable_ariaSet, unstable_ariaSet,
@@ -176,18 +176,18 @@ const createDOMProps = (component, props, styleResolver) => {
// FOCUS // FOCUS
// Assume that 'link' is focusable by default (uses <a>). // Assume that 'link' is focusable by default (uses <a>).
// Assume that 'button' is not (uses <div role='button'>) but must be treated as such. // Assume that 'button' is not (uses <div role='button'>) but must be treated as such.
const focusable = const isInteractiveElement =
!disabled &&
importantForAccessibility !== 'no' &&
importantForAccessibility !== 'no-hide-descendants';
if (
role === 'link' || role === 'link' ||
component === 'a' || component === 'a' ||
component === 'button' || component === 'button' ||
component === 'input' || component === 'input' ||
component === 'select' || component === 'select' ||
component === 'textarea' component === 'textarea';
) { const focusable =
!disabled &&
importantForAccessibility !== 'no' &&
importantForAccessibility !== 'no-hide-descendants';
if (isInteractiveElement) {
if (accessible === false || !focusable) { if (accessible === false || !focusable) {
domProps.tabIndex = '-1'; domProps.tabIndex = '-1';
} else { } else {
@@ -251,6 +251,29 @@ const createDOMProps = (component, props, styleResolver) => {
domProps['data-testid'] = testID; domProps['data-testid'] = testID;
} }
// Keyboard accessibility
// Button-like roles should trigger 'onClick' if SPACE or ENTER keys are pressed.
// Button-like roles should not trigger 'onClick' if they are disabled.
if (domProps['data-focusable']) {
const onClick = domProps.onClick;
if (onClick != null) {
if (disabled) {
domProps.onClick = undefined;
} else if (!isInteractiveElement) {
const onKeyDown = domProps.onKeyDown;
domProps.onKeyDown = function(e) {
if (!e.isDefaultPrevented() && (e.key === 'Enter' || e.key === ' ')) {
e.preventDefault();
if (onKeyDown != null) {
onKeyDown(e);
}
onClick(e);
}
};
}
}
}
return domProps; return domProps;
}; };