-
Notifications
You must be signed in to change notification settings - Fork 3.8k
fix: break VoiceOver grouping on iOS so task checkbox is independently focusable #91530
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.
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
c4b9452
37a3806
8e10ec2
2bd995f
a7cc4a0
661fac7
9523a10
72a67fd
e51b279
500904a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ import MenuItem from '@components/MenuItem'; | |
| import MenuItemWithTopDescription from '@components/MenuItemWithTopDescription'; | ||
| import OfflineWithFeedback from '@components/OfflineWithFeedback'; | ||
| import {usePersonalDetails} from '@components/OnyxListItemProvider'; | ||
| import PressableWithoutFeedback from '@components/Pressable/PressableWithoutFeedback'; | ||
| import PressableWithSecondaryInteraction from '@components/PressableWithSecondaryInteraction'; | ||
| import RenderHTML from '@components/RenderHTML'; | ||
| import {ShowContextMenuActionsContext, ShowContextMenuStateContext} from '@components/ShowContextMenuContext'; | ||
|
|
@@ -30,6 +31,7 @@ import Navigation from '@libs/Navigation/Navigation'; | |
| import {getPersonalDetailsForAccountIDs} from '@libs/OptionsListUtils'; | ||
| import Parser from '@libs/Parser'; | ||
| import {getDisplayNameForParticipant, getDisplayNamesWithTooltips, isCompletedTaskReport, isOpenTaskReport} from '@libs/ReportUtils'; | ||
| import shouldBreakAccessibilityGrouping from '@libs/shouldBreakAccessibilityGrouping'; | ||
| import StringUtils from '@libs/StringUtils'; | ||
| import {isActiveTaskEditRoute} from '@libs/TaskUtils'; | ||
| import {callFunctionIfActionIsAllowed} from '@userActions/Session'; | ||
|
|
@@ -70,6 +72,7 @@ function TaskView({report, parentReport, action}: TaskViewProps) { | |
| const taskTitleWithoutPre = StringUtils.removePreCodeBlock(report?.reportName); | ||
| const titleWithoutImage = Parser.replace(Parser.htmlToMarkdown(taskTitleWithoutPre), {disabledRules: [...CONST.TASK_TITLE_DISABLED_RULES]}); | ||
| const taskTitle = `<task-title>${titleWithoutImage}</task-title>`; | ||
| const taskAccessibilityLabel = titleWithoutImage ? `${translate('task.task')}: ${titleWithoutImage}` : translate('task.task'); | ||
|
|
||
| const assigneeTooltipDetails = getDisplayNamesWithTooltips( | ||
| getPersonalDetailsForAccountIDs(report?.managerID ? [report?.managerID] : [], personalDetails), | ||
|
|
@@ -136,6 +139,7 @@ function TaskView({report, parentReport, action}: TaskViewProps) { | |
| <Hoverable> | ||
| {(hovered) => ( | ||
| <PressableWithSecondaryInteraction | ||
| accessible={shouldBreakAccessibilityGrouping() ? false : undefined} | ||
| onPress={callFunctionIfActionIsAllowed((e) => { | ||
| if (isDisableInteractive) { | ||
| return; | ||
|
|
@@ -152,7 +156,7 @@ function TaskView({report, parentReport, action}: TaskViewProps) { | |
| StyleUtils.getButtonBackgroundColorStyle(getButtonState(hovered, pressed, false, disableState, !isDisableInteractive), true), | ||
| isDisableInteractive && styles.cursorDefault, | ||
| ]} | ||
| accessibilityLabel={taskTitle || translate('task.task')} | ||
| accessibilityLabel={taskAccessibilityLabel} | ||
| disabled={isDisableInteractive} | ||
| sentryLabel={CONST.SENTRY_LABEL.TASK.VIEW_TITLE} | ||
| > | ||
|
|
@@ -162,7 +166,6 @@ function TaskView({report, parentReport, action}: TaskViewProps) { | |
| <View style={[styles.flexRow, styles.flex1]}> | ||
| <Checkbox | ||
| onPress={callFunctionIfActionIsAllowed(() => { | ||
| // If we're already navigating to these task editing pages, early return not to mark as completed, otherwise we would have not found page. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please restore this comment |
||
| if (isActiveTaskEditRoute(report?.reportID)) { | ||
| return; | ||
| } | ||
|
|
@@ -177,21 +180,55 @@ function TaskView({report, parentReport, action}: TaskViewProps) { | |
| containerSize={24} | ||
| containerBorderRadius={8} | ||
| caretSize={16} | ||
| accessibilityLabel={taskTitle || translate('task.task')} | ||
| accessibilityLabel={taskAccessibilityLabel} | ||
| disabled={!isTaskActionable} | ||
| sentryLabel={CONST.SENTRY_LABEL.TASK.VIEW_CHECKBOX} | ||
| /> | ||
| <View style={[styles.flexRow, styles.flex1]}> | ||
| <RenderHTML html={taskTitle} /> | ||
| </View> | ||
| {!isDisableInteractive && ( | ||
| <View style={styles.taskRightIconContainer}> | ||
| <Icon | ||
| additionalStyles={[styles.alignItemsCenter]} | ||
| src={icons.ArrowRight} | ||
| fill={StyleUtils.getIconFillColor(getButtonState(hovered, pressed, false, disableState))} | ||
| /> | ||
| </View> | ||
| {shouldBreakAccessibilityGrouping() ? ( | ||
| <PressableWithoutFeedback | ||
| onPress={callFunctionIfActionIsAllowed((e) => { | ||
| if (isDisableInteractive) { | ||
| return; | ||
| } | ||
| if (e?.type === 'click') { | ||
| (e.currentTarget as HTMLElement).blur(); | ||
| } | ||
|
Krishna2323 marked this conversation as resolved.
Outdated
|
||
| Navigation.navigate(createDynamicRoute(DYNAMIC_ROUTES.TASK_TITLE.path)); | ||
| })} | ||
| role={CONST.ROLE.BUTTON} | ||
| accessibilityLabel={taskAccessibilityLabel} | ||
| disabled={isDisableInteractive} | ||
| style={[styles.flexRow, styles.flex1]} | ||
| sentryLabel={CONST.SENTRY_LABEL.TASK.VIEW_TITLE} | ||
| > | ||
| <View style={[styles.flexRow, styles.flex1]}> | ||
| <RenderHTML html={taskTitle} /> | ||
| </View> | ||
| {!isDisableInteractive && ( | ||
| <View style={styles.taskRightIconContainer}> | ||
| <Icon | ||
| additionalStyles={[styles.alignItemsCenter]} | ||
| src={icons.ArrowRight} | ||
| fill={StyleUtils.getIconFillColor(getButtonState(hovered, pressed, false, disableState))} | ||
| /> | ||
| </View> | ||
| )} | ||
| </PressableWithoutFeedback> | ||
| ) : ( | ||
| <> | ||
| <View style={[styles.flexRow, styles.flex1]}> | ||
| <RenderHTML html={taskTitle} /> | ||
| </View> | ||
| {!isDisableInteractive && ( | ||
| <View style={styles.taskRightIconContainer}> | ||
| <Icon | ||
| additionalStyles={[styles.alignItemsCenter]} | ||
| src={icons.ArrowRight} | ||
| fill={StyleUtils.getIconFillColor(getButtonState(hovered, pressed, false, disableState))} | ||
| /> | ||
| </View> | ||
| )} | ||
| </> | ||
| )} | ||
| </View> | ||
| </OfflineWithFeedback> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| /** | ||
| * On iOS, VoiceOver groups all children of an accessible parent into a single | ||
| * focus target, preventing individual elements from being focusable. Returning | ||
| * true signals that the parent should set accessible={false} to break this grouping. | ||
| */ | ||
| export default function shouldBreakAccessibilityGrouping(): boolean { | ||
| return true; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| /** | ||
| * On non-iOS platforms, accessible elements don't group children in a way | ||
| * that prevents individual focus, so we don't need to break grouping. | ||
| */ | ||
| export default function shouldBreakAccessibilityGrouping(): boolean { | ||
| return false; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,6 +45,7 @@ import { | |
| getReportActionMessage, | ||
| getReportActionText, | ||
| getWhisperedTo, | ||
| isCreatedTaskReportAction, | ||
| isDeletedParentAction as isDeletedParentActionUtils, | ||
| isMessageDeleted, | ||
| isMoneyRequestAction, | ||
|
|
@@ -62,6 +63,7 @@ import { | |
| shouldDisplayThreadReplies as shouldDisplayThreadRepliesUtils, | ||
| } from '@libs/ReportUtils'; | ||
| import SelectionScraper from '@libs/SelectionScraper'; | ||
| import shouldBreakAccessibilityGrouping from '@libs/shouldBreakAccessibilityGrouping'; | ||
| import {ReactionListContext} from '@pages/inbox/ReportScreenContext'; | ||
| import AttachmentModalContext from '@pages/media/AttachmentModalScreen/AttachmentModalContext'; | ||
| import {clearAllRelatedReportActionErrors} from '@userActions/ClearReportActionErrors'; | ||
|
|
@@ -522,6 +524,7 @@ function PureReportActionItem({ | |
| )} | ||
| <PressableWithSecondaryInteraction | ||
| ref={popoverAnchorRef} | ||
| accessible={shouldBreakAccessibilityGrouping() && isCreatedTaskReportAction(action) ? false : undefined} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As setting false here, no pressed feedback. I'm afraid QA will mark this as regression Screen.Recording.2026-05-25.at.3.50.00.AM.movproduction: Screen.Recording.2026-05-25.at.3.51.11.AM.mov
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tried to fix that, could you please check? |
||
| onPress={() => { | ||
| if (!hasDraft) { | ||
| onPress?.(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Raw html is announced here.