Skip to content

Commit c607312

Browse files
committed
[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.
1 parent 55576f3 commit c607312

File tree

7 files changed

+58
-70
lines changed

7 files changed

+58
-70
lines changed

packages/react-native-web/src/exports/Text/index.js

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ const Text = forwardRef<TextProps, *>((props, ref) => {
3434
nativeID,
3535
numberOfLines,
3636
onBlur,
37+
onClick,
3738
onContextMenu,
3839
onFocus,
3940
onLayout,
@@ -127,19 +128,14 @@ const Text = forwardRef<TextProps, *>((props, ref) => {
127128
onStartShouldSetResponderCapture
128129
});
129130

130-
function createEnterHandler(fn) {
131-
return e => {
132-
if (e.keyCode === 13) {
133-
fn && fn(e);
134-
}
135-
};
136-
}
137-
138-
function createPressHandler(fn) {
139-
return e => {
131+
function handleClick(e) {
132+
if (onClick != null) {
133+
onClick(e);
134+
}
135+
if (onClick == null && onPress != null) {
140136
e.stopPropagation();
141-
fn && fn(e);
142-
};
137+
onPress(e);
138+
}
143139
}
144140

145141
const component = hasTextAncestor ? 'span' : 'div';
@@ -157,14 +153,13 @@ const Text = forwardRef<TextProps, *>((props, ref) => {
157153
importantForAccessibility,
158154
nativeID,
159155
onBlur,
156+
onClick: handleClick,
160157
onContextMenu,
161158
onFocus,
162159
ref: setRef,
163160
style,
164161
testID,
165162
// unstable
166-
onClick: onPress != null ? createPressHandler(onPress) : null,
167-
onKeyDown: onPress != null ? createEnterHandler(onPress) : null,
168163
onMouseDown,
169164
onMouseEnter,
170165
onMouseLeave,

packages/react-native-web/src/exports/Text/types.js

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ export type TextProps = {
103103
nativeID?: ?string,
104104
numberOfLines?: ?number,
105105
onBlur?: (e: any) => void,
106+
onClick?: (e: any) => void,
107+
onContextMenu?: (e: any) => void,
106108
onFocus?: (e: any) => void,
107109
onLayout?: (e: LayoutEvent) => void,
108110
onPress?: (e: any) => void,
@@ -126,10 +128,6 @@ export type TextProps = {
126128
style?: GenericStyleProp<TextStyle>,
127129
testID?: ?string,
128130
// unstable
129-
onContextMenu?: (e: any) => void,
130-
onKeyDown?: (e: any) => void,
131-
onKeyPress?: (e: any) => void,
132-
onKeyUp?: (e: any) => void,
133131
onMouseDown?: (e: any) => void,
134132
onMouseEnter?: (e: any) => void,
135133
onMouseLeave?: (e: any) => void,

packages/react-native-web/src/exports/View/index.js

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,13 @@ const View = forwardRef<ViewProps, *>((props, ref) => {
4444
accessibilityRole,
4545
accessibilityState,
4646
accessibilityValue,
47+
accessible,
4748
forwardedRef,
4849
hitSlop,
4950
importantForAccessibility,
5051
nativeID,
5152
onBlur,
53+
onClick,
5254
onContextMenu,
5355
onFocus,
5456
onLayout,
@@ -71,12 +73,7 @@ const View = forwardRef<ViewProps, *>((props, ref) => {
7173
pointerEvents,
7274
testID,
7375
// unstable
74-
onClick,
75-
onClickCapture,
76-
onScroll,
77-
onWheel,
7876
onKeyDown,
79-
onKeyPress,
8077
onKeyUp,
8178
onMouseDown,
8279
onMouseEnter,
@@ -85,6 +82,7 @@ const View = forwardRef<ViewProps, *>((props, ref) => {
8582
onMouseOver,
8683
onMouseOut,
8784
onMouseUp,
85+
onScroll,
8886
onTouchCancel,
8987
onTouchCancelCapture,
9088
onTouchEnd,
@@ -93,6 +91,7 @@ const View = forwardRef<ViewProps, *>((props, ref) => {
9391
onTouchMoveCapture,
9492
onTouchStart,
9593
onTouchStartCapture,
94+
onWheel,
9695
href,
9796
itemID,
9897
itemRef,
@@ -159,24 +158,21 @@ const View = forwardRef<ViewProps, *>((props, ref) => {
159158
accessibilityRole,
160159
accessibilityState,
161160
accessibilityValue,
161+
accessible,
162162
children,
163163
classList,
164164
importantForAccessibility,
165165
nativeID,
166166
onBlur,
167+
onClick,
167168
onContextMenu,
168169
onFocus,
169170
pointerEvents,
170171
ref: setRef,
171172
style,
172173
testID,
173174
// unstable
174-
onClick,
175-
onClickCapture,
176-
onScroll,
177-
onWheel,
178175
onKeyDown,
179-
onKeyPress,
180176
onKeyUp,
181177
onMouseDown,
182178
onMouseEnter,
@@ -185,6 +181,7 @@ const View = forwardRef<ViewProps, *>((props, ref) => {
185181
onMouseOver,
186182
onMouseOut,
187183
onMouseUp,
184+
onScroll,
188185
onTouchCancel,
189186
onTouchCancelCapture,
190187
onTouchEnd,
@@ -193,6 +190,7 @@ const View = forwardRef<ViewProps, *>((props, ref) => {
193190
onTouchMoveCapture,
194191
onTouchStart,
195192
onTouchStartCapture,
193+
onWheel,
196194
href,
197195
itemID,
198196
itemRef,

packages/react-native-web/src/exports/View/types.js

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ export type ViewProps = {
9898
importantForAccessibility?: 'auto' | 'yes' | 'no' | 'no-hide-descendants',
9999
nativeID?: ?string,
100100
onBlur?: (e: any) => void,
101+
onClick?: (e: any) => void,
102+
onContextMenu?: (e: any) => void,
101103
onFocus?: (e: any) => void,
102104
onLayout?: (e: LayoutEvent) => void,
103105
onMoveShouldSetResponder?: (e: any) => boolean,
@@ -120,13 +122,7 @@ export type ViewProps = {
120122
style?: GenericStyleProp<ViewStyle>,
121123
testID?: ?string,
122124
// unstable
123-
onClick?: (e: any) => void,
124-
onClickCapture?: (e: any) => void,
125-
onContextMenu?: (e: any) => void,
126-
onScroll?: (e: any) => void,
127-
onWheel?: (e: any) => void,
128125
onKeyDown?: (e: any) => void,
129-
onKeyPress?: (e: any) => void,
130126
onKeyUp?: (e: any) => void,
131127
onMouseDown?: (e: any) => void,
132128
onMouseEnter?: (e: any) => void,
@@ -135,6 +131,7 @@ export type ViewProps = {
135131
onMouseOver?: (e: any) => void,
136132
onMouseOut?: (e: any) => void,
137133
onMouseUp?: (e: any) => void,
134+
onScroll?: (e: any) => void,
138135
onTouchCancel?: (e: any) => void,
139136
onTouchCancelCapture?: (e: any) => void,
140137
onTouchEnd?: (e: any) => void,
@@ -143,6 +140,7 @@ export type ViewProps = {
143140
onTouchMoveCapture?: (e: any) => void,
144141
onTouchStart?: (e: any) => void,
145142
onTouchStartCapture?: (e: any) => void,
143+
onWheel?: (e: any) => void,
146144
href?: ?string,
147145
itemID?: ?string,
148146
itemRef?: ?string,

packages/react-native-web/src/exports/createElement/__tests__/index-test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ describe('modules/createElement', () => {
2525
});
2626

2727
const testRole = ({ accessibilityRole, disabled }) => {
28-
[{ key: 'Enter', which: 13 }, { key: 'Space', which: 32 }].forEach(({ key, which }) => {
28+
[{ key: 'Enter' }, { key: ' ' }].forEach(({ key }) => {
2929
test(`"onClick" is ${disabled ? 'not ' : ''}called when "${key}" key is pressed`, () => {
3030
const onClick = jest.fn();
3131
const component = shallow(
@@ -35,7 +35,7 @@ describe('modules/createElement', () => {
3535
isDefaultPrevented() {},
3636
nativeEvent: {},
3737
preventDefault() {},
38-
which
38+
key
3939
});
4040
expect(onClick).toHaveBeenCalledTimes(disabled ? 0 : 1);
4141
});

packages/react-native-web/src/exports/createElement/index.js

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,39 +11,15 @@ import AccessibilityUtil from '../../modules/AccessibilityUtil';
1111
import createDOMProps from '../../modules/createDOMProps';
1212
import React from 'react';
1313

14-
const adjustProps = domProps => {
15-
const { onClick, role } = domProps;
16-
17-
const isButtonLikeRole = AccessibilityUtil.buttonLikeRoles[role];
18-
const isDisabled = AccessibilityUtil.isDisabled(domProps);
19-
20-
// Button-like roles should not trigger 'onClick' if they are disabled.
21-
if (isButtonLikeRole && isDisabled && domProps.onClick != null) {
22-
domProps.onClick = undefined;
23-
}
24-
25-
// Button-like roles should trigger 'onClick' if SPACE or ENTER keys are pressed.
26-
if (isButtonLikeRole && !isDisabled) {
27-
domProps.onKeyPress = function(e) {
28-
if (!e.isDefaultPrevented() && (e.which === 13 || e.which === 32)) {
29-
e.preventDefault();
30-
if (onClick) {
31-
onClick(e);
32-
}
33-
}
34-
};
35-
}
36-
};
37-
3814
const createElement = (component, props, ...children) => {
39-
// use equivalent platform elements where possible
15+
// Use equivalent platform elements where possible.
4016
let accessibilityComponent;
4117
if (component && component.constructor === String) {
4218
accessibilityComponent = AccessibilityUtil.propsToAccessibilityComponent(props);
4319
}
4420
const Component = accessibilityComponent || component;
4521
const domProps = createDOMProps(Component, props);
46-
adjustProps(domProps);
22+
4723
return React.createElement(Component, domProps, ...children);
4824
};
4925

packages/react-native-web/src/modules/createDOMProps/index.js

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ const createDOMProps = (component, props, styleResolver) => {
6767
accessibilityRelationship,
6868
accessibilityState,
6969
accessibilityValue,
70+
accessible,
7071
classList,
7172
disabled: providedDisabled,
7273
importantForAccessibility,
@@ -75,7 +76,6 @@ const createDOMProps = (component, props, styleResolver) => {
7576
style: providedStyle,
7677
testID,
7778
/* eslint-disable */
78-
accessible,
7979
accessibilityRole,
8080
/* eslint-enable */
8181
unstable_ariaSet,
@@ -176,18 +176,18 @@ const createDOMProps = (component, props, styleResolver) => {
176176
// FOCUS
177177
// Assume that 'link' is focusable by default (uses <a>).
178178
// Assume that 'button' is not (uses <div role='button'>) but must be treated as such.
179-
const focusable =
180-
!disabled &&
181-
importantForAccessibility !== 'no' &&
182-
importantForAccessibility !== 'no-hide-descendants';
183-
if (
179+
const isInteractiveElement =
184180
role === 'link' ||
185181
component === 'a' ||
186182
component === 'button' ||
187183
component === 'input' ||
188184
component === 'select' ||
189-
component === 'textarea'
190-
) {
185+
component === 'textarea';
186+
const focusable =
187+
!disabled &&
188+
importantForAccessibility !== 'no' &&
189+
importantForAccessibility !== 'no-hide-descendants';
190+
if (isInteractiveElement) {
191191
if (accessible === false || !focusable) {
192192
domProps.tabIndex = '-1';
193193
} else {
@@ -251,6 +251,29 @@ const createDOMProps = (component, props, styleResolver) => {
251251
domProps['data-testid'] = testID;
252252
}
253253

254+
// Keyboard accessibility
255+
// Button-like roles should trigger 'onClick' if SPACE or ENTER keys are pressed.
256+
// Button-like roles should not trigger 'onClick' if they are disabled.
257+
if (domProps['data-focusable']) {
258+
const onClick = domProps.onClick;
259+
if (onClick != null) {
260+
if (disabled) {
261+
domProps.onClick = undefined;
262+
} else if (!isInteractiveElement) {
263+
const onKeyDown = domProps.onKeyDown;
264+
domProps.onKeyDown = function(e) {
265+
if (!e.isDefaultPrevented() && (e.key === 'Enter' || e.key === ' ')) {
266+
e.preventDefault();
267+
if (onKeyDown != null) {
268+
onKeyDown(e);
269+
}
270+
onClick(e);
271+
}
272+
};
273+
}
274+
}
275+
}
276+
254277
return domProps;
255278
};
256279

0 commit comments

Comments
 (0)