Skip to content

Commit

Permalink
fix: correct the way how disabling touchable is handled (#3803)
Browse files Browse the repository at this point in the history
  • Loading branch information
lukewalczak authored Apr 17, 2023
1 parent 9337d33 commit 0ed4dc4
Show file tree
Hide file tree
Showing 9 changed files with 162 additions and 11 deletions.
12 changes: 10 additions & 2 deletions src/components/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import color from 'color';

import { useInternalTheme } from '../../core/theming';
import type { $Omit, ThemeProp } from '../../types';
import hasTouchHandler from '../../utils/hasTouchHandler';
import ActivityIndicator from '../ActivityIndicator';
import Icon, { IconSource } from '../Icon';
import Surface from '../Surface';
Expand Down Expand Up @@ -197,6 +198,13 @@ const Button = ({
const { roundness, isV3, animation } = theme;
const uppercase = uppercaseProp ?? !theme.isV3;

const hasPassedTouchHandler = hasTouchHandler({
onPress,
onPressIn,
onPressOut,
onLongPress,
});

const isElevationEntitled =
!disabled && (isV3 ? isMode('elevated') : isMode('contained'));
const initialElevation = isV3 ? 1 : 2;
Expand Down Expand Up @@ -308,8 +316,8 @@ const Button = ({
borderless
onPress={onPress}
onLongPress={onLongPress}
onPressIn={handlePressIn}
onPressOut={handlePressOut}
onPressIn={hasPassedTouchHandler ? handlePressIn : undefined}
onPressOut={hasPassedTouchHandler ? handlePressOut : undefined}
delayLongPress={delayLongPress}
accessibilityLabel={accessibilityLabel}
accessibilityHint={accessibilityHint}
Expand Down
8 changes: 7 additions & 1 deletion src/components/Card/Card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import useLatestCallback from 'use-latest-callback';

import { useInternalTheme } from '../../core/theming';
import type { $Omit, ThemeProp } from '../../types';
import hasTouchHandler from '../../utils/hasTouchHandler';
import Surface from '../Surface';
import CardActions from './CardActions';
import CardContent from './CardContent';
Expand Down Expand Up @@ -158,6 +159,11 @@ const Card = ({
[cardMode]
);

const hasPassedTouchHandler = hasTouchHandler({
onPress,
onLongPress,
});

// Default animated value
const { current: elevation } = React.useRef<Animated.Value>(
new Animated.Value(cardElevation)
Expand Down Expand Up @@ -295,7 +301,7 @@ const Card = ({
/>
)}

{onPress || onLongPress ? (
{hasPassedTouchHandler ? (
<TouchableWithoutFeedback
delayPressIn={0}
disabled={disabled}
Expand Down
7 changes: 5 additions & 2 deletions src/components/Chip/Chip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
import { useInternalTheme } from '../../core/theming';
import { white } from '../../styles/themes/v2/colors';
import type { $Omit, EllipsizeProp, ThemeProp } from '../../types';
import hasTouchHandler from '../../utils/hasTouchHandler';
import type { IconSource } from '../Icon';
import Icon from '../Icon';
import MaterialCommunityIcon from '../MaterialCommunityIcon';
Expand Down Expand Up @@ -177,6 +178,8 @@ const Chip = ({
new Animated.Value(isV3 && elevated ? 1 : 0)
);

const hasPassedTouchHandler = hasTouchHandler({ onPress, onLongPress });

const isOutlined = mode === 'outlined';

const handlePressIn = () => {
Expand Down Expand Up @@ -265,8 +268,8 @@ const Chip = ({
borderless
style={[{ borderRadius }, styles.touchable]}
onPress={onPress}
onPressIn={handlePressIn}
onPressOut={handlePressOut}
onPressIn={hasPassedTouchHandler ? handlePressIn : undefined}
onPressOut={hasPassedTouchHandler ? handlePressOut : undefined}
onLongPress={onLongPress}
delayLongPress={delayLongPress}
underlayColor={underlayColor}
Expand Down
19 changes: 16 additions & 3 deletions src/components/TouchableRipple/TouchableRipple.native.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {

import { useInternalTheme } from '../../core/theming';
import type { ThemeProp } from '../../types';
import hasTouchHandler from '../../utils/hasTouchHandler';
import { getTouchableRippleColors } from './utils';

const ANDROID_VERSION_LOLLIPOP = 21;
Expand All @@ -23,6 +24,8 @@ export type Props = React.ComponentProps<typeof Pressable> & {
disabled?: boolean;
onPress?: (e: GestureResponderEvent) => void | null;
onLongPress?: (e: GestureResponderEvent) => void;
onPressIn?: (e: GestureResponderEvent) => void;
onPressOut?: (e: GestureResponderEvent) => void;
rippleColor?: string;
underlayColor?: string;
children: React.ReactNode;
Expand All @@ -44,7 +47,17 @@ const TouchableRipple = ({
const theme = useInternalTheme(themeOverrides);
const [showUnderlay, setShowUnderlay] = React.useState<boolean>(false);

const disabled = disabledProp || !rest.onPress;
const { onPress, onLongPress, onPressIn, onPressOut } = rest;

const hasPassedTouchHandler = hasTouchHandler({
onPress,
onLongPress,
onPressIn,
onPressOut,
});

const disabled = disabledProp || !hasPassedTouchHandler;

const { calculatedRippleColor, calculatedUnderlayColor } =
getTouchableRippleColors({
theme,
Expand All @@ -61,12 +74,12 @@ const TouchableRipple = ({

const handlePressIn = (e: GestureResponderEvent) => {
setShowUnderlay(true);
rest.onPressIn?.(e);
onPressIn?.(e);
};

const handlePressOut = (e: GestureResponderEvent) => {
setShowUnderlay(false);
rest.onPressOut?.(e);
onPressOut?.(e);
};

if (TouchableRipple.supported) {
Expand Down
24 changes: 21 additions & 3 deletions src/components/TouchableRipple/TouchableRipple.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {

import { useInternalTheme } from '../../core/theming';
import type { ThemeProp } from '../../types';
import hasTouchHandler from '../../utils/hasTouchHandler';
import { getTouchableRippleColors } from './utils';

export type Props = React.ComponentPropsWithRef<typeof Pressable> & {
Expand Down Expand Up @@ -38,6 +39,14 @@ export type Props = React.ComponentPropsWithRef<typeof Pressable> & {
* Function to execute on long press.
*/
onLongPress?: (e: GestureResponderEvent) => void;
/**
* Function to execute immediately when a touch is engaged, before `onPressOut` and `onPress`.
*/
onPressIn?: (e: GestureResponderEvent) => void;
/**
* Function to execute when a touch is released.
*/
onPressOut?: (e: GestureResponderEvent) => void;
/**
* Color of the ripple effect (Android >= 5.0 and Web).
*/
Expand Down Expand Up @@ -100,8 +109,10 @@ const TouchableRipple = ({
...rest
}: Props) => {
const theme = useInternalTheme(themeOverrides);
const { onPress, onLongPress, onPressIn, onPressOut } = rest;

const handlePressIn = (e: any) => {
const { centered, onPressIn } = rest;
const { centered } = rest;

onPressIn?.(e);

Expand Down Expand Up @@ -199,7 +210,7 @@ const TouchableRipple = ({
};

const handlePressOut = (e: any) => {
rest.onPressOut?.(e);
onPressOut?.(e);

const containers = e.currentTarget.querySelectorAll(
'[data-paper-ripple]'
Expand Down Expand Up @@ -228,7 +239,14 @@ const TouchableRipple = ({
});
};

const disabled = disabledProp || !rest.onPress;
const hasPassedTouchHandler = hasTouchHandler({
onPress,
onLongPress,
onPressIn,
onPressOut,
});

const disabled = disabledProp || !hasPassedTouchHandler;

return (
<Pressable
Expand Down
24 changes: 24 additions & 0 deletions src/components/__tests__/Button.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,30 @@ it('renders disabled button', () => {
expect(tree).toMatchSnapshot();
});

it('renders disabled button if there is no touch handler passed', () => {
const { getByTestId } = render(
<Button testID="disabled-button">Disabled button</Button>
);

expect(getByTestId('disabled-button').props.accessibilityState).toMatchObject(
{
disabled: true,
}
);
});

it('renders active button if only onLongPress handler is passed', () => {
const { getByTestId } = render(
<Button onLongPress={() => {}} testID="active-button">
Active button
</Button>
);

expect(getByTestId('active-button').props.accessibilityState).toMatchObject({
disabled: false,
});
});

it('renders button with color', () => {
const tree = render(
<Button textColor={pink500}>Custom Button</Button>
Expand Down
22 changes: 22 additions & 0 deletions src/components/__tests__/Chip.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,28 @@ it('renders selected chip', () => {
expect(tree).toMatchSnapshot();
});

it('renders disabled chip if there is no touch handler passed', () => {
const { getByTestId } = render(
<Chip testID="disabled-chip">Disabled chip</Chip>
);

expect(getByTestId('disabled-chip').props.accessibilityState).toMatchObject({
disabled: true,
});
});

it('renders active chip if only onLongPress handler is passed', () => {
const { getByTestId } = render(
<Chip onLongPress={() => {}} testID="active-chip">
Active chip
</Chip>
);

expect(getByTestId('active-chip').props.accessibilityState).toMatchObject({
disabled: false,
});
});

describe('getChipColors - text color', () => {
it('should return correct disabled color, for theme version 3', () => {
expect(
Expand Down
37 changes: 37 additions & 0 deletions src/utils/__tests__/hasTouchHandler.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import hasTouchHandler from '../hasTouchHandler';

describe('hasTouchHandler', () => {
it.each([
[{ onPress: jest.fn() }],
[{ onLongPress: jest.fn() }],
[{ onPressIn: jest.fn() }],
[{ onPressOut: jest.fn() }],
])(
'should return true if touchableEventObject contains touchable event %p',
(touchableEventObject) => {
const result = hasTouchHandler(touchableEventObject);

expect(result).toBe(true);
}
);

it('should return true if two touchable events are passed, but one is undefined', () => {
const result = hasTouchHandler({
onPress: jest.fn(),
onLongPress: undefined,
});

expect(result).toBe(true);
});

it('should return false if touchable events are undefined', () => {
const result = hasTouchHandler({
onPress: undefined,
onLongPress: undefined,
onPressIn: undefined,
onPressOut: undefined,
});

expect(result).toBe(false);
});
});
20 changes: 20 additions & 0 deletions src/utils/hasTouchHandler.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import type { GestureResponderEvent } from 'react-native';

const touchableEvents = [
'onPress',
'onLongPress',
'onPressIn',
'onPressOut',
] as const;

type TouchableEventObject = Partial<
Record<typeof touchableEvents[number], (event: GestureResponderEvent) => void>
>;

export default function hasTouchHandler(
touchableEventObject: TouchableEventObject
) {
return touchableEvents.some((event) => {
return Boolean(touchableEventObject[event]);
});
}

0 comments on commit 0ed4dc4

Please sign in to comment.