Skip to content

fix: expand interactive targets to the 48dp minimum - #5080

Open
lukemorawski wants to merge 1 commit into
callstack:mainfrom
lukemorawski:feat/48dp-touch-targets
Open

fix: expand interactive targets to the 48dp minimum#5080
lukemorawski wants to merge 1 commit into
callstack:mainfrom
lukemorawski:feat/48dp-touch-targets

Conversation

@lukemorawski

Copy link
Copy Markdown
Contributor

Motivation

Touch targets matched the drawn box: Checkbox 40x40, RadioButton 32x36,
IconButton 40x40, Chip close icon 26x18.

MD3 grows the target outside the component rather than resizing it, and only when the
component is interactive, so the 40dp state layer stays 40dp and gains slop around it.

Two mechanisms, since one does not cover both platforms:

  • native: TouchableRipple measures itself with onLayout and sets hitSlop
  • web: an aria-hidden absolutely positioned child at max(48px, 100%)

react-native-web removed hitSlop in 0.13.0, so web needs its own. An absolutely
positioned child is what material-web uses. Pseudo elements are not an option, a
::before is clipped like any other child.

Measuring and applying are separate. A component that mounts disabled gets no layout
event once it is enabled, since RN does not replay one, so gating the measurement on
interactivity would leave it small permanently.

A caller hitSlop wins on both platforms.

A parent with overflow: 'hidden' clips the expanded target. IconButton has been
shipping a hitSlop that never applied for this reason. So on web the ripple is now
clipped by its own container instead of by the touchable. Output is pixel identical.

That change pulls in:

  • IconButton's Surface drops overflow: 'hidden', and the radius moves to the
    overlay and the touchable so they clip themselves. Its own hitSlop is removed.
  • the press underlay on iOS and older Android takes the touchable's radius. It was
    square, and only looked right because a parent clipped it.
  • Chip's close button fills the 34dp column the chip already reserved, rather than
    just the 26x18 icon.

Related issue

Closes #5079

Touches the same file as #5071, which splits interactive from control. No conflict,
but whichever lands second needs a look.

Test plan

Lint, typecheck and tests pass.

The suite renders an element tree with no layout and no hit testing, so it only pins
props. Checked on device by tapping inside the expected slop and again past it.

iOS 18.3 Android 15 web
Checkbox 40dp 48 48 48
RadioButton 32x36 48 48 48
IconButton 40dp 48 48 48
past the slop miss miss miss
caller hitSlop wins ok ok ok
disabled gets nothing ok ok ok

Both run Fabric, and it applies from first mount without scrolling.

On Chip, the close button takes the right 34dp and the body the rest, on all three.

Notes

Chip changes behaviour. The right 34dp fires onClose where it fired onPress.
That matches MD3, where the primary action stops where the trailing one starts.

borderless no longer clips content on web. It still clips the ripple. The touchable
cannot clip without clipping the target. Nothing in Paper depends on it, checked
across 569 touchables on 15 screens. Prop doc updated.

Mount cost is up 1.6% on device, 165ms to 167.8ms for 300 touchables, three runs each
way, dev build. Not measurable in jest.

Targets can now overlap, which is the MD3 default. On web the later sibling takes the
shared strip. They can reserve space instead if you prefer.

Comment on lines +143 to +150
// A caller hitSlop wins, so there is nothing to measure for. `null` counts as
// supplied, it means "no slop".
const shouldMeasure = hitSlop === undefined;

// Gates whether the measurement is applied, not whether it happens. RN emits
// onLayout on mount and on layout change, so a touchable that mounts disabled
// gets no event once it is enabled and would stay small.
const shouldExpand = shouldMeasure && !disabled;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we keep measuring while custom hitSlop is set?

for example:

  1. component mounts at 32×32 with hitSlop={6}.
  2. caller removes it with hitSlop={undefined}.
  3. component should now calculate { top: 8, right: 8, bottom: 8, left: 8 }.
  4. currently it cannot, because const shouldMeasure = hitSlop === undefined; (line 145) disabled measurement while hitSlop={6}.
    adding onLayout afterward doesn't trigger new event unless the layout changes (RN docs](https://reactnative.dev/docs/view.html#onlayout))

so could what about removing shouldMeasure:

Suggested change
// A caller hitSlop wins, so there is nothing to measure for. `null` counts as
// supplied, it means "no slop".
const shouldMeasure = hitSlop === undefined;
// Gates whether the measurement is applied, not whether it happens. RN emits
// onLayout on mount and on layout change, so a touchable that mounts disabled
// gets no event once it is enabled and would stay small.
const shouldExpand = shouldMeasure && !disabled;
// Gates whether the measurement is applied, not whether it happens. RN emits
// onLayout on mount and on layout change, so a touchable that mounts disabled
// gets no event once it is enabled and would stay small.
const shouldExpand = hitSlop === undefined && !disabled;

and using handleLayout only in both Pressable's:

- onLayout={shouldMeasure ? handleLayout : onLayout}
+ onLayout={handleLayout}

I guess, this should keep latest measurement ready while still letting the caller-provided hitSlop win

ref={ref}
disabled={disabled}
hitSlop={shouldExpand ? expansion : hitSlop}
onLayout={shouldMeasure ? handleLayout : onLayout}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check out this comment

Suggested change
onLayout={shouldMeasure ? handleLayout : onLayout}
onLayout={handleLayout}

ref={ref}
disabled={disabled}
hitSlop={shouldExpand ? expansion : hitSlop}
onLayout={shouldMeasure ? handleLayout : onLayout}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check out this comment

Suggested change
onLayout={shouldMeasure ? handleLayout : onLayout}
onLayout={handleLayout}

style={[
StyleSheet.absoluteFill,
{ backgroundColor, opacity: backgroundOpacity },
{ backgroundColor, opacity: backgroundOpacity, borderRadius },

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about preserving custom corner styles like borderTopLeftRadius, borderTopRightRadius etc when moving clipping from Surface to TouchableRipple?

before this PR Surface’s overflow: 'hidden' clipped the ripple with a square top-left corner.
now only borderRadius is forwarded. so could we extract every border*Radius property & apply them to both new clipping layers?

so what about replacing lines 150 - 160 with smth like that:

const flattenedStyle = (StyleSheet.flatten(style) || {}) as ViewStyle;

const {
  borderWidth = mode === 'outlined' && !selected ? 1 : 0,
} = flattenedStyle;

const [, radiusStyles] = splitStyles(
  flattenedStyle,
  (key) => key.startsWith('border') && key.endsWith('Radius')
);

const {
  borderRadius = buttonSize / 2,
  ...cornerRadiusStyles
} = radiusStyles;

const shapeStyles = {
  borderRadius,
  ...cornerRadiusStyles,
};

const borderStyles = {
  borderWidth,
  borderColor,
  ...shapeStyles,
};

and then applying shapeStyles here:

Suggested change
{ backgroundColor, opacity: backgroundOpacity, borderRadius },
{ backgroundColor, opacity: backgroundOpacity, ...shapeStyles },

and in TouchableRipple :

<TouchableRipple
        borderless
        centered
        onPress={onPress}
        aria-label={ariaLabel}
        style={[
          styles.touchable,
          shapeStyles,
          // The Surface used to clip the ripple, so the touchable does it now.
          // Native only: its own overflow does not clip its hitSlop, but on web
          // it would clip the touch target, where the container already clips.
          Platform.OS !== 'web' && styles.clipToShape,
          contentStyle,
        ]}

style={[styles.touchable, contentStyle]}
style={[
styles.touchable,
{ borderRadius },

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check out this comment

Suggested change
{ borderRadius },
shapeStyles,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Touch targets fall below the 48dp minimum, and IconButton's hitSlop never applies

2 participants