Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
77 changes: 59 additions & 18 deletions src/components/ReportActionItem/TaskPreview.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {delegateEmailSelector} from '@selectors/Account';
import React from 'react';
import React, {useState} from 'react';
import {View} from 'react-native';
import type {StyleProp, ViewStyle} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
Expand Down Expand Up @@ -33,6 +33,7 @@ import Navigation from '@libs/Navigation/Navigation';
import Parser from '@libs/Parser';
import {getOriginalMessage} from '@libs/ReportActionsUtils';
import {isCanceledTaskReport, isOpenTaskReport, isReportManager} from '@libs/ReportUtils';
import shouldBreakAccessibilityGrouping from '@libs/shouldBreakAccessibilityGrouping';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
Expand Down Expand Up @@ -88,9 +89,17 @@ function TaskPreview({action, chatReportID, currentUserPersonalDetails, isHovere
// The reportAction might not contain details regarding the taskReport
// Only the direct parent reportAction will contain details about the taskReport
// Other linked reportActions will only contain the taskReportID and we will grab the details from there
const isTaskCompleted = !isEmptyObject(taskReport)
const isTaskCompletedFromOnyx = !isEmptyObject(taskReport)
? taskReport?.stateNum === CONST.REPORT.STATE_NUM.APPROVED && taskReport.statusNum === CONST.REPORT.STATUS_NUM.APPROVED
: action?.childStateNum === CONST.REPORT.STATE_NUM.APPROVED && action?.childStatusNum === CONST.REPORT.STATUS_NUM.APPROVED;
const [prevIsTaskCompletedFromOnyx, setPrevIsTaskCompletedFromOnyx] = useState(isTaskCompletedFromOnyx);
const [isTaskCompleted, setIsTaskCompleted] = useState(isTaskCompletedFromOnyx);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Double states for the same state variable seems workaround to me

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Now local state is only used when shouldBreakGrouping && isScreenReaderActive (iOS + VoiceOver).


if (prevIsTaskCompletedFromOnyx !== isTaskCompletedFromOnyx) {
setPrevIsTaskCompletedFromOnyx(isTaskCompletedFromOnyx);
setIsTaskCompleted(isTaskCompletedFromOnyx);
}
Comment on lines +101 to +107
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This local state update logic now runs on every platform, not just iOS.
If completeTask/reopenTask ever fails or is queued offline and the underlying Onyx state doesn't flip, the local state might read "checked" briefly until the next re-render syncs back. Please verify this case.
And consider scoping this to shouldBreakGrouping only.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Scoped. setLocalIsTaskCompleted is only called on iOS + VoiceOver. On all other platforms, isTaskCompleted resolves directly to the Onyx value.


const parentReportAction = useParentReportAction(taskContextReport);
const taskAssigneeAccountID = getTaskAssigneeAccountID(taskContextReport, parentReportAction) ?? action?.childManagerAccountID ?? CONST.DEFAULT_NUMBER_ID;
const parentReport = useParentReport(taskContextReport?.reportID);
Expand All @@ -106,6 +115,7 @@ function TaskPreview({action, chatReportID, currentUserPersonalDetails, isHovere
const iconWrapperStyle = StyleUtils.getTaskPreviewIconWrapper(hasAssignee ? avatarSize : undefined);

const shouldShowGreenDotIndicator = isOpenTaskReport(taskContextReport, action) && isReportManager(taskContextReport);
const taskAccessibilityLabel = taskTitleWithoutImage ? `${translate('task.task')}: ${taskTitleWithoutImage}` : translate('task.task');
Copy link
Copy Markdown
Contributor

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.

Image

if (isDeletedParentAction) {
return <RenderHTML html={`<deleted-action>${translate('parentReportAction.deletedTask')}</deleted-action>`} />;
}
Expand All @@ -121,6 +131,7 @@ function TaskPreview({action, chatReportID, currentUserPersonalDetails, isHovere
return (
<View style={[styles.chatItemMessage, !hasAssignee && styles.mv1]}>
<PressableWithoutFeedback
accessible={shouldBreakAccessibilityGrouping() ? false : undefined}
onPress={() => Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(taskReportID, undefined, undefined, Navigation.getActiveRoute()))}
onPressIn={() => canUseTouchScreen() && ControlSelection.block()}
onPressOut={() => ControlSelection.unblock()}
Expand All @@ -135,7 +146,7 @@ function TaskPreview({action, chatReportID, currentUserPersonalDetails, isHovere
shouldUseHapticsOnLongPress
style={[styles.flexRow, styles.justifyContentBetween, style]}
role={CONST.ROLE.BUTTON}
accessibilityLabel={translate('task.task')}
accessibilityLabel={taskAccessibilityLabel}
sentryLabel={CONST.SENTRY_LABEL.TASK.PREVIEW_CARD}
>
<View style={[styles.flex1, styles.flexRow, styles.alignItemsStart, styles.mr2]}>
Expand All @@ -145,32 +156,62 @@ function TaskPreview({action, chatReportID, currentUserPersonalDetails, isHovere
isChecked={isTaskCompleted}
disabled={!isTaskActionable}
onPress={callFunctionIfActionIsAllowed(() => {
setIsTaskCompleted((prev) => !prev);
if (isTaskCompleted) {
reopenTask(taskContextReport, parentReport, currentUserPersonalDetails.accountID, delegateEmail, taskReportID);
} else {
completeTask(taskContextReport, parentReport?.hasOutstandingChildTask ?? false, hasOutstandingChildTask, parentReportAction, delegateEmail, taskReportID);
}
})}
accessibilityLabel={translate('task.task')}
accessibilityLabel={taskAccessibilityLabel}
sentryLabel={CONST.SENTRY_LABEL.TASK.PREVIEW_CHECKBOX}
/>
</View>
{hasAssignee && (
<UserDetailsTooltip accountID={taskAssigneeAccountID}>
<View>
<Avatar
containerStyles={[styles.mr2, isTaskCompleted ? styles.opacitySemiTransparent : undefined]}
source={avatar}
size={avatarSize}
avatarID={taskAssigneeAccountID}
type={CONST.ICON_TYPE_AVATAR}
/>
{shouldBreakAccessibilityGrouping() ? (
<PressableWithoutFeedback
onPress={() => Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(taskReportID, undefined, undefined, Navigation.getActiveRoute()))}
style={[styles.flex1, styles.flexRow, styles.alignItemsStart]}
Comment thread
Krishna2323 marked this conversation as resolved.
Outdated
role={CONST.ROLE.BUTTON}
accessibilityLabel={taskAccessibilityLabel}
sentryLabel={CONST.SENTRY_LABEL.TASK.PREVIEW_CARD}
>
{hasAssignee && (
<UserDetailsTooltip accountID={taskAssigneeAccountID}>
<View>
<Avatar
containerStyles={[styles.mr2, isTaskCompleted ? styles.opacitySemiTransparent : undefined]}
source={avatar}
size={avatarSize}
avatarID={taskAssigneeAccountID}
type={CONST.ICON_TYPE_AVATAR}
/>
</View>
</UserDetailsTooltip>
)}
<View style={[styles.alignSelfCenter, styles.flex1]}>
<RenderHTML html={getTaskHTML()} />
</View>
</UserDetailsTooltip>
</PressableWithoutFeedback>
) : (
<>
{hasAssignee && (
<UserDetailsTooltip accountID={taskAssigneeAccountID}>
<View>
<Avatar
containerStyles={[styles.mr2, isTaskCompleted ? styles.opacitySemiTransparent : undefined]}
source={avatar}
size={avatarSize}
avatarID={taskAssigneeAccountID}
type={CONST.ICON_TYPE_AVATAR}
/>
</View>
</UserDetailsTooltip>
)}
<View style={[styles.alignSelfCenter, styles.flex1]}>
<RenderHTML html={getTaskHTML()} />
</View>
</>
)}
<View style={[styles.alignSelfCenter, styles.flex1]}>
<RenderHTML html={getTaskHTML()} />
</View>
</View>
{shouldShowGreenDotIndicator && (
<View style={iconWrapperStyle}>
Expand Down
77 changes: 61 additions & 16 deletions src/components/ReportActionItem/TaskView.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {delegateEmailSelector} from '@selectors/Account';
import {hasSeenTourSelector} from '@selectors/Onboarding';
import React, {useEffect, useMemo} from 'react';
import React, {useEffect, useMemo, useState} from 'react';
import {View} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import {AttachmentContext} from '@components/AttachmentContext';
Expand All @@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -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),
Expand All @@ -79,7 +82,14 @@ function TaskView({report, parentReport, action}: TaskViewProps) {
);

const isOpen = isOpenTaskReport(report);
const isCompleted = isCompletedTaskReport(report);
const isCompletedFromOnyx = isCompletedTaskReport(report);
const [prevIsCompletedFromOnyx, setPrevIsCompletedFromOnyx] = useState(isCompletedFromOnyx);
const [isCompleted, setIsCompleted] = useState(isCompletedFromOnyx);

if (prevIsCompletedFromOnyx !== isCompletedFromOnyx) {
setPrevIsCompletedFromOnyx(isCompletedFromOnyx);
setIsCompleted(isCompletedFromOnyx);
}
const isParentReportArchived = useReportIsArchived(parentReport?.reportID);
const hasOutstandingChildTask = useHasOutstandingChildTask(report);
const isTaskModifiable = canModifyTask(report, currentUserPersonalDetails.accountID, isParentReportArchived);
Expand Down Expand Up @@ -136,6 +146,7 @@ function TaskView({report, parentReport, action}: TaskViewProps) {
<Hoverable>
{(hovered) => (
<PressableWithSecondaryInteraction
accessible={shouldBreakAccessibilityGrouping() ? false : undefined}
onPress={callFunctionIfActionIsAllowed((e) => {
if (isDisableInteractive) {
return;
Expand All @@ -152,7 +163,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}
>
Expand All @@ -162,10 +173,10 @@ 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.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please restore this comment

if (isActiveTaskEditRoute(report?.reportID)) {
return;
}
setIsCompleted((prev) => !prev);
if (isCompleted) {
reopenTask(report, parentReport, currentUserPersonalDetails.accountID, delegateEmail);
} else {
Expand All @@ -177,21 +188,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();
}
Comment thread
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>
Expand Down
Loading
Loading