Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Porting BottomSheet accessibility from v2 to v4 #1288

Merged
merged 6 commits into from
Jan 3, 2024
Merged

Porting BottomSheet accessibility from v2 to v4 #1288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f6f52aa
Added accessibility to the bottomSheet component
Mahmoud-SK Feb 8, 2023
4144828
Accessibility changes - BottomSheetBackdrop
Mahmoud-SK Feb 8, 2023
cf21b04
Accessibility changes - BottomSheetHandle
Mahmoud-SK Feb 8, 2023
d076e97
Minor adjustments after review
Mahmoud-SK Dec 8, 2023
23d3bcd
removed unused spread operator
Mahmoud-SK Dec 8, 2023
01d49e2
fix: removed change announcement
Mahmoud-SK Jan 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/components/bottomSheet/BottomSheet.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ import {
INITIAL_CONTAINER_OFFSET,
INITIAL_VALUE,
DEFAULT_DYNAMIC_SIZING,
DEFAULT_ACCESSIBLE,
DEFAULT_ACCESSIBILITY_LABEL,
DEFAULT_ACCESSIBILITY_ROLE
} from './constants';
import type { BottomSheetMethods, Insets } from '../../types';
import type { BottomSheetProps, AnimateToPositionType } from './types';
Expand Down Expand Up @@ -159,6 +162,13 @@ const BottomSheetComponent = forwardRef<BottomSheet, BottomSheetProps>(
backgroundComponent,
footerComponent,
children: Content,

// accessibility
accessible: _providedAccessible = DEFAULT_ACCESSIBLE,
accessibilityLabel:
_providedAccessibilityLabel = DEFAULT_ACCESSIBILITY_LABEL,
accessibilityRole:
_providedAccessibilityRole = DEFAULT_ACCESSIBILITY_ROLE,
} = props;
//#endregion

Expand Down Expand Up @@ -1635,6 +1645,9 @@ const BottomSheetComponent = forwardRef<BottomSheet, BottomSheetProps>(
<Animated.View
pointerEvents="box-none"
style={contentMaskContainerStyle}
accessible={_providedAccessible ?? undefined}
accessibilityRole={_providedAccessibilityRole ?? undefined}
accessibilityLabel={_providedAccessibilityLabel ?? undefined}
>
<BottomSheetDraggableView
key="BottomSheetRootDraggableView"
Expand Down
15 changes: 15 additions & 0 deletions src/components/bottomSheet/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,15 @@ const INITIAL_CONTAINER_OFFSET = {
const INITIAL_HANDLE_HEIGHT = -999;
const INITIAL_POSITION = SCREEN_HEIGHT;

// accessibility
const DEFAULT_ACCESSIBLE = true;
const DEFAULT_ACCESSIBILITY_LABEL = 'Bottom Sheet';
Mahmoud-SK marked this conversation as resolved.
Show resolved Hide resolved
const DEFAULT_ACCESSIBILITY_ROLE = 'adjustable';
const DEFAULT_ENABLE_ACCESSIBILITY_CHANGE_ANNOUNCEMENT = true;
const DEFAULT_ACCESSIBILITY_POSITION_CHANGE_ANNOUNCEMENT = (
positionInScreen: string
) => `Bottom sheet snapped to ${positionInScreen}% of the screen`;

export {
DEFAULT_HANDLE_HEIGHT,
DEFAULT_OVER_DRAG_RESISTANCE_FACTOR,
Expand All @@ -53,4 +62,10 @@ export {
INITIAL_HANDLE_HEIGHT,
INITIAL_SNAP_POINT,
INITIAL_VALUE,
// accessibility
DEFAULT_ACCESSIBLE,
DEFAULT_ACCESSIBILITY_LABEL,
DEFAULT_ACCESSIBILITY_ROLE,
DEFAULT_ENABLE_ACCESSIBILITY_CHANGE_ANNOUNCEMENT,
DEFAULT_ACCESSIBILITY_POSITION_CHANGE_ANNOUNCEMENT,
};
8 changes: 6 additions & 2 deletions src/components/bottomSheet/types.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ import type {
KEYBOARD_BLUR_BEHAVIOR,
KEYBOARD_INPUT_MODE,
} from '../../constants';
import type { GestureEventsHandlersHookType } from '../../types';
import type {
GestureEventsHandlersHookType,
NullableAccessibilityProps,
} from '../../types';

export interface BottomSheetProps
extends BottomSheetAnimationConfigs,
Expand All @@ -31,7 +34,8 @@ export interface BottomSheetProps
| 'waitFor'
| 'simultaneousHandlers'
>
> {
>,
Omit<NullableAccessibilityProps, 'accessibilityHint'> {
//#region configuration
/**
* Initial snap point index, provide `-1` to initiate bottom sheet in closed state.
Expand Down
40 changes: 28 additions & 12 deletions src/components/bottomSheetBackdrop/BottomSheetBackdrop.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ import {
DEFAULT_DISAPPEARS_ON_INDEX,
DEFAULT_ENABLE_TOUCH_THROUGH,
DEFAULT_PRESS_BEHAVIOR,
DEFAULT_ACCESSIBLE,
DEFAULT_ACCESSIBILITY_ROLE,
DEFAULT_ACCESSIBILITY_LABEL,
DEFAULT_ACCESSIBILITY_HINT,
} from './constants';
import { styles } from './styles';
import type { BottomSheetDefaultBackdropProps } from './types';
Expand All @@ -33,6 +37,10 @@ const BottomSheetBackdropComponent = ({
onPress,
style,
children,
accessible: _providedAccessible = DEFAULT_ACCESSIBLE,
accessibilityRole: _providedAccessibilityRole = DEFAULT_ACCESSIBILITY_ROLE,
accessibilityLabel: _providedAccessibilityLabel = DEFAULT_ACCESSIBILITY_LABEL,
accessibilityHint: _providedAccessibilityHint = DEFAULT_ACCESSIBILITY_HINT,
}: BottomSheetDefaultBackdropProps) => {
//#region hooks
const { snapToIndex, close } = useBottomSheet();
Expand Down Expand Up @@ -112,27 +120,35 @@ const BottomSheetBackdropComponent = ({
},
[disappearsOnIndex]
);
//#endregion

return pressBehavior !== 'none' ? (
<TapGestureHandler onGestureEvent={gestureHandler}>
const AnimatedView = () => {
return (
<Animated.View
style={containerStyle}
pointerEvents={pointerEvents}
accessible={true}
accessibilityRole="button"
accessibilityLabel="Bottom Sheet backdrop"
accessibilityHint={`Tap to ${
typeof pressBehavior === 'string' ? pressBehavior : 'move'
} the Bottom Sheet`}
accessible={_providedAccessible ?? undefined}
Copy link

@aweary aweary Mar 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Why would you mark the root View as accessible for the bottom sheet? That makes it so you cannot focus any of the descendant views (which is all of the sheet content) with VoiceOver on iOS.

Copy link

Choose a reason for hiding this comment

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

Yep, just tested this and this is the problem I'm seeing as well. @flo-sch I see you just opened an accessibility PR, did you want to take a look at this as well there?

accessibilityRole={_providedAccessibilityRole ?? undefined}
accessibilityLabel={_providedAccessibilityLabel ?? undefined}
accessibilityHint={
_providedAccessibilityHint
? _providedAccessibilityHint
: `Tap to ${
typeof pressBehavior === 'string' ? pressBehavior : 'move'
} the Bottom Sheet`
}
>
{children}
</Animated.View>
Copy link

Choose a reason for hiding this comment

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

I think you also need to add the accessibility props to the <Animated.View> below:

    <Animated.View
      style={containerStyle}
      pointerEvents={pointerEvents}
      accessible={_providedAccessible ?? undefined}
      accessibilityRole={_providedAccessibilityRole ?? undefined}
      accessibilityLabel={_providedAccessibilityLabel ?? undefined}
      accessibilityHint={_providedAccessibilityHint ?? undefined}
      {...rest}
    >
      {children}
    </Animated.View>

Maybe add this to a local const and use that because it is the exact same component as is used inside <TapGestureHandler/> when pressBehavior !== 'none'.

Copy link
Author

Choose a reason for hiding this comment

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

Good idea, done.

);
};
//#endregion

return pressBehavior !== 'none' ? (
<TapGestureHandler onGestureEvent={gestureHandler}>
<AnimatedView />
</TapGestureHandler>
) : (
<Animated.View pointerEvents={pointerEvents} style={containerStyle}>
{children}
</Animated.View>
<AnimatedView />
);
};

Expand Down
9 changes: 9 additions & 0 deletions src/components/bottomSheetBackdrop/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,19 @@ const DEFAULT_DISAPPEARS_ON_INDEX = 0;
const DEFAULT_ENABLE_TOUCH_THROUGH = false;
const DEFAULT_PRESS_BEHAVIOR = 'close' as const;

const DEFAULT_ACCESSIBLE = true;
const DEFAULT_ACCESSIBILITY_ROLE = 'button';
const DEFAULT_ACCESSIBILITY_LABEL = 'Bottom sheet backdrop';
const DEFAULT_ACCESSIBILITY_HINT = 'Tap to close the bottom sheet';

export {
DEFAULT_OPACITY,
DEFAULT_APPEARS_ON_INDEX,
DEFAULT_DISAPPEARS_ON_INDEX,
DEFAULT_ENABLE_TOUCH_THROUGH,
DEFAULT_PRESS_BEHAVIOR,
DEFAULT_ACCESSIBLE,
DEFAULT_ACCESSIBILITY_ROLE,
DEFAULT_ACCESSIBILITY_LABEL,
DEFAULT_ACCESSIBILITY_HINT,
};
8 changes: 6 additions & 2 deletions src/components/bottomSheetBackdrop/types.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { ReactNode } from 'react';
import type { ViewProps } from 'react-native';
import type { BottomSheetVariables } from '../../types';
import type {
BottomSheetVariables,
NullableAccessibilityProps,
} from '../../types';

export interface BottomSheetBackdropProps
extends Pick<ViewProps, 'style'>,
Expand All @@ -9,7 +12,8 @@ export interface BottomSheetBackdropProps
export type BackdropPressBehavior = 'none' | 'close' | 'collapse' | number;

export interface BottomSheetDefaultBackdropProps
extends BottomSheetBackdropProps {
extends BottomSheetBackdropProps,
NullableAccessibilityProps {
/**
* Backdrop opacity.
* @type number
Expand Down
18 changes: 17 additions & 1 deletion src/components/bottomSheetHandle/BottomSheetHandle.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,21 @@ import React, { memo, useMemo } from 'react';
import Animated from 'react-native-reanimated';
import { styles } from './styles';
import type { BottomSheetDefaultHandleProps } from './types';
import {
DEFAULT_ACCESSIBLE,
DEFAULT_ACCESSIBILITY_ROLE,
DEFAULT_ACCESSIBILITY_LABEL,
DEFAULT_ACCESSIBILITY_HINT,
} from './constants';

const BottomSheetHandleComponent = ({
style,
indicatorStyle: _indicatorStyle,
children,
accessible = DEFAULT_ACCESSIBLE,
accessibilityRole = DEFAULT_ACCESSIBILITY_ROLE,
accessibilityLabel = DEFAULT_ACCESSIBILITY_LABEL,
accessibilityHint = DEFAULT_ACCESSIBILITY_HINT,
}: BottomSheetDefaultHandleProps) => {
// styles
const containerStyle = useMemo(
Expand All @@ -23,7 +33,13 @@ const BottomSheetHandleComponent = ({

// render
return (
<Animated.View style={containerStyle}>
<Animated.View
style={containerStyle}
accessible={accessible ?? undefined}
accessibilityRole={accessibilityRole ?? undefined}
accessibilityLabel={accessibilityLabel ?? undefined}
accessibilityHint={accessibilityHint ?? undefined}
>
<Animated.View style={indicatorStyle} />
{children}
</Animated.View>
Expand Down
12 changes: 12 additions & 0 deletions src/components/bottomSheetHandle/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
const DEFAULT_ACCESSIBLE = true;
const DEFAULT_ACCESSIBILITY_ROLE = 'adjustable';
const DEFAULT_ACCESSIBILITY_LABEL = 'Bottom sheet handle';
const DEFAULT_ACCESSIBILITY_HINT =
'Drag up or down to extend or minimize the bottom sheet';

export {
DEFAULT_ACCESSIBLE,
DEFAULT_ACCESSIBILITY_ROLE,
DEFAULT_ACCESSIBILITY_LABEL,
DEFAULT_ACCESSIBILITY_HINT,
};
9 changes: 7 additions & 2 deletions src/components/bottomSheetHandle/types.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
import type React from 'react';
import type { ViewProps } from 'react-native';
import type { AnimateProps } from 'react-native-reanimated';
import type { BottomSheetVariables } from '../../types';
import type {
BottomSheetVariables,
NullableAccessibilityProps,
} from '../../types';

export interface BottomSheetHandleProps extends BottomSheetVariables {}

export interface BottomSheetDefaultHandleProps extends BottomSheetHandleProps {
export interface BottomSheetDefaultHandleProps
extends BottomSheetHandleProps,
NullableAccessibilityProps {
/**
* View style to be applied to the handle container.
* @type Animated.AnimateStyle<ViewStyle> | ViewStyle
Expand Down
4 changes: 0 additions & 4 deletions src/components/bottomSheetHandleContainer/BottomSheetHandleContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,6 @@ function BottomSheetHandleContainerComponent({
>
<Animated.View
key="BottomSheetHandleContainer"
accessible={true}
accessibilityRole="adjustable"
accessibilityLabel="Bottom Sheet handle"
accessibilityHint="Drag up or down to extend or minimize the Bottom Sheet"
onLayout={handleContainerLayout}
>
<HandleComponent
Expand Down
8 changes: 8 additions & 0 deletions src/types.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type {
SectionList,
NativeScrollEvent,
NativeSyntheticEvent,
AccessibilityProps,
} from 'react-native';
import type {
GestureEventPayload,
Expand Down Expand Up @@ -163,3 +164,10 @@ export type ScrollEventsHandlersHookType = (
handleOnMomentumEnd?: ScrollEventHandlerCallbackType;
};
//#endregion

export interface NullableAccessibilityProps extends AccessibilityProps {
accessible?: AccessibilityProps['accessible'] | null;
accessibilityLabel?: AccessibilityProps['accessibilityLabel'] | null;
accessibilityHint?: AccessibilityProps['accessibilityHint'] | null;
accessibilityRole?: AccessibilityProps['accessibilityRole'] | null;
}