From 438edd96f62603afe5904965d35b14a98f8a8516 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Fri, 17 Jan 2025 12:41:14 +0100 Subject: [PATCH] Fix edit attachments (#8468) * Fix edit attachments * Address feedback --- app/components/files/file.tsx | 2 +- app/components/files/files.test.tsx | 616 ++++++++++++++++++ app/components/files/files.tsx | 55 +- app/components/files_search/file_results.tsx | 2 +- .../handlers/post.test.ts | 184 +++++- .../server_data_operator/handlers/post.ts | 10 +- app/hooks/files.test.ts | 145 +++++ app/hooks/files.ts | 20 +- app/hooks/gallery.ts | 2 +- app/hooks/utils.test.tsx | 73 +++ app/hooks/utils.ts | 19 + test/api_mocks/file.ts | 19 + 12 files changed, 1108 insertions(+), 39 deletions(-) create mode 100644 app/components/files/files.test.tsx create mode 100644 app/hooks/files.test.ts create mode 100644 app/hooks/utils.test.tsx create mode 100644 app/hooks/utils.ts create mode 100644 test/api_mocks/file.ts diff --git a/app/components/files/file.tsx b/app/components/files/file.tsx index bd3ce0d40..057c6c7a3 100644 --- a/app/components/files/file.tsx +++ b/app/components/files/file.tsx @@ -94,7 +94,7 @@ const File = ({ } else { onPress(index); } - }, [index]); + }, [index, onPress]); const {styles, onGestureEvent, ref} = useGalleryItem(galleryIdentifier, index, handlePreviewPress); diff --git a/app/components/files/files.test.tsx b/app/components/files/files.test.tsx new file mode 100644 index 000000000..3825057a5 --- /dev/null +++ b/app/components/files/files.test.tsx @@ -0,0 +1,616 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. +import {render, fireEvent} from '@testing-library/react-native'; +import React, {useMemo, type ComponentProps} from 'react'; +import {DeviceEventEmitter, Text, TouchableOpacity, View} from 'react-native'; + +import {Events} from '@constants'; +import {useIsTablet} from '@hooks/device'; +import {useImageAttachments} from '@hooks/files'; +import {mockFileInfo} from '@test/api_mocks/file'; +import {isImage, isVideo} from '@utils/file'; +import {fileToGalleryItem, openGalleryAtIndex} from '@utils/gallery'; +import {getViewPortWidth} from '@utils/images'; + +import File from './file'; +import Files from './files'; + +import type FileModel from '@typings/database/models/servers/file'; + +jest.mock('@hooks/device', () => ({ + useIsTablet: jest.fn().mockReturnValue(false), +})); + +jest.mock('@hooks/files', () => ({ + useImageAttachments: jest.fn().mockReturnValue({ + images: [], + nonImages: [], + }), +})); + +jest.mock('@hooks/utils', () => ({ + usePreventDoubleTap: (fn: () => void) => fn, +})); + +jest.mock('@utils/file', () => ({ + isImage: jest.fn().mockReturnValue(true), + isVideo: jest.fn().mockReturnValue(false), +})); + +jest.mock('@utils/gallery', () => ({ + fileToGalleryItem: jest.fn(), + openGalleryAtIndex: jest.fn(), +})); + +jest.mock('@utils/images', () => ({ + getViewPortWidth: jest.fn().mockReturnValue(300), +})); + +jest.mock('./file', () => ({ + __esModule: true, + default: jest.fn(), +})); + +jest.mocked(File).mockImplementation((props) => ( + + {props.galleryIdentifier} + {String(props.canDownloadFiles)} + {props.index} + {String(props.isSingleImage)} + {String(props.nonVisibleImagesCount)} + {String(props.publicLinkEnabled)} + {props.wrapperWidth} + {String(props.inViewPort)} + props.onPress(props.index)} + /> + props.updateFileForGallery(props.index, {...props.file, uri: 'updated'})} + /> + +)); + +function getBaseProps(): ComponentProps { + return { + canDownloadFiles: true, + failed: false, + filesInfo: [], + isReplyPost: false, + layoutWidth: 300, + location: 'test-location', + postId: 'test-post-id', + postProps: {}, + publicLinkEnabled: true, + }; +} + +describe('Files', () => { + it('should render attachments, with images in the image row', () => { + const filesInfo = [ + mockFileInfo({id: '1'}), + mockFileInfo({id: '2'}), + mockFileInfo({id: '3'}), + mockFileInfo({id: '4'}), + ]; + jest.mocked(useImageAttachments).mockImplementation((fi) => { + return useMemo(() => ({ + images: fi.filter((f) => f.id === '1' || f.id === '2'), + nonImages: fi.filter((f) => f.id === '3' || f.id === '4'), + }), [fi]); + }); + + const {getByTestId} = render( + , + ); + expect(getByTestId('1')).toBeVisible(); + expect(getByTestId('2')).toBeVisible(); + expect(getByTestId('3')).toBeVisible(); + expect(getByTestId('4')).toBeVisible(); + expect(getByTestId('image-row')).toContainElement(getByTestId('1')); + expect(getByTestId('image-row')).toContainElement(getByTestId('2')); + expect(getByTestId('image-row')).not.toContainElement(getByTestId('3')); + expect(getByTestId('image-row')).not.toContainElement(getByTestId('4')); + }); + + it('should not show the image row if no images', () => { + const filesInfo = [ + mockFileInfo({id: '3'}), + mockFileInfo({id: '4'}), + ]; + jest.mocked(useImageAttachments).mockImplementation((fi) => { + return useMemo(() => ({ + images: fi.filter((f) => f.id === '1' || f.id === '2'), + nonImages: fi.filter((f) => f.id === '3' || f.id === '4'), + }), [fi]); + }); + + const {getByTestId, queryByTestId} = render( + , + ); + expect(getByTestId('3')).toBeVisible(); + expect(getByTestId('4')).toBeVisible(); + expect(queryByTestId('image-row')).not.toBeVisible(); + }); + + it('should have different opacity if failed', () => { + const baseProps = getBaseProps(); + baseProps.failed = true; + + const {getByTestId, rerender} = render( + , + ); + + expect(getByTestId('files-container')).toHaveStyle({opacity: 0.5}); + + baseProps.failed = false; + rerender(); + + expect(getByTestId('files-container')).not.toHaveStyle({opacity: 0.5}); + }); + + it('should drill all relevant props', () => { + const filesInfo = [ + mockFileInfo({id: '1'}), + mockFileInfo({id: '2'}), + ]; + + jest.mocked(useImageAttachments).mockImplementation((fi) => { + return useMemo(() => ({ + images: fi.filter((f) => f.id === '1'), + nonImages: fi.filter((f) => f.id === '2'), + }), [fi]); + }); + + const baseProps = getBaseProps(); + baseProps.filesInfo = filesInfo; + baseProps.canDownloadFiles = false; + baseProps.publicLinkEnabled = false; + + const {getByTestId, rerender} = render(); + expect(getByTestId('1-canDownloadFiles')).toHaveTextContent('false'); + expect(getByTestId('1-publicLinkEnabled')).toHaveTextContent('false'); + expect(getByTestId('2-canDownloadFiles')).toHaveTextContent('false'); + expect(getByTestId('2-publicLinkEnabled')).toHaveTextContent('false'); + + baseProps.canDownloadFiles = true; + baseProps.publicLinkEnabled = true; + + rerender(); + expect(getByTestId('1-canDownloadFiles')).toHaveTextContent('true'); + expect(getByTestId('1-publicLinkEnabled')).toHaveTextContent('true'); + expect(getByTestId('2-canDownloadFiles')).toHaveTextContent('true'); + expect(getByTestId('2-publicLinkEnabled')).toHaveTextContent('true'); + + baseProps.canDownloadFiles = true; + baseProps.publicLinkEnabled = false; + + rerender(); + expect(getByTestId('1-canDownloadFiles')).toHaveTextContent('true'); + expect(getByTestId('1-publicLinkEnabled')).toHaveTextContent('false'); + expect(getByTestId('2-canDownloadFiles')).toHaveTextContent('true'); + expect(getByTestId('2-publicLinkEnabled')).toHaveTextContent('false'); + + baseProps.canDownloadFiles = false; + baseProps.publicLinkEnabled = true; + + rerender(); + expect(getByTestId('1-canDownloadFiles')).toHaveTextContent('false'); + expect(getByTestId('1-publicLinkEnabled')).toHaveTextContent('true'); + expect(getByTestId('2-canDownloadFiles')).toHaveTextContent('false'); + expect(getByTestId('2-publicLinkEnabled')).toHaveTextContent('true'); + }); + + it('should set layoutWidth if provided', () => { + const filesInfo = [ + mockFileInfo({id: '1'}), + mockFileInfo({id: '2'}), + ]; + + jest.mocked(useImageAttachments).mockImplementation((fi) => { + return useMemo(() => ({ + images: fi.filter((f) => f.id === '1'), + nonImages: fi.filter((f) => f.id === '2'), + }), [fi]); + }); + + const baseProps = getBaseProps(); + baseProps.filesInfo = filesInfo; + baseProps.layoutWidth = 400; + + const {getByTestId, rerender} = render(); + expect(getByTestId('1-wrapperWidth')).toHaveTextContent('400'); + expect(getByTestId('2-wrapperWidth')).toHaveTextContent('400'); + expect(getByTestId('image-row')).toHaveStyle({width: 400}); + + baseProps.layoutWidth = 500; + rerender(); + expect(getByTestId('1-wrapperWidth')).toHaveTextContent('500'); + expect(getByTestId('2-wrapperWidth')).toHaveTextContent('500'); + expect(getByTestId('image-row')).toHaveStyle({width: 500}); + }); + + it('should use ((getViewportWidth result) - 6) if layoutWidth is not provided', () => { + const filesInfo = [ + mockFileInfo({id: '1'}), + mockFileInfo({id: '2'}), + ]; + + jest.mocked(useImageAttachments).mockImplementation((fi) => { + return useMemo(() => ({ + images: fi.filter((f) => f.id === '1'), + nonImages: fi.filter((f) => f.id === '2'), + }), [fi]); + }); + jest.mocked(getViewPortWidth).mockReturnValue(300); + jest.mocked(useIsTablet).mockReturnValue(false); + + const baseProps = getBaseProps(); + baseProps.filesInfo = filesInfo; + baseProps.layoutWidth = undefined; + baseProps.isReplyPost = false; + + const {getByTestId, rerender} = render(); + expect(getByTestId('1-wrapperWidth')).toHaveTextContent('294'); + expect(getByTestId('2-wrapperWidth')).toHaveTextContent('294'); + expect(getByTestId('image-row')).toHaveStyle({width: 294}); + expect(getViewPortWidth).toHaveBeenCalledWith(false, false); + + jest.mocked(getViewPortWidth).mockReturnValue(400); + jest.mocked(useIsTablet).mockReturnValue(true); + baseProps.isReplyPost = true; + + rerender(); + expect(getByTestId('1-wrapperWidth')).toHaveTextContent('394'); + expect(getByTestId('2-wrapperWidth')).toHaveTextContent('394'); + expect(getByTestId('image-row')).toHaveStyle({width: 394}); + expect(getViewPortWidth).toHaveBeenCalledWith(true, true); + }); + + it('calling onPress on the child should open gallery', () => { + const filesInfo = [ + mockFileInfo({id: '1'}), + mockFileInfo({id: '2'}), + ]; + + jest.mocked(useImageAttachments).mockImplementation((fi) => { + return useMemo(() => ({ + images: fi.filter((f) => f.id === '1'), + nonImages: fi.filter((f) => f.id === '2'), + }), [fi]); + }); + + jest.mocked(fileToGalleryItem).mockImplementation((file) => ({ + height: 100, + id: file.id || '', + lastPictureUpdate: 0, + mime_type: file.mime_type, + name: file.name, + type: 'image', + uri: file.uri || '', + width: file.width, + })); + + const {getByTestId} = render( + , + ); + + fireEvent.press(getByTestId('1-onPress')); + expect(openGalleryAtIndex).toHaveBeenCalledWith( + 'test-post-id-fileAttachments-test-location', + 0, + expect.arrayContaining([expect.objectContaining({id: '1'}), expect.objectContaining({id: '2'})]), + ); + + fireEvent.press(getByTestId('2-onPress')); + expect(openGalleryAtIndex).toHaveBeenCalledWith( + 'test-post-id-fileAttachments-test-location', + 1, + expect.arrayContaining([expect.objectContaining({id: '1'}), expect.objectContaining({id: '2'})]), + ); + }); + + it('calling updateFileForGallery updates the file', () => { + const filesInfo = [ + mockFileInfo({id: '1', uri: 'original'}), + mockFileInfo({id: '2', uri: 'original'}), + ]; + + jest.mocked(useImageAttachments).mockImplementation((fi) => { + return useMemo(() => ({ + images: fi.filter((f) => f.id === '1'), + nonImages: fi.filter((f) => f.id === '2'), + }), [fi]); + }); + + jest.mocked(fileToGalleryItem).mockImplementation((file) => ({ + height: 100, + id: file.id || '', + lastPictureUpdate: 0, + mime_type: file.mime_type, + name: file.name, + type: 'image', + uri: file.uri || '', + width: file.width, + })); + + const {getByTestId} = render( + , + ); + + fireEvent.press(getByTestId('1-updateFileForGallery')); + fireEvent.press(getByTestId('1-onPress')); + expect(openGalleryAtIndex).toHaveBeenCalledWith( + 'test-post-id-fileAttachments-test-location', + 0, + expect.arrayContaining([expect.objectContaining({id: '1', uri: 'updated'}), expect.objectContaining({id: '2', uri: 'original'})]), + ); + + fireEvent.press(getByTestId('2-onPress')); + expect(openGalleryAtIndex).toHaveBeenCalledWith( + 'test-post-id-fileAttachments-test-location', + 1, + expect.arrayContaining([expect.objectContaining({id: '1', uri: 'updated'}), expect.objectContaining({id: '2', uri: 'original'})]), + ); + }); + + it('should update files for gallery on new props', () => { + const {rerender, getByTestId} = render( + , + ); + const newFilesInfo = [ + mockFileInfo({id: '1', name: 'image1.png', user_id: 'user1'}), + ]; + rerender( + , + ); + expect(getByTestId('1')).toBeVisible(); + }); + + it('should set inViewPort to true on ITEM_IN_VIEWPORT event', () => { + const filesInfo = [ + mockFileInfo({id: '1', name: 'image1.png', user_id: 'user1'}), + mockFileInfo({id: '2', name: 'image2.png', user_id: 'user2'}), + ]; + + jest.mocked(useImageAttachments).mockImplementation((fi) => { + return useMemo(() => ({ + images: fi.filter((f) => f.id === '1'), + nonImages: fi.filter((f) => f.id === '2'), + }), [fi]); + }); + + const {getByTestId} = render( + , + ); + + expect(getByTestId('1-inViewPort')).toHaveTextContent('false'); + expect(getByTestId('2-inViewPort')).toHaveTextContent('false'); + DeviceEventEmitter.emit(Events.ITEM_IN_VIEWPORT, {'unrelated-event': true}); + expect(getByTestId('1-inViewPort')).toHaveTextContent('false'); + expect(getByTestId('2-inViewPort')).toHaveTextContent('false'); + DeviceEventEmitter.emit(Events.ITEM_IN_VIEWPORT, {'test-location-test-post-id': true}); + expect(getByTestId('1-inViewPort')).toHaveTextContent('true'); + expect(getByTestId('2-inViewPort')).toHaveTextContent('true'); + }); + + it('should ignore ITEM_IN_VIEWPORT event if not for the current post or location', () => { + const filesInfo = [ + mockFileInfo({id: '1', name: 'image1.png', user_id: 'user1'}), + mockFileInfo({id: '2', name: 'image2.png', user_id: 'user2'}), + ]; + + jest.mocked(useImageAttachments).mockImplementation((fi) => { + return useMemo(() => ({ + images: fi.filter((f) => f.id === '1'), + nonImages: fi.filter((f) => f.id === '2'), + }), [fi]); + }); + + const baseProps = getBaseProps(); + baseProps.filesInfo = filesInfo; + baseProps.location = 'location1'; + baseProps.postId = 'post1'; + + const {getByTestId, rerender} = render(); + + expect(getByTestId('1-inViewPort')).toHaveTextContent('false'); + expect(getByTestId('2-inViewPort')).toHaveTextContent('false'); + + baseProps.location = 'location2'; + rerender(); + expect(getByTestId('1-inViewPort')).toHaveTextContent('false'); + expect(getByTestId('2-inViewPort')).toHaveTextContent('false'); + + DeviceEventEmitter.emit(Events.ITEM_IN_VIEWPORT, {'location1-post1': true}); + expect(getByTestId('1-inViewPort')).toHaveTextContent('false'); + expect(getByTestId('2-inViewPort')).toHaveTextContent('false'); + + baseProps.postId = 'post2'; + rerender(); + expect(getByTestId('1-inViewPort')).toHaveTextContent('false'); + expect(getByTestId('2-inViewPort')).toHaveTextContent('false'); + + DeviceEventEmitter.emit(Events.ITEM_IN_VIEWPORT, {'location2-post1': true}); + expect(getByTestId('1-inViewPort')).toHaveTextContent('false'); + expect(getByTestId('2-inViewPort')).toHaveTextContent('false'); + + DeviceEventEmitter.emit(Events.ITEM_IN_VIEWPORT, {'location2-post2': true}); + expect(getByTestId('1-inViewPort')).toHaveTextContent('true'); + expect(getByTestId('2-inViewPort')).toHaveTextContent('true'); + }); + + it('should pass isSingleImage to the children', () => { + let filesInfo = [ + mockFileInfo({id: '1'}), + mockFileInfo({id: '3'}), + mockFileInfo({id: '4'}), + ]; + + const selectImages = (f: FileInfo | FileModel | undefined) => f?.id === '1' || f?.id === '2'; + + jest.mocked(useImageAttachments).mockImplementation((fi) => { + return useMemo(() => ({ + images: fi.filter((f) => selectImages(f)), + nonImages: fi.filter((f) => !selectImages(f)), + }), [fi]); + }); + + jest.mocked(isImage).mockImplementation((f) => selectImages(f)); + jest.mocked(isVideo).mockReturnValue(false); + + const {rerender, getByTestId} = render( + , + ); + + expect(getByTestId('1-isSingleImage')).toHaveTextContent('true'); + expect(getByTestId('3-isSingleImage')).toHaveTextContent('true'); + expect(getByTestId('4-isSingleImage')).toHaveTextContent('true'); + + jest.mocked(isImage).mockReturnValue(false); + jest.mocked(isVideo).mockImplementation((f) => selectImages(f)); + rerender( + , + ); + + expect(getByTestId('1-isSingleImage')).toHaveTextContent('true'); + expect(getByTestId('3-isSingleImage')).toHaveTextContent('true'); + expect(getByTestId('4-isSingleImage')).toHaveTextContent('true'); + + filesInfo = [ + mockFileInfo({id: '1'}), + mockFileInfo({id: '2'}), + mockFileInfo({id: '3'}), + mockFileInfo({id: '4'}), + ]; + + jest.mocked(isImage).mockImplementation((f) => selectImages(f)); + jest.mocked(isVideo).mockReturnValue(false); + + rerender( + , + ); + + expect(getByTestId('1-isSingleImage')).toHaveTextContent('false'); + expect(getByTestId('2-isSingleImage')).toHaveTextContent('false'); + expect(getByTestId('3-isSingleImage')).toHaveTextContent('false'); + expect(getByTestId('4-isSingleImage')).toHaveTextContent('false'); + + jest.mocked(isImage).mockReturnValue(false); + jest.mocked(isVideo).mockImplementation((f) => selectImages(f)); + rerender( + , + ); + + expect(getByTestId('1-isSingleImage')).toHaveTextContent('false'); + expect(getByTestId('2-isSingleImage')).toHaveTextContent('false'); + expect(getByTestId('3-isSingleImage')).toHaveTextContent('false'); + expect(getByTestId('4-isSingleImage')).toHaveTextContent('false'); + }); + + it('should trim more than 4 images and properly add the non visible images count to the last image', () => { + const filesInfo = [ + mockFileInfo({id: '1'}), + mockFileInfo({id: '2'}), + mockFileInfo({id: '3'}), + mockFileInfo({id: '4'}), + mockFileInfo({id: '5'}), + mockFileInfo({id: '6'}), + mockFileInfo({id: '7'}), + mockFileInfo({id: '8'}), + mockFileInfo({id: '9'}), + mockFileInfo({id: '10'}), + ]; + + jest.mocked(useImageAttachments).mockImplementation((fi) => { + return useMemo(() => ({ + images: fi.filter((f) => f.id === '1' || f.id === '2' || f.id === '3' || f.id === '4' || f.id === '5'), + nonImages: fi.filter((f) => f.id === '6' || f.id === '7' || f.id === '8' || f.id === '9' || f.id === '10'), + }), [fi]); + }); + + const {getByTestId, queryByTestId} = render( + , + ); + + expect(getByTestId('1-nonVisibleImagesCount')).toHaveTextContent('undefined'); + expect(getByTestId('2-nonVisibleImagesCount')).toHaveTextContent('undefined'); + expect(getByTestId('3-nonVisibleImagesCount')).toHaveTextContent('undefined'); + expect(getByTestId('4-nonVisibleImagesCount')).toHaveTextContent('1'); + expect(queryByTestId('5-nonVisibleImagesCount')).not.toBeVisible(); + expect(getByTestId('6-nonVisibleImagesCount')).toHaveTextContent('undefined'); + expect(getByTestId('7-nonVisibleImagesCount')).toHaveTextContent('undefined'); + expect(getByTestId('8-nonVisibleImagesCount')).toHaveTextContent('undefined'); + expect(getByTestId('9-nonVisibleImagesCount')).toHaveTextContent('undefined'); + expect(getByTestId('10-nonVisibleImagesCount')).toHaveTextContent('undefined'); + }); + + it('should add gutter to the container of to all elements but the first only on image row', () => { + const filesInfo = [ + mockFileInfo({id: '1'}), + mockFileInfo({id: '2'}), + mockFileInfo({id: '3'}), + mockFileInfo({id: '4'}), + mockFileInfo({id: '5'}), + mockFileInfo({id: '6'}), + ]; + + jest.mocked(useImageAttachments).mockImplementation((fi) => { + return useMemo(() => ({ + images: fi.filter((f) => f.id === '1' || f.id === '2' || f.id === '3'), + nonImages: fi.filter((f) => f.id === '4' || f.id === '5' || f.id === '6'), + }), [fi]); + }); + + const {getByTestId} = render( + , + ); + + expect(getByTestId('1-file-container')).not.toHaveStyle({marginLeft: 8}); + expect(getByTestId('2-file-container')).toHaveStyle({marginLeft: 8}); + expect(getByTestId('3-file-container')).toHaveStyle({marginLeft: 8}); + expect(getByTestId('4-file-container')).not.toHaveStyle({marginLeft: 8}); + expect(getByTestId('5-file-container')).not.toHaveStyle({marginLeft: 8}); + expect(getByTestId('6-file-container')).not.toHaveStyle({marginLeft: 8}); + }); +}); diff --git a/app/components/files/files.tsx b/app/components/files/files.tsx index 3cdddb2af..a5a5e9b6c 100644 --- a/app/components/files/files.tsx +++ b/app/components/files/files.tsx @@ -1,7 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import React, {useEffect, useMemo, useState} from 'react'; +import React, {useCallback, useEffect, useMemo, useState} from 'react'; import {DeviceEventEmitter, type StyleProp, StyleSheet, View, type ViewStyle} from 'react-native'; import Animated from 'react-native-reanimated'; @@ -9,10 +9,10 @@ import {Events} from '@constants'; import {GalleryInit} from '@context/gallery'; import {useIsTablet} from '@hooks/device'; import {useImageAttachments} from '@hooks/files'; +import {usePreventDoubleTap} from '@hooks/utils'; import {isImage, isVideo} from '@utils/file'; import {fileToGalleryItem, openGalleryAtIndex} from '@utils/gallery'; import {getViewPortWidth} from '@utils/images'; -import {preventDoubleTap} from '@utils/tap'; import File from './file'; @@ -49,29 +49,38 @@ const styles = StyleSheet.create({ }, }); -const Files = ({canDownloadFiles, failed, filesInfo, isReplyPost, layoutWidth, location, postId, postProps, publicLinkEnabled}: FilesProps) => { +const Files = ({ + canDownloadFiles, + failed, + filesInfo, + isReplyPost, + layoutWidth, + location, + postId, + postProps, + publicLinkEnabled, +}: FilesProps) => { const galleryIdentifier = `${postId}-fileAttachments-${location}`; const [inViewPort, setInViewPort] = useState(false); const isTablet = useIsTablet(); - const {images: imageAttachments, nonImages: nonImageAttachments} = useImageAttachments(filesInfo, publicLinkEnabled); - - const filesForGallery = useMemo(() => imageAttachments.concat(nonImageAttachments), - [imageAttachments, nonImageAttachments]); + const {images: imageAttachments, nonImages: nonImageAttachments} = useImageAttachments(filesInfo); + const [filesForGallery, setFilesForGallery] = useState(() => [...imageAttachments, ...nonImageAttachments]); const attachmentIndex = (fileId: string) => { return filesForGallery.findIndex((file) => file.id === fileId) || 0; }; - const handlePreviewPress = preventDoubleTap((idx: number) => { + const handlePreviewPress = usePreventDoubleTap(useCallback((idx: number) => { const items = filesForGallery.map((f) => fileToGalleryItem(f, f.user_id, postProps)); openGalleryAtIndex(galleryIdentifier, idx, items); - }); + }, [filesForGallery, galleryIdentifier, postProps])); - const updateFileForGallery = (idx: number, file: FileInfo) => { - 'worklet'; - filesForGallery[idx] = file; - }; + const updateFileForGallery = useCallback((idx: number, file: FileInfo) => { + const newFilesForGallery = [...filesForGallery]; + newFilesForGallery[idx] = file; + setFilesForGallery(newFilesForGallery); + }, [filesForGallery]); const isSingleImage = useMemo(() => filesInfo.filter((f) => isImage(f) || isVideo(f)).length === 1, [filesInfo]); @@ -79,6 +88,7 @@ const Files = ({canDownloadFiles, failed, filesInfo, isReplyPost, layoutWidth, l let nonVisibleImagesCount: number; let container: StyleProp = items.length > 1 ? styles.container : undefined; const containerWithGutter = [container, styles.gutter]; + const wrapperWidth = getViewPortWidth(isReplyPost, isTablet) - 6; return items.map((file, idx) => { if (moreImagesCount && idx === MAX_VISIBLE_ROW_IMAGES - 1) { @@ -91,6 +101,7 @@ const Files = ({canDownloadFiles, failed, filesInfo, isReplyPost, layoutWidth, l return ( @@ -126,7 +137,10 @@ const Files = ({canDownloadFiles, failed, filesInfo, isReplyPost, layoutWidth, l } return ( - + { renderItems(visibleImages, nonVisibleImagesCount, true) } ); @@ -140,11 +154,18 @@ const Files = ({canDownloadFiles, failed, filesInfo, isReplyPost, layoutWidth, l }); return () => onScrollEnd.remove(); - }, []); + }, [location, postId]); + + useEffect(() => { + setFilesForGallery([...imageAttachments, ...nonImageAttachments]); + }, [imageAttachments, nonImageAttachments]); return ( - + {renderImageRow()} {renderItems(nonImageAttachments)} diff --git a/app/components/files_search/file_results.tsx b/app/components/files_search/file_results.tsx index 5056d9b36..0aa23b87c 100644 --- a/app/components/files_search/file_results.tsx +++ b/app/components/files_search/file_results.tsx @@ -76,7 +76,7 @@ const FileResults = ({ const containerStyle = useMemo(() => ([paddingTop, {flexGrow: 1}]), [paddingTop]); const numOptions = getNumberFileMenuOptions(canDownloadFiles, publicLinkEnabled); - const {images: imageAttachments, nonImages: nonImageAttachments} = useImageAttachments(fileInfos, publicLinkEnabled); + const {images: imageAttachments, nonImages: nonImageAttachments} = useImageAttachments(fileInfos); const filesForGallery = useMemo(() => imageAttachments.concat(nonImageAttachments), [imageAttachments, nonImageAttachments]); const channelNames = useMemo(() => getChannelNamesWithID(fileChannels), [fileChannels]); diff --git a/app/database/operator/server_data_operator/handlers/post.test.ts b/app/database/operator/server_data_operator/handlers/post.test.ts index 5778e3413..04d68cb6c 100644 --- a/app/database/operator/server_data_operator/handlers/post.test.ts +++ b/app/database/operator/server_data_operator/handlers/post.test.ts @@ -22,7 +22,7 @@ describe('*** Operator: Post Handlers tests ***', () => { let operator: ServerDataOperator; let posts: Post[] = []; - beforeEach(() => { + beforeEach(async () => { posts = [ { id: '8swgtrrdiff89jnsiwiip3y1eoe', @@ -101,8 +101,8 @@ describe('*** Operator: Post Handlers tests ***', () => { files: [ { id: 'f1oxe5rtepfs7n3zifb4sso7po', - user_id: '89ertha8xpfsumpucqppy5knao', - post_id: 'a7ebyw883trm884p1qcgt8yw4a', + user_id: 'q3mzxua9zjfczqakxdkowc6u6yy', + post_id: '8swgtrrdiff89jnsiwiip3y1eoe', create_at: 1608270920357, update_at: 1608270920357, delete_at: 0, @@ -169,13 +169,15 @@ describe('*** Operator: Post Handlers tests ***', () => { metadata: {}, }, ]; - }); - beforeAll(async () => { await DatabaseManager.init(['baseHandler.test.com']); operator = DatabaseManager.serverDatabases['baseHandler.test.com']!.operator; }); + afterEach(async () => { + DatabaseManager.destroyServerDatabase('baseHandler.test.com'); + }); + it('=> HandleDraft: should write to the the Draft table', async () => { expect.assertions(1); @@ -264,8 +266,8 @@ describe('*** Operator: Post Handlers tests ***', () => { files: [ { id: 'f1oxe5rtepfs7n3zifb4sso7po', - user_id: '89ertha8xpfsumpucqppy5knao', - post_id: 'a7ebyw883trm884p1qcgt8yw4a', + user_id: 'q3mzxua9zjfczqakxdkowc6u6yy', + post_id: '8swgtrrdiff89jnsiwiip3y1eoe', create_at: 1608270920357, update_at: 1608270920357, delete_at: 0, @@ -367,6 +369,174 @@ describe('*** Operator: Post Handlers tests ***', () => { prepareRecordsOnly: true, }); }); + + it('=> HandlePosts: should remove files no longer present in the post', async () => { + const postWithMetadata = posts[0]; + const uploadedFiles = postWithMetadata.metadata.files!; + const updatedPosts: Post[] = [ + { + ...postWithMetadata, + update_at: Date.now(), + metadata: { + ...postWithMetadata.metadata, + files: [], + }, + }, + ]; + + const order = [ + '8swgtrrdiff89jnsiwiip3y1eoe', + '8fcnk3p1jt8mmkaprgajoxz115a', + '3y3w3a6gkbg73bnj3xund9o5ic', + ]; + + const actionType = ActionType.POSTS.RECEIVED_IN_CHANNEL; + + await operator.handlePosts({ + actionType, + order, + posts, + prepareRecordsOnly: false, + }); + + let files = await operator.database.get('File').query(Q.where('post_id', postWithMetadata.id)).fetch(); + expect(files).toHaveLength(1); + expect(files[0].id).toBe(uploadedFiles[0].id); + + await operator.handlePosts({ + actionType, + order: [uploadedFiles[0].id!], + posts: updatedPosts, + }); + + files = await operator.database.get('File').query(Q.where('post_id', postWithMetadata.id)).fetch(); + expect(files).toHaveLength(0); + }); + + it('=> HandlePosts: should add new files if new files are added', async () => { + const postWithMetadata = posts[0]; + const uploadedFiles = postWithMetadata.metadata.files!; + const updatedPosts: Post[] = [ + { + ...postWithMetadata, + update_at: Date.now(), + metadata: { + ...postWithMetadata.metadata, + files: [ + ...postWithMetadata.metadata.files!, + { + id: 'another-file-id', + user_id: 'q3mzxua9zjfczqakxdkowc6u6yy', + post_id: '8swgtrrdiff89jnsiwiip3y1eoe', + create_at: 1608270920357, + update_at: 1608270920357, + delete_at: 0, + name: '4qtwrg.jpg', + extension: 'jpg', + size: 89208, + mime_type: 'image/jpeg', + width: 500, + height: 656, + has_preview_image: true, + mini_preview: + '/9j/2wCEAAMCAgMCAgMDAwMEAwMEBQgFBQQEBQoHBwYIDAoMDAsKCwsNDhIQDQ4RDgsLEBYQERMUFRUVDA8XGBYUGBIUFRQBAwQEBQQFCQUFCRQNCw0UFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFP/AABEIABAAEAMBIgACEQEDEQH/xAGiAAABBQEBAQEBAQAAAAAAAAAAAQIDBAUGBwgJCgsQAAIBAwMCBAMFBQQEAAABfQECAwAEEQUSITFBBhNRYQcicRQygZGhCCNCscEVUtHwJDNicoIJChYXGBkaJSYnKCkqNDU2Nzg5OkNERUZHSElKU1RVVldYWVpjZGVmZ2hpanN0dXZ3eHl6g4SFhoeIiYqSk5SVlpeYmZqio6Slpqeoqaqys7S1tre4ubrCw8TFxsfIycrS09TV1tfY2drh4uPk5ebn6Onq8fLz9PX29/j5+gEAAwEBAQEBAQEBAQAAAAAAAAECAwQFBgcICQoLEQACAQIEBAMEBwUEBAABAncAAQIDEQQFITEGEkFRB2FxEyIygQgUQpGhscEJIzNS8BVictEKFiQ04SXxFxgZGiYnKCkqNTY3ODk6Q0RFRkdISUpTVFVWV1hZWmNkZWZnaGlqc3R1dnd4eXqCg4SFhoeIiYqSk5SVlpeYmZqio6Slpqeoqaqys7S1tre4ubrCw8TFxsfIycrS09TV1tfY2dri4+Tl5ufo6ery8/T19vf4+fr/2gAMAwEAAhEDEQA/AN/T/iZp+pX15FpUmnwLbXtpJpyy2sQLw8CcBXA+bksCDnHGOaf4W+P3xIshbQ6loB8RrbK11f3FpbBFW3ZwiFGHB2kr25BIOeCPPbX4S3407T7rTdDfxFNIpDyRaw9lsB4OECHGR15yO4GK6fRPhR4sGmSnxAs8NgchNOjvDPsjz8qSHA37cDk5JPPFdlOpTdPlcVt/Ku1lrvr17b67EPnjrH8/626H/9k=', + }, + ], + }, + }, + ]; + + const order = [ + '8swgtrrdiff89jnsiwiip3y1eoe', + '8fcnk3p1jt8mmkaprgajoxz115a', + '3y3w3a6gkbg73bnj3xund9o5ic', + ]; + + const actionType = ActionType.POSTS.RECEIVED_IN_CHANNEL; + + await operator.handlePosts({ + actionType, + order, + posts, + prepareRecordsOnly: false, + }); + + let files = await operator.database.get('File').query(Q.where('post_id', postWithMetadata.id)).fetch(); + expect(files).toHaveLength(1); + expect(files[0].id).toBe(uploadedFiles[0].id); + + await operator.handlePosts({ + actionType, + order: [uploadedFiles[0].id!], + posts: updatedPosts, + }); + + files = await operator.database.get('File').query(Q.where('post_id', postWithMetadata.id)).fetch(); + expect(files).toHaveLength(2); + expect(files.map((file) => file.id)).toEqual(expect.arrayContaining(['f1oxe5rtepfs7n3zifb4sso7po', 'another-file-id'])); + }); + + it('=> HandlePosts: should substitute files new files are added and old files are removed', async () => { + const postWithMetadata = posts[0]; + const uploadedFiles = postWithMetadata.metadata.files!; + const updatedPosts: Post[] = [ + { + ...postWithMetadata, + update_at: Date.now(), + metadata: { + ...postWithMetadata.metadata, + files: [ + { + id: 'another-file-id', + user_id: 'q3mzxua9zjfczqakxdkowc6u6yy', + post_id: '8swgtrrdiff89jnsiwiip3y1eoe', + create_at: 1608270920357, + update_at: 1608270920357, + delete_at: 0, + name: '4qtwrg.jpg', + extension: 'jpg', + size: 89208, + mime_type: 'image/jpeg', + width: 500, + height: 656, + has_preview_image: true, + mini_preview: + '/9j/2wCEAAMCAgMCAgMDAwMEAwMEBQgFBQQEBQoHBwYIDAoMDAsKCwsNDhIQDQ4RDgsLEBYQERMUFRUVDA8XGBYUGBIUFRQBAwQEBQQFCQUFCRQNCw0UFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFP/AABEIABAAEAMBIgACEQEDEQH/xAGiAAABBQEBAQEBAQAAAAAAAAAAAQIDBAUGBwgJCgsQAAIBAwMCBAMFBQQEAAABfQECAwAEEQUSITFBBhNRYQcicRQygZGhCCNCscEVUtHwJDNicoIJChYXGBkaJSYnKCkqNDU2Nzg5OkNERUZHSElKU1RVVldYWVpjZGVmZ2hpanN0dXZ3eHl6g4SFhoeIiYqSk5SVlpeYmZqio6Slpqeoqaqys7S1tre4ubrCw8TFxsfIycrS09TV1tfY2drh4uPk5ebn6Onq8fLz9PX29/j5+gEAAwEBAQEBAQEBAQAAAAAAAAECAwQFBgcICQoLEQACAQIEBAMEBwUEBAABAncAAQIDEQQFITEGEkFRB2FxEyIygQgUQpGhscEJIzNS8BVictEKFiQ04SXxFxgZGiYnKCkqNTY3ODk6Q0RFRkdISUpTVFVWV1hZWmNkZWZnaGlqc3R1dnd4eXqCg4SFhoeIiYqSk5SVlpeYmZqio6Slpqeoqaqys7S1tre4ubrCw8TFxsfIycrS09TV1tfY2dri4+Tl5ufo6ery8/T19vf4+fr/2gAMAwEAAhEDEQA/AN/T/iZp+pX15FpUmnwLbXtpJpyy2sQLw8CcBXA+bksCDnHGOaf4W+P3xIshbQ6loB8RrbK11f3FpbBFW3ZwiFGHB2kr25BIOeCPPbX4S3407T7rTdDfxFNIpDyRaw9lsB4OECHGR15yO4GK6fRPhR4sGmSnxAs8NgchNOjvDPsjz8qSHA37cDk5JPPFdlOpTdPlcVt/Ku1lrvr17b67EPnjrH8/626H/9k=', + }, + ], + }, + }, + ]; + + const order = [ + '8swgtrrdiff89jnsiwiip3y1eoe', + '8fcnk3p1jt8mmkaprgajoxz115a', + '3y3w3a6gkbg73bnj3xund9o5ic', + ]; + + const actionType = ActionType.POSTS.RECEIVED_IN_CHANNEL; + + await operator.handlePosts({ + actionType, + order, + posts, + prepareRecordsOnly: false, + }); + + let files = await operator.database.get('File').query(Q.where('post_id', postWithMetadata.id)).fetch(); + expect(files).toHaveLength(1); + expect(files[0].id).toBe(uploadedFiles[0].id); + + await operator.handlePosts({ + actionType, + order: [uploadedFiles[0].id!], + posts: updatedPosts, + }); + + files = await operator.database.get('File').query(Q.where('post_id', postWithMetadata.id)).fetch(); + expect(files).toHaveLength(1); + expect(files[0].id).toBe('another-file-id'); + }); }); describe('*** Operator: merge chunks ***', () => { diff --git a/app/database/operator/server_data_operator/handlers/post.ts b/app/database/operator/server_data_operator/handlers/post.ts index 377aefd67..14793424f 100644 --- a/app/database/operator/server_data_operator/handlers/post.ts +++ b/app/database/operator/server_data_operator/handlers/post.ts @@ -217,8 +217,8 @@ const PostHandler = >(supercla return result; }, new Set()); + const database: Database = this.database; if (deletedPostIds.size) { - const database: Database = this.database; const postsToDelete = await database.get(POST).query(Q.where('id', Q.oneOf(Array.from(deletedPostIds)))).fetch(); if (postsToDelete.length) { await database.write(async () => { @@ -260,6 +260,14 @@ const PostHandler = >(supercla batch.push(...postFiles); } + const allFiles = await database.get(MM_TABLES.SERVER.FILE).query(Q.where('post_id', Q.oneOf(uniquePosts.map((p) => p.id)))).fetch(); + const receivedFilesSet = new Set(files.map((f) => f.id)); + allFiles.forEach((f) => { + if (!receivedFilesSet.has(f.id)) { + batch.push(f.prepareDestroyPermanently()); + } + }); + if (emojis.length) { const postEmojis = await this.handleCustomEmojis({emojis, prepareRecordsOnly: true}); batch.push(...postEmojis); diff --git a/app/hooks/files.test.ts b/app/hooks/files.test.ts new file mode 100644 index 000000000..980a3b371 --- /dev/null +++ b/app/hooks/files.test.ts @@ -0,0 +1,145 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {renderHook} from '@testing-library/react-hooks'; + +import {buildFilePreviewUrl, buildFileUrl} from '@actions/remote/file'; +import {useServerUrl} from '@context/server'; +import {mockFileInfo} from '@test/api_mocks/file'; +import {isGif, isImage, isVideo} from '@utils/file'; + +import {useImageAttachments} from './files'; + +jest.mock('@actions/remote/file', () => ({ + buildFilePreviewUrl: jest.fn(), + buildFileUrl: jest.fn(), +})); + +jest.mock('@utils/file', () => ({ + isGif: jest.fn(), + isImage: jest.fn(), + isVideo: jest.fn(), +})); + +jest.mock('@context/server', () => ({ + useServerUrl: jest.fn(), +})); + +describe('useImageAttachments', () => { + const serverUrl = 'https://example.com'; + + beforeEach(() => { + jest.mocked(useServerUrl).mockReturnValue(serverUrl); + }); + + it('should separate images and non-images correctly', () => { + const filesInfo = [ + mockFileInfo({id: '1', localPath: 'path/to/image1', uri: `${serverUrl}/files/image1`}), + mockFileInfo({id: '2', localPath: 'path/to/video1', uri: `${serverUrl}/files/video1`}), + mockFileInfo({id: '3', localPath: 'path/to/file1', uri: `${serverUrl}/files/file1`}), + ]; + + jest.mocked(isImage).mockImplementation((file) => file?.id === '1'); + jest.mocked(isVideo).mockImplementation((file) => file?.id === '2'); + jest.mocked(isGif).mockReturnValue(false); + jest.mocked(buildFilePreviewUrl).mockImplementation((url, id) => `${url}/preview/${id}`); + jest.mocked(buildFileUrl).mockImplementation((url, id) => `${url}/file/${id}`); + + const {result} = renderHook(() => useImageAttachments(filesInfo)); + + expect(result.current.images).toEqual([ + mockFileInfo({id: '1', localPath: 'path/to/image1', uri: 'path/to/image1'}), + mockFileInfo({id: '2', localPath: 'path/to/video1', uri: 'path/to/video1'}), + ]); + + expect(result.current.nonImages).toEqual([ + mockFileInfo({id: '3', localPath: 'path/to/file1', uri: `${serverUrl}/files/file1`}), + ]); + }); + + it('should use preview URL for images without localPath', () => { + const filesInfo = [ + mockFileInfo({id: '1', localPath: '', uri: `${serverUrl}/files/image1`}), + ]; + + jest.mocked(isImage).mockReturnValue(true); + jest.mocked(isVideo).mockReturnValue(false); + jest.mocked(isGif).mockReturnValue(false); + jest.mocked(buildFilePreviewUrl).mockImplementation((url, id) => `${url}/preview/${id}`); + + const {result} = renderHook(() => useImageAttachments(filesInfo)); + + expect(result.current.images).toEqual([ + mockFileInfo({id: '1', localPath: '', uri: 'https://example.com/preview/1'}), + ]); + }); + + it('should use file URL for gifs and videos without local path', () => { + const filesInfo = [ + mockFileInfo({id: '1', localPath: '', uri: `${serverUrl}/files/image1`}), + mockFileInfo({id: '2', localPath: '', uri: `${serverUrl}/files/video1`}), + ]; + + jest.mocked(isImage).mockImplementation((file) => file?.id === '1' || file?.id === '3'); + jest.mocked(isVideo).mockImplementation((file) => file?.id === '2'); + jest.mocked(isGif).mockImplementation((file) => file?.id === '1'); + jest.mocked(buildFileUrl).mockImplementation((url, id) => `${url}/file/${id}`); + + const {result} = renderHook(() => useImageAttachments(filesInfo)); + + expect(result.current.images).toEqual([ + mockFileInfo({id: '1', localPath: '', uri: 'https://example.com/file/1'}), + mockFileInfo({id: '2', localPath: '', uri: 'https://example.com/file/2'}), + ]); + }); + + it('should return the same values if the same arguments are passed', () => { + const filesInfo = [ + mockFileInfo({id: '1', localPath: 'path/to/image1', uri: `${serverUrl}/files/image1`}), + mockFileInfo({id: '2', localPath: 'path/to/video1', uri: `${serverUrl}/files/video1`}), + ]; + + jest.mocked(isImage).mockImplementation((file) => file?.id === '1'); + jest.mocked(isVideo).mockImplementation((file) => file?.id === '2'); + jest.mocked(isGif).mockReturnValue(false); + jest.mocked(buildFilePreviewUrl).mockImplementation((url, id) => `${url}/preview/${id}`); + jest.mocked(buildFileUrl).mockImplementation((url, id) => `${url}/file/${id}`); + + const {result, rerender} = renderHook(() => useImageAttachments(filesInfo)); + + const firstResult = result.current; + + rerender(); + + const secondResult = result.current; + + expect(firstResult).toBe(secondResult); + }); + + it('should handle empty filesInfo array', () => { + const filesInfo: FileInfo[] = []; + + const {result} = renderHook(() => useImageAttachments(filesInfo)); + + expect(result.current.images).toEqual([]); + expect(result.current.nonImages).toEqual([]); + }); + + it('should handle files with no id', () => { + const filesInfo = [ + mockFileInfo({id: '', localPath: 'path/to/image1', uri: `${serverUrl}/files/image1`}), + mockFileInfo({id: '', localPath: '', uri: `${serverUrl}/files/image2`}), + ]; + + jest.mocked(isImage).mockReturnValue(true); + jest.mocked(isVideo).mockReturnValue(false); + jest.mocked(isGif).mockReturnValue(false); + jest.mocked(buildFilePreviewUrl).mockImplementation((url, id) => `${url}/preview/${id}`); + + const {result} = renderHook(() => useImageAttachments(filesInfo)); + + expect(result.current.images).toEqual([ + mockFileInfo({id: '', localPath: 'path/to/image1', uri: 'path/to/image1'}), + ]); + }); +}); diff --git a/app/hooks/files.ts b/app/hooks/files.ts index 49da59e51..98711060f 100644 --- a/app/hooks/files.ts +++ b/app/hooks/files.ts @@ -49,32 +49,30 @@ const getFileInfo = async (serverUrl: string, bookmarks: ChannelBookmarkModel[], } }; -export const useImageAttachments = (filesInfo: FileInfo[], publicLinkEnabled: boolean) => { +export const useImageAttachments = (filesInfo: FileInfo[]) => { const serverUrl = useServerUrl(); return useMemo(() => { return filesInfo.reduce(({images, nonImages}: {images: FileInfo[]; nonImages: FileInfo[]}, file) => { const imageFile = isImage(file); const videoFile = isVideo(file); - if (imageFile || (videoFile && publicLinkEnabled)) { + if (imageFile || videoFile) { let uri; if (file.localPath) { uri = file.localPath; } else { - uri = (isGif(file) || videoFile) ? buildFileUrl(serverUrl, file.id!) : buildFilePreviewUrl(serverUrl, file.id!); + // If no local path and no id, we skip the image + if (!file.id) { + return {images, nonImages}; + } + uri = (isGif(file) || videoFile) ? buildFileUrl(serverUrl, file.id) : buildFilePreviewUrl(serverUrl, file.id); } images.push({...file, uri}); } else { - let uri = file.uri; - if (videoFile) { - // fallback if public links are not enabled - uri = buildFileUrl(serverUrl, file.id!); - } - - nonImages.push({...file, uri}); + nonImages.push({...file}); } return {images, nonImages}; }, {images: [], nonImages: []}); - }, [filesInfo, publicLinkEnabled]); + }, [filesInfo, serverUrl]); }; export const useChannelBookmarkFiles = (bookmarks: ChannelBookmarkModel[], publicLinkEnabled: boolean) => { diff --git a/app/hooks/gallery.ts b/app/hooks/gallery.ts index 763c6a95d..13e07db0b 100644 --- a/app/hooks/gallery.ts +++ b/app/hooks/gallery.ts @@ -212,7 +212,7 @@ export function useGalleryItem( ) { const gallery = useGallery(identifier); const ref = useAnimatedRef(); - const {opacity, activeIndex} = gallery!.sharedValues; + const {opacity, activeIndex} = gallery.sharedValues; const styles = useAnimatedStyle(() => { return { diff --git a/app/hooks/utils.test.tsx b/app/hooks/utils.test.tsx new file mode 100644 index 000000000..bd7aeeeda --- /dev/null +++ b/app/hooks/utils.test.tsx @@ -0,0 +1,73 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {renderHook, act} from '@testing-library/react-hooks'; + +import {usePreventDoubleTap} from './utils'; + +describe('usePreventDoubleTap', () => { + const callback = jest.fn(); + + it('should allow the first tap', () => { + const {result} = renderHook(() => usePreventDoubleTap(callback)); + + act(() => { + result.current(); + }); + + expect(callback).toHaveBeenCalled(); + }); + + it('should prevent the second tap within the delay', () => { + const {result} = renderHook(() => usePreventDoubleTap(callback)); + + act(() => { + result.current(); + }); + + expect(callback).toHaveBeenCalledTimes(1); + + act(() => { + result.current(); + }); + + expect(callback).toHaveBeenCalledTimes(1); + }); + + it('should allow the tap after the delay', () => { + jest.useFakeTimers(); + + const {result} = renderHook(() => usePreventDoubleTap(callback)); + + act(() => { + result.current(); + }); + + expect(callback).toHaveBeenCalledTimes(1); + + act(() => { + result.current(); + }); + + expect(callback).toHaveBeenCalledTimes(1); + + act(() => { + jest.advanceTimersByTime(750); + result.current(); + }); + + expect(callback).toHaveBeenCalledTimes(2); + jest.useRealTimers(); + }); + it('should return the same result if the same callback is passed', () => { + const {result, rerender} = renderHook(() => usePreventDoubleTap(callback)); + + const firstResult = result.current; + + rerender(); + + const secondResult = result.current; + + expect(firstResult).toBe(secondResult); + }); +}); diff --git a/app/hooks/utils.ts b/app/hooks/utils.ts new file mode 100644 index 000000000..314d4d0f8 --- /dev/null +++ b/app/hooks/utils.ts @@ -0,0 +1,19 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {useCallback, useRef} from 'react'; + +const DELAY = 750; + +export const usePreventDoubleTap = (callback: T) => { + const lastTapRef = useRef(null); + + return useCallback((...args: unknown[]) => { + const now = Date.now(); + if (lastTapRef.current && now - lastTapRef.current < DELAY) { + return; + } + lastTapRef.current = now; + callback(...args); + }, [callback]); +}; diff --git a/test/api_mocks/file.ts b/test/api_mocks/file.ts new file mode 100644 index 000000000..528879a5a --- /dev/null +++ b/test/api_mocks/file.ts @@ -0,0 +1,19 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +export function mockFileInfo(overwrite: Partial = {}): FileInfo { + return { + id: '1', + localPath: 'path/to/image1', + uri: '', + has_preview_image: true, + extension: 'png', + height: 100, + width: 100, + mime_type: 'image/png', + name: 'image1', + size: 100, + user_id: '1', + ...overwrite, + }; +}