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.

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

Fix Opened offline attachment directed to conversation page on online #49832

Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
48af2f2
change attachment target to prevInitialPageRef when current viewed at…
wildan-m Sep 23, 2024
6ece814
don't dismiss only when the prev attachment with the same initialPage…
wildan-m Sep 23, 2024
19db64e
Merge branch 'main' of https://github.com/wildan-m/App into wildan/fi…
wildan-m Sep 26, 2024
34ba2c7
Merge branch 'main' of https://github.com/wildan-m/App into wildan/fi…
wildan-m Sep 26, 2024
2f21ca5
remove unnecessary code
wildan-m Sep 27, 2024
67c1f2b
adjust comment
wildan-m Sep 27, 2024
f95d459
fix lint, replace deprecated withOnyx
wildan-m Sep 27, 2024
de935c4
implement for native
wildan-m Sep 27, 2024
ce69418
lint fix, replace deprecate withOnyx
wildan-m Sep 27, 2024
613d8f3
typecheck fix
wildan-m Sep 27, 2024
f458e85
remove unused code
wildan-m Sep 27, 2024
15ac1ed
Update src/components/Attachments/AttachmentCarousel/index.tsx
wildan-m Sep 28, 2024
b42d3cd
refactor
wildan-m Sep 28, 2024
8ef2085
refactor for native
wildan-m Sep 28, 2024
e30a878
Merge branch 'main' of https://github.com/wildan-m/App into wildan/fi…
wildan-m Sep 29, 2024
ed960b1
remove unnecessary comment
wildan-m Sep 29, 2024
ca1e6d5
access the array directly instead of checking -1 in index
wildan-m Sep 29, 2024
4167773
adjust comment position
wildan-m Sep 29, 2024
6741ab6
Merge branch 'main' of https://github.com/wildan-m/App into wildan/fi…
wildan-m Sep 30, 2024
3e2e8d4
resolve issue incorrect newIndex on onPageSelected event
wildan-m Sep 30, 2024
fbd028f
refine comment
wildan-m Sep 30, 2024
9a844fa
refactor
wildan-m Sep 30, 2024
3c10da9
fix issue attachment briefly changed before dismiss
wildan-m Sep 30, 2024
aaea3ba
Merge branch 'main' of https://github.com/wildan-m/App into wildan/fi…
wildan-m Oct 1, 2024
04d8e9f
Refactor
wildan-m Oct 1, 2024
78a920e
Merge branch 'main' of https://github.com/wildan-m/App into wildan/fi…
wildan-m Oct 2, 2024
d76d54d
revert pager key solution
wildan-m Oct 2, 2024
aebb27f
fix lint
wildan-m Oct 2, 2024
8e34c56
fix lint
wildan-m Oct 2, 2024
7bb7bb2
revert compare with -1 after new eslint rule
wildan-m Oct 2, 2024
eb33707
sync web code with native
wildan-m Oct 2, 2024
2f7068a
-1 check for array access using .at
wildan-m Oct 2, 2024
f104033
Merge branch 'main' of https://github.com/wildan-m/App into wildan/fi…
wildan-m Oct 2, 2024
33cd212
Refine comment, add why point
wildan-m Oct 3, 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
48 changes: 22 additions & 26 deletions src/components/Attachments/AttachmentCarousel/index.native.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, {useCallback, useEffect, useRef, useState} from 'react';
import {Keyboard, View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import type {Attachment, AttachmentSource} from '@components/Attachments/types';
import BlockingView from '@components/BlockingViews/BlockingView';
import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
Expand All @@ -15,47 +15,52 @@ import CarouselButtons from './CarouselButtons';
import extractAttachments from './extractAttachments';
import type {AttachmentCarouselPagerHandle} from './Pager';
import AttachmentCarouselPager from './Pager';
import type {AttachmentCaraouselOnyxProps, AttachmentCarouselProps} from './types';
import type {AttachmentCarouselProps} from './types';
import useCarouselArrows from './useCarouselArrows';

function AttachmentCarousel({report, reportActions, parentReportActions, source, onNavigate, setDownloadButtonVisibility, onClose, type, accountID}: AttachmentCarouselProps) {
function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibility, onClose, type, accountID}: AttachmentCarouselProps) {
const styles = useThemeStyles();
const {translate} = useLocalize();
const pagerRef = useRef<AttachmentCarouselPagerHandle>(null);
const [parentReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.parentReportID}`, {canEvict: false});
const [reportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`, {canEvict: false});
const [page, setPage] = useState<number>();
const [attachments, setAttachments] = useState<Attachment[]>([]);
const {shouldShowArrows, setShouldShowArrows, autoHideArrows, cancelAutoHideArrows} = useCarouselArrows();
const [activeSource, setActiveSource] = useState<AttachmentSource>(source);

const compareImage = useCallback((attachment: Attachment) => attachment.source === source, [source]);

useEffect(() => {
const parentReportAction = report.parentReportActionID && parentReportActions ? parentReportActions[report.parentReportActionID] : undefined;
let targetAttachments: Attachment[] = [];
let newAttachments: Attachment[] = [];
if (type === CONST.ATTACHMENT_TYPE.NOTE && accountID) {
targetAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.NOTE, {privateNotes: report.privateNotes, accountID});
newAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.NOTE, {privateNotes: report.privateNotes, accountID});
} else {
targetAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.REPORT, {parentReportAction, reportActions});
newAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.REPORT, {parentReportAction, reportActions});
}

const initialPage = targetAttachments.findIndex(compareImage);
let newIndex = newAttachments.findIndex(compareImage);
const index = attachments.findIndex(compareImage);

// If no matching attachment with the same index, dismiss the modal
rlinoz marked this conversation as resolved.
Show resolved Hide resolved
if (newIndex === -1 && newAttachments.at(index)) {
newIndex = index;
}

// Dismiss the modal when deleting an attachment during its display in preview.
if (initialPage === -1 && attachments.find(compareImage)) {
if (newIndex === -1 && attachments.at(index)) {
Navigation.dismissModal();
} else {
setPage(initialPage);
setAttachments(targetAttachments);
setPage(newIndex);
setAttachments(newAttachments);

// Update the download button visibility in the parent modal
if (setDownloadButtonVisibility) {
setDownloadButtonVisibility(initialPage !== -1);
setDownloadButtonVisibility(newIndex !== -1);
}

const attachment = targetAttachments.at(initialPage);

const attachment = newAttachments.at(newIndex);
// Update the parent modal's state with the source and name from the mapped attachments
if (initialPage !== -1 && attachment !== undefined && onNavigate) {
if (newIndex !== -1 && attachment !== undefined && onNavigate) {
onNavigate(attachment);
}
}
Expand Down Expand Up @@ -148,13 +153,4 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source,

AttachmentCarousel.displayName = 'AttachmentCarousel';

export default withOnyx<AttachmentCarouselProps, AttachmentCaraouselOnyxProps>({
parentReportActions: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.parentReportID}`,
canEvict: false,
},
reportActions: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`,
canEvict: false,
},
})(AttachmentCarousel);
export default AttachmentCarousel;
49 changes: 23 additions & 26 deletions src/components/Attachments/AttachmentCarousel/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type {ListRenderItemInfo} from 'react-native';
import {Keyboard, PixelRatio, View} from 'react-native';
import type {GestureType} from 'react-native-gesture-handler';
import {Gesture, GestureDetector} from 'react-native-gesture-handler';
import {withOnyx} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import Animated, {scrollTo, useAnimatedRef, useSharedValue} from 'react-native-reanimated';
import type {Attachment, AttachmentSource} from '@components/Attachments/types';
import BlockingView from '@components/BlockingViews/BlockingView';
Expand All @@ -26,7 +26,7 @@ import CarouselButtons from './CarouselButtons';
import CarouselItem from './CarouselItem';
import extractAttachments from './extractAttachments';
import AttachmentCarouselPagerContext from './Pager/AttachmentCarouselPagerContext';
import type {AttachmentCaraouselOnyxProps, AttachmentCarouselProps, UpdatePageProps} from './types';
import type {AttachmentCarouselProps, UpdatePageProps} from './types';
import useCarouselArrows from './useCarouselArrows';
import useCarouselContextEvents from './useCarouselContextEvents';

Expand All @@ -38,7 +38,7 @@ const viewabilityConfig = {

const MIN_FLING_VELOCITY = 500;

function AttachmentCarousel({report, reportActions, parentReportActions, source, onNavigate, setDownloadButtonVisibility, type, accountID, onClose}: AttachmentCarouselProps) {
function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibility, type, accountID, onClose}: AttachmentCarouselProps) {
const theme = useTheme();
const {translate} = useLocalize();
const {windowWidth} = useWindowDimensions();
Expand All @@ -48,7 +48,8 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source,
const scrollRef = useAnimatedRef<Animated.FlatList<ListRenderItemInfo<Attachment>>>();
const nope = useSharedValue(false);
const pagerRef = useRef<GestureType>(null);

const [parentReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.parentReportID}`, {canEvict: false});
const [reportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`, {canEvict: false});
const canUseTouchScreen = DeviceCapabilities.canUseTouchScreen();

const modalStyles = styles.centeredModalStyles(shouldUseNarrowLayout, true);
Expand All @@ -73,38 +74,43 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source,

useEffect(() => {
const parentReportAction = report.parentReportActionID && parentReportActions ? parentReportActions[report.parentReportActionID] : undefined;
let targetAttachments: Attachment[] = [];
let newAttachments: Attachment[] = [];
if (type === CONST.ATTACHMENT_TYPE.NOTE && accountID) {
targetAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.NOTE, {privateNotes: report.privateNotes, accountID});
newAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.NOTE, {privateNotes: report.privateNotes, accountID});
} else {
targetAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.REPORT, {parentReportAction, reportActions: reportActions ?? undefined});
newAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.REPORT, {parentReportAction, reportActions: reportActions ?? undefined});
}

if (isEqual(attachments, targetAttachments)) {
if (isEqual(attachments, newAttachments)) {
if (attachments.length === 0) {
setPage(-1);
setDownloadButtonVisibility?.(false);
}
return;
}

const initialPage = targetAttachments.findIndex(compareImage);
let newIndex = newAttachments.findIndex(compareImage);
const index = attachments.findIndex(compareImage);

// If no matching attachment with the same index, dismiss the modal
rlinoz marked this conversation as resolved.
Show resolved Hide resolved
if (newIndex === -1 && newAttachments.at(index)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to check if index is not -1 too otherwise newAttachments.at(index) may be truthy (please fix same in other places)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

newIndex = index;
}

// Dismiss the modal when deleting an attachment during its display in preview.
if (initialPage === -1 && attachments.find(compareImage)) {
if (newIndex === -1 && attachments.at(index)) {
Navigation.dismissModal();
} else {
setPage(initialPage);
setAttachments(targetAttachments);
setPage(newIndex);
setAttachments(newAttachments);

// Update the download button visibility in the parent modal
if (setDownloadButtonVisibility) {
setDownloadButtonVisibility(initialPage !== -1);
setDownloadButtonVisibility(newIndex !== -1);
}

const attachment = targetAttachments.at(initialPage);
const attachment = newAttachments.at(newIndex);
// Update the parent modal's state with the source and name from the mapped attachments
if (initialPage !== -1 && attachment !== undefined && onNavigate) {
if (newIndex !== -1 && attachment !== undefined && onNavigate) {
onNavigate(attachment);
}
}
Expand Down Expand Up @@ -307,13 +313,4 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source,

AttachmentCarousel.displayName = 'AttachmentCarousel';

export default withOnyx<AttachmentCarouselProps, AttachmentCaraouselOnyxProps>({
parentReportActions: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.parentReportID}`,
canEvict: false,
},
reportActions: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`,
canEvict: false,
},
})(AttachmentCarousel);
export default AttachmentCarousel;
15 changes: 3 additions & 12 deletions src/components/Attachments/AttachmentCarousel/types.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,14 @@
import type {ViewToken} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import type {ValueOf} from 'type-fest';
import type {Attachment, AttachmentSource} from '@components/Attachments/types';
import type CONST from '@src/CONST';
import type {Report, ReportActions} from '@src/types/onyx';
import type {Report} from '@src/types/onyx';

type UpdatePageProps = {
viewableItems: ViewToken[];
};

type AttachmentCaraouselOnyxProps = {
/** Object of report actions for this report */
reportActions: OnyxEntry<ReportActions>;

/** The report actions of the parent report */
parentReportActions: OnyxEntry<ReportActions>;
};

type AttachmentCarouselProps = AttachmentCaraouselOnyxProps & {
type AttachmentCarouselProps = {
/** Source is used to determine the starting index in the array of attachments */
source: AttachmentSource;

Expand All @@ -40,4 +31,4 @@ type AttachmentCarouselProps = AttachmentCaraouselOnyxProps & {
onClose: () => void;
};

export type {AttachmentCarouselProps, UpdatePageProps, AttachmentCaraouselOnyxProps};
export type {AttachmentCarouselProps, UpdatePageProps};
Loading