From 10cbd31801f32097d5679e05d0258c413a1b75a1 Mon Sep 17 00:00:00 2001 From: Rajat Dabade Date: Mon, 7 Jul 2025 16:24:27 +0530 Subject: [PATCH] MM-64415 Attachment style updated for draft and Edit post screen (#8961) * MM-64415 Attachment style updated for draft and Edit post screen * Minor * constants * Addressed Review comments * UX review comments * Error state changes * Fix the style issue for retry state on the attachment * Fix some styling * Fix the space extra char issue in post --- .../post_draft/uploads/index.test.tsx | 238 ++++++++++++++++++ app/components/post_draft/uploads/index.tsx | 2 +- .../uploads/upload_item/index.test.tsx | 209 ++++++++++++++- .../post_draft/uploads/upload_item/index.tsx | 204 +++++++++++---- .../uploads/upload_item/upload_remove.tsx | 4 +- .../uploads/upload_item/upload_retry.tsx | 6 +- app/screens/edit_post/edit_post.test.tsx | 59 +++-- app/screens/edit_post/edit_post.tsx | 36 +-- .../edit_post_input/edit_post_input.test.tsx | 1 - .../edit_post_input/edit_post_input.tsx | 4 +- app/screens/edit_post/post_error/index.tsx | 58 +++-- 11 files changed, 705 insertions(+), 116 deletions(-) create mode 100644 app/components/post_draft/uploads/index.test.tsx diff --git a/app/components/post_draft/uploads/index.test.tsx b/app/components/post_draft/uploads/index.test.tsx new file mode 100644 index 000000000..c11a07125 --- /dev/null +++ b/app/components/post_draft/uploads/index.test.tsx @@ -0,0 +1,238 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React from 'react'; + +import {renderWithEverything} from '@test/intl-test-helper'; +import TestHelper from '@test/test_helper'; + +import Uploads from './index'; + +import type {Database} from '@nozbe/watermelondb'; + +jest.mock('@managers/draft_upload_manager', () => ({ + isUploading: jest.fn(() => false), + registerProgressHandler: jest.fn(() => jest.fn()), +})); + +jest.mock('@utils/gallery', () => ({ + fileToGalleryItem: jest.fn((file) => ({ + id: file.id, + type: 'image', + uri: file.localPath || file.uri, + name: file.name, + width: file.width, + height: file.height, + })), + openGalleryAtIndex: jest.fn(), +})); + +describe('Uploads', () => { + const serverUrl = 'serverUrl'; + let database: Database; + + beforeEach(async () => { + jest.clearAllMocks(); + database = (await TestHelper.setupServerDatabase(serverUrl)).database; + }); + + const baseProps = { + currentUserId: 'currentUserId', + channelId: 'channelId', + rootId: 'rootId', + uploadFileError: null, + }; + + it('should render without files', () => { + const props = { + ...baseProps, + files: [], + }; + + const {getByTestId} = renderWithEverything( + , + {database}, + ); + + expect(getByTestId('uploads')).toBeTruthy(); + }); + + it('should render with image files', () => { + const imageFiles = [ + { + id: 'image1', + clientId: 'client1', + name: 'image1.jpg', + extension: 'jpg', + mime_type: 'image/jpeg', + size: 1024, + width: 800, + height: 600, + localPath: '/path/to/image1.jpg', + failed: false, + }, + { + id: 'image2', + clientId: 'client2', + name: 'image2.png', + extension: 'png', + mime_type: 'image/png', + size: 2048, + width: 1024, + height: 768, + localPath: '/path/to/image2.png', + failed: false, + }, + ] as FileInfo[]; + + const props = { + ...baseProps, + files: imageFiles, + }; + + const {getByTestId} = renderWithEverything( + , + {database}, + ); + + expect(getByTestId('uploads')).toBeTruthy(); + }); + + it('should render with document files', () => { + const documentFiles = [ + { + id: 'doc1', + clientId: 'client1', + name: 'document.pdf', + extension: 'pdf', + mime_type: 'application/pdf', + size: 5120, + width: 0, + height: 0, + localPath: '/path/to/document.pdf', + failed: false, + }, + { + id: 'doc2', + clientId: 'client2', + name: 'spreadsheet.xlsx', + extension: 'xlsx', + mime_type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + size: 3072, + width: 0, + height: 0, + localPath: '/path/to/spreadsheet.xlsx', + failed: false, + }, + ] as FileInfo[]; + + const props = { + ...baseProps, + files: documentFiles, + }; + + const {getByTestId} = renderWithEverything( + , + {database}, + ); + + expect(getByTestId('uploads')).toBeTruthy(); + }); + + it('should render with mixed file types', () => { + const mixedFiles = [ + { + id: 'image1', + clientId: 'client1', + name: 'photo.jpg', + extension: 'jpg', + mime_type: 'image/jpeg', + size: 1024, + width: 800, + height: 600, + localPath: '/path/to/photo.jpg', + failed: false, + }, + { + id: 'doc1', + clientId: 'client2', + name: 'report.pdf', + extension: 'pdf', + mime_type: 'application/pdf', + size: 5120, + width: 0, + height: 0, + localPath: '/path/to/report.pdf', + failed: false, + }, + ] as FileInfo[]; + + const props = { + ...baseProps, + files: mixedFiles, + }; + + const {getByTestId} = renderWithEverything( + , + {database}, + ); + + expect(getByTestId('uploads')).toBeTruthy(); + }); + + it('should display upload error when present', () => { + const props = { + ...baseProps, + files: [], + uploadFileError: 'File upload failed', + }; + + const {getByText} = renderWithEverything( + , + {database}, + ); + + expect(getByText('File upload failed')).toBeTruthy(); + }); + + it('should filter out failed and uploading files from gallery', () => { + const filesWithFailedAndUploading = [ + { + id: 'file1', + clientId: 'client1', + name: 'success.jpg', + extension: 'jpg', + mime_type: 'image/jpeg', + size: 1024, + width: 800, + height: 600, + localPath: '/path/to/success.jpg', + failed: false, + }, + { + id: 'file2', + clientId: 'client2', + name: 'failed.jpg', + extension: 'jpg', + mime_type: 'image/jpeg', + size: 1024, + width: 800, + height: 600, + localPath: '/path/to/failed.jpg', + failed: true, + }, + ] as FileInfo[]; + + const props = { + ...baseProps, + files: filesWithFailedAndUploading, + }; + + const {getByTestId} = renderWithEverything( + , + {database}, + ); + + expect(getByTestId('uploads')).toBeTruthy(); + }); +}); diff --git a/app/components/post_draft/uploads/index.tsx b/app/components/post_draft/uploads/index.tsx index 6305e7ef1..83c5de377 100644 --- a/app/components/post_draft/uploads/index.tsx +++ b/app/components/post_draft/uploads/index.tsx @@ -18,7 +18,7 @@ import {makeStyleSheetFromTheme} from '@utils/theme'; import UploadItem from './upload_item'; -const CONTAINER_HEIGHT_MAX = 67; +const CONTAINER_HEIGHT_MAX = 80; const CONTAINER_HEIGHT_MIN = 0; const ERROR_HEIGHT_MAX = 20; const ERROR_HEIGHT_MIN = 0; diff --git a/app/components/post_draft/uploads/upload_item/index.test.tsx b/app/components/post_draft/uploads/upload_item/index.test.tsx index a243b4987..bd5f2bffb 100644 --- a/app/components/post_draft/uploads/upload_item/index.test.tsx +++ b/app/components/post_draft/uploads/upload_item/index.test.tsx @@ -4,6 +4,8 @@ import React from 'react'; import {updateDraftFile} from '@actions/local/draft'; +import FileIcon from '@components/files/file_icon'; +import ImageFile from '@components/files/image_file'; import {EditPostProvider} from '@context/edit_post'; import DraftEditPostUploadManager from '@managers/draft_upload_manager'; import {fireEvent, renderWithEverything} from '@test/intl-test-helper'; @@ -29,6 +31,25 @@ jest.mock('@context/server', () => ({ useServerUrl: () => 'serverUrl', })); +jest.mock('@utils/file', () => ({ + isImage: jest.fn(), + getFormattedFileSize: jest.fn((size) => `${size} KB`), +})); + +jest.mock('@components/files/image_file', () => ({ + __esModule: true, + default: jest.fn(), +})); +jest.mocked(ImageFile).mockImplementation((props) => React.createElement('ImageFile', {testID: 'image-file', ...props})); + +jest.mock('@components/files/file_icon', () => ({ + __esModule: true, + default: jest.fn(), +})); +jest.mocked(FileIcon).mockImplementation((props) => React.createElement('FileIcon', {...props})); + +const {isImage} = require('@utils/file'); + describe('UploadItem', () => { const serverUrl = 'serverUrl'; let database: Database; @@ -38,13 +59,13 @@ describe('UploadItem', () => { database = (await TestHelper.setupServerDatabase(serverUrl)).database; }); - const baseProps: Parameters[0] = { - channelId: 'channelId', - galleryIdentifier: 'galleryIdentifier', - index: 0, + const baseProps = { + channelId: 'channelId' as string, + galleryIdentifier: 'galleryIdentifier' as string, + index: 0 as number, file: { - id: 'fileId', - name: 'fileName', + id: 'id', + name: 'name', extension: 'extension', has_preview_image: false, height: 0, @@ -53,12 +74,188 @@ describe('UploadItem', () => { width: 0, bytesRead: 0, clientId: 'clientId', + localPath: 'localPath', failed: false, } as FileInfo, openGallery: jest.fn(), rootId: 'rootId' as string, }; + describe('Image Files', () => { + beforeEach(() => { + isImage.mockReturnValue(true); + }); + + it('should display thumbnail for image files', () => { + const imageFile = { + ...baseProps.file, + name: 'image.jpg', + extension: 'jpg', + mime_type: 'image/jpeg', + size: 1024, + } as FileInfo; + + const props = { + ...baseProps, + file: imageFile, + }; + + const {getByTestId, queryByText} = renderWithEverything( + + + , + {database}, + ); + + // Should display image thumbnail + expect(getByTestId('image-file')).toBeTruthy(); + + // For image files, file name and size should not be displayed + expect(queryByText('image.jpg')).toBeNull(); + expect(queryByText('JPG')).toBeNull(); + expect(queryByText('1024 KB')).toBeNull(); + }); + + it('should not display file info for image files', () => { + const imageFile = { + ...baseProps.file, + name: 'test-image.png', + extension: 'png', + mime_type: 'image/png', + size: 2048, + } as FileInfo; + + const props = { + ...baseProps, + file: imageFile, + }; + + const {queryByText} = renderWithEverything( + + + , + {database}, + ); + + // Should not display file name or size information + expect(queryByText('test-image.png')).toBeNull(); + expect(queryByText('PNG')).toBeNull(); + expect(queryByText('2048 KB')).toBeNull(); + }); + }); + + describe('Non-Image Files', () => { + beforeEach(() => { + isImage.mockReturnValue(false); + }); + + it('should display file name, extension, and size for non-image files', () => { + const documentFile = { + ...baseProps.file, + name: 'document.pdf', + extension: 'pdf', + mime_type: 'application/pdf', + size: 5120, + } as FileInfo; + + const props = { + ...baseProps, + file: documentFile, + }; + + const {getByText, getByTestId} = renderWithEverything( + + + , + {database}, + ); + + // Should display file icon + expect(getByTestId('id')).toBeTruthy(); + + // Should display file name + expect(getByText('document.pdf')).toBeTruthy(); + + // Should display extension and formatted size + expect(getByText('PDF 5120 KB')).toBeTruthy(); + }); + + it('should display file info for different file types', () => { + const excelFile = { + ...baseProps.file, + name: 'spreadsheet.xlsx', + extension: 'xlsx', + mime_type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + size: 3072, + } as FileInfo; + + const props = { + ...baseProps, + file: excelFile, + }; + + const {getByText} = renderWithEverything( + + + , + {database}, + ); + + expect(getByText('spreadsheet.xlsx')).toBeTruthy(); + expect(getByText('XLSX 3072 KB')).toBeTruthy(); + }); + + it('should handle files without extension gracefully', () => { + const fileWithoutExtension = { + ...baseProps.file, + name: 'document', + extension: '', + mime_type: 'application/octet-stream', + size: 1024, + } as FileInfo; + + const props = { + ...baseProps, + file: fileWithoutExtension, + }; + + const {getByText} = renderWithEverything( + + + , + {database}, + ); + + expect(getByText('document')).toBeTruthy(); + expect(getByText('DOCUMENT 1024 KB')).toBeTruthy(); + }); + + it('should extract extension from file name when extension field is missing', () => { + const fileNameWithExtension = { + ...baseProps.file, + name: 'report.docx', + extension: '', + mime_type: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', + size: 2048, + } as FileInfo; + + const props = { + ...baseProps, + file: fileNameWithExtension, + }; + + const {getByText} = renderWithEverything( + + + , + {database}, + ); + + expect(getByText('report.docx')).toBeTruthy(); + expect(getByText('DOCX 2048 KB')).toBeTruthy(); + }); + }); + it('When file is failed, onclick of retry button, it should call prepareUpload with correct arguments in edit mode', () => { const updateFileCallback = jest.fn(); const failedFile = { diff --git a/app/components/post_draft/uploads/upload_item/index.tsx b/app/components/post_draft/uploads/upload_item/index.tsx index 306cf7dc6..802f20ca5 100644 --- a/app/components/post_draft/uploads/upload_item/index.tsx +++ b/app/components/post_draft/uploads/upload_item/index.tsx @@ -2,7 +2,7 @@ // See LICENSE.txt for license information. import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; -import {StyleSheet, TouchableWithoutFeedback, View} from 'react-native'; +import {TouchableWithoutFeedback, View, Text} from 'react-native'; import Animated from 'react-native-reanimated'; import {updateDraftFile} from '@actions/local/draft'; @@ -15,8 +15,9 @@ import {useTheme} from '@context/theme'; import useDidUpdate from '@hooks/did_update'; import {useGalleryItem} from '@hooks/gallery'; import DraftEditPostUploadManager from '@managers/draft_upload_manager'; -import {isImage} from '@utils/file'; -import {changeOpacity} from '@utils/theme'; +import {isImage, getFormattedFileSize} from '@utils/file'; +import {changeOpacity, makeStyleSheetFromTheme} from '@utils/theme'; +import {typography} from '@utils/typography'; import UploadRemove from './upload_remove'; import UploadRetry from './upload_retry'; @@ -30,34 +31,112 @@ type Props = { rootId: string; } -const style = StyleSheet.create({ - preview: { - paddingTop: 5, - marginLeft: 12, - }, - previewContainer: { - height: 56, - width: 56, - borderRadius: 4, - }, - progress: { - backgroundColor: 'rgba(0, 0, 0, 0.1)', - height: 53, - width: 53, - justifyContent: 'flex-end', - position: 'absolute', - borderRadius: 4, - paddingLeft: 3, - }, - progressContainer: { - paddingVertical: undefined, - position: undefined, - justifyContent: undefined, - }, - filePreview: { - width: 56, - height: 56, - }, +// Constants for dimensions +const DIMENSIONS = { + THUMBNAIL_SIZE: 64, + ICON_SIZE: 48, + FILE_CONTAINER_WIDTH: 264, + FILE_CONTAINER_HEIGHT: 64, +} as const; + +const getStyleSheet = makeStyleSheetFromTheme((theme) => { + return { + preview: { + paddingTop: 5, + marginLeft: 12, + }, + previewContainer: { + borderRadius: 4, + borderWidth: 1, + borderColor: changeOpacity(theme.centerChannelColor, 0.16), + backgroundColor: theme.centerChannelBg, + alignItems: 'center', + position: 'relative', + }, + imageOnlyContainer: { + width: DIMENSIONS.THUMBNAIL_SIZE, + height: DIMENSIONS.THUMBNAIL_SIZE, + padding: 0, + }, + fileWithInfoContainer: { + width: DIMENSIONS.FILE_CONTAINER_WIDTH, + height: DIMENSIONS.FILE_CONTAINER_HEIGHT, + flexDirection: 'row', + alignItems: 'center', + flexShrink: 0, + gap: 8, + paddingVertical: 12, + paddingLeft: 8, + paddingRight: 16, + }, + iconContainer: { + width: DIMENSIONS.ICON_SIZE, + height: DIMENSIONS.ICON_SIZE, + borderRadius: 4, + justifyContent: 'center', + alignItems: 'center', + }, + imageContainer: { + width: DIMENSIONS.THUMBNAIL_SIZE, + height: DIMENSIONS.THUMBNAIL_SIZE, + borderRadius: 4, + marginRight: 8, + overflow: 'hidden', + borderWidth: 1, + borderColor: changeOpacity(theme.centerChannelColor, 0.16), + shadowColor: '#000000', + shadowOffset: { + width: 0, + height: 2, + }, + shadowOpacity: 0.08, + shadowRadius: 3, + elevation: 1, + }, + imageOnlyThumbnail: { + width: DIMENSIONS.THUMBNAIL_SIZE, + height: DIMENSIONS.THUMBNAIL_SIZE, + borderRadius: 4, + overflow: 'hidden', + shadowColor: '#000000', + shadowOffset: { + width: 0, + height: 2, + }, + shadowOpacity: 0.08, + shadowRadius: 3, + elevation: 1, + }, + fileInfo: { + flex: 1, + justifyContent: 'center', + alignItems: 'flex-start', + }, + fileName: { + color: theme.centerChannelColor, + ...typography('Body', 200, 'SemiBold'), + marginBottom: 2, + }, + fileSize: { + color: changeOpacity(theme.centerChannelColor, 0.64), + ...typography('Body', 75, 'Regular'), + }, + progress: { + position: 'absolute', + left: 0, + right: 0, + top: 0, + bottom: 0, + backgroundColor: 'rgba(0, 0, 0, 0.1)', + borderRadius: 4, + justifyContent: 'flex-end', + }, + progressContainer: { + paddingVertical: undefined, + position: undefined, + justifyContent: undefined, + }, + }; }); export default function UploadItem({ @@ -65,6 +144,7 @@ export default function UploadItem({ rootId, openGallery, }: Props) { const theme = useTheme(); + const style = getStyleSheet(theme); const serverUrl = useServerUrl(); const removeCallback = useRef<(() => void) | undefined>(undefined); const [progress, setProgress] = useState(0); @@ -127,37 +207,67 @@ export default function UploadItem({ const {styles, onGestureEvent, ref} = useGalleryItem(galleryIdentifier, index, handlePress); - const filePreviewComponent = useMemo(() => { + const fileDisplayComponent = () => { if (isImage(file)) { return ( - + + + ); } return ( - + + + ); - }, [file, ref, theme.centerChannelColor]); + }; + + const fileExtension = file.extension?.toUpperCase() || file.name?.split('.').pop()?.toUpperCase() || ''; + + const formattedSize = getFormattedFileSize(file.size || 0); + + const isImageFile = isImage(file); + const containerStyle = useMemo(() => [ + style.previewContainer, + isImageFile ? style.imageOnlyContainer : style.fileWithInfoContainer, + ], [isImageFile, style.fileWithInfoContainer, style.imageOnlyContainer, style.previewContainer]); return ( - + - - {filePreviewComponent} + + {fileDisplayComponent()} + + {!isImageFile && ( + + + {file.name || 'Unknown file'} + + + {fileExtension && `${fileExtension} `}{formattedSize} + + + )} + {file.failed && { tappableContainer: { position: 'absolute', elevation: 11, - top: -7, - right: -8, + top: -12, + right: -10, width: 24, height: 24, }, diff --git a/app/components/post_draft/uploads/upload_item/upload_retry.tsx b/app/components/post_draft/uploads/upload_item/upload_retry.tsx index 7053166c0..edf039a39 100644 --- a/app/components/post_draft/uploads/upload_item/upload_retry.tsx +++ b/app/components/post_draft/uploads/upload_item/upload_retry.tsx @@ -15,8 +15,10 @@ const style = StyleSheet.create({ failed: { backgroundColor: 'rgba(0, 0, 0, 0.8)', position: 'absolute', - height: '100%', - width: '100%', + left: 0, + right: 0, + top: 0, + bottom: 0, alignItems: 'center', justifyContent: 'center', borderRadius: 4, diff --git a/app/screens/edit_post/edit_post.test.tsx b/app/screens/edit_post/edit_post.test.tsx index 7d8d873b4..2fb2cece4 100644 --- a/app/screens/edit_post/edit_post.test.tsx +++ b/app/screens/edit_post/edit_post.test.tsx @@ -4,7 +4,6 @@ import React from 'react'; import {Alert} from 'react-native'; -import useFileUploadError from '@hooks/file_upload_error'; import DraftEditPostUploadManager from '@managers/draft_upload_manager'; import {fireEvent, renderWithEverything} from '@test/intl-test-helper'; import TestHelper from '@test/test_helper'; @@ -15,7 +14,6 @@ import EditPost from './edit_post'; import type {Database} from '@nozbe/watermelondb'; import type PostModel from '@typings/database/models/servers/post'; -jest.mock('@hooks/file_upload_error'); jest.mock('@utils/file/file_picker'); jest.mock('@managers/draft_upload_manager', () => ({ prepareUpload: jest.fn(), @@ -84,7 +82,6 @@ const ERROR_MESSAGES = { describe('Edit Post', () => { let database: Database; - let mockNewUploadError: jest.Mock; const baseProps: Parameters[0] = { componentId: 'EditPost', @@ -107,14 +104,6 @@ describe('Edit Post', () => { canUploadFiles: true, }; - const setupMocks = () => { - mockNewUploadError = jest.fn(); - jest.mocked(useFileUploadError).mockReturnValue({ - uploadError: null, - newUploadError: mockNewUploadError, - }); - }; - const setupPickerMock = (file: Partial) => { jest.mocked(PickerUtil).mockImplementation((intl, onUploadFiles) => ({ attachFileFromFiles: jest.fn(() => { @@ -157,7 +146,6 @@ describe('Edit Post', () => { beforeEach(() => { jest.clearAllMocks(); - setupMocks(); }); describe('Rendering', () => { @@ -177,7 +165,7 @@ describe('Edit Post', () => { const screen = renderEditPost(props); triggerFileUpload(screen); - expect(mockNewUploadError).toHaveBeenCalledWith(ERROR_MESSAGES.uploadsDisabled); + expect(screen.getByText(ERROR_MESSAGES.uploadsDisabled)).toBeVisible(); }); it('should show error when maximum file count is reached', () => { @@ -186,7 +174,7 @@ describe('Edit Post', () => { const screen = renderEditPost(props); triggerFileUpload(screen); - expect(mockNewUploadError).toHaveBeenCalledWith(ERROR_MESSAGES.maxFilesReached); + expect(screen.getByText(ERROR_MESSAGES.maxFilesReached)).toBeVisible(); }); it('should show error when file size exceeds limit', () => { @@ -195,7 +183,7 @@ describe('Edit Post', () => { const screen = renderEditPost(props); triggerFileUpload(screen); - expect(mockNewUploadError).toHaveBeenCalledWith(ERROR_MESSAGES.fileTooLarge); + expect(screen.getByText(ERROR_MESSAGES.fileTooLarge)).toBeVisible(); }); }); @@ -216,9 +204,8 @@ describe('Edit Post', () => { ); expect(DraftEditPostUploadManager.registerErrorHandler).toHaveBeenCalledWith( TEST_FILES.newFile.clientId, - mockNewUploadError, + expect.any(Function), ); - expect(mockNewUploadError).toHaveBeenCalledWith(null); }); }); @@ -246,4 +233,42 @@ describe('Edit Post', () => { ); }); }); + + describe('Error Display', () => { + it('should display upload error in PostError component when file upload fails', () => { + setupPickerMock(TEST_FILES.largeFile); + const props = {...baseProps, maxFileSize: 1000}; + const screen = renderEditPost(props); + + // Trigger file upload that will cause size error + triggerFileUpload(screen); + + // Verify that the PostError component is displayed with the error + expect(screen.getByTestId('edit_post.message.input.error')).toBeVisible(); + expect(screen.getByText(ERROR_MESSAGES.fileTooLarge)).toBeVisible(); + }); + + it('should display upload error with divider when error is present', () => { + setupPickerMock(TEST_FILES.smallFile); + const props = {...baseProps, canUploadFiles: false}; + const screen = renderEditPost(props); + + // Trigger file upload that will cause upload disabled error + triggerFileUpload(screen); + + // Verify that the error is displayed + expect(screen.getByText(ERROR_MESSAGES.uploadsDisabled)).toBeVisible(); + + // Verify that the PostError component has the hasError styling applied + const editPostInput = screen.getByTestId('edit_post.message.input'); + expect(editPostInput).toBeVisible(); + }); + + it('should not display PostError component when no upload error exists', () => { + const screen = renderEditPost(); + + // Verify that no error message is displayed + expect(screen.queryByTestId('edit_post.message.input.error')).toBeNull(); + }); + }); }); diff --git a/app/screens/edit_post/edit_post.tsx b/app/screens/edit_post/edit_post.tsx index 1964630f3..465f7c1bb 100644 --- a/app/screens/edit_post/edit_post.tsx +++ b/app/screens/edit_post/edit_post.tsx @@ -18,7 +18,6 @@ import useAndroidHardwareBackHandler from '@hooks/android_back_handler'; import {useAutocompleteDefaultAnimatedValues} from '@hooks/autocomplete'; import {useKeyboardOverlap} from '@hooks/device'; import useDidUpdate from '@hooks/did_update'; -import useFileUploadError from '@hooks/file_upload_error'; import {useInputPropagation} from '@hooks/input'; import useNavButtonPressed from '@hooks/navigation_button_pressed'; import DraftEditPostUploadManager from '@managers/draft_upload_manager'; @@ -103,13 +102,11 @@ const EditPost = ({ const serverUrl = useServerUrl(); const shouldDeleteOnSave = !postMessage && canDelete && !hasFilesAttached; - const {uploadError, newUploadError} = useFileUploadError(); const shouldEnableSaveButton = useCallback(() => { const loadingFiles = postFiles.filter((v) => v.clientId && DraftEditPostUploadManager.isUploading(v.clientId)); const hasUploadingFiles = loadingFiles.length > 0; - - const tooLong = postMessage.trim().length > maxPostSize; + const tooLong = postMessage.length > maxPostSize; const messageChanged = editingMessage !== postMessage; @@ -189,20 +186,20 @@ const EditPost = ({ } if (!canUploadFiles) { - newUploadError(uploadDisabledWarning(intl)); + setErrorLine(uploadDisabledWarning(intl)); return; } const currentFileCount = postFiles?.length || 0; const availableCount = maxFileCount - currentFileCount; if (newFiles.length > availableCount) { - newUploadError(fileMaxWarning(intl, maxFileCount)); + setErrorLine(fileMaxWarning(intl, maxFileCount)); return; } const largeFile = newFiles.find((file) => file.size > maxFileSize); if (largeFile) { - newUploadError(fileSizeWarning(intl, maxFileSize)); + setErrorLine(fileSizeWarning(intl, maxFileSize)); return; } @@ -219,11 +216,14 @@ const EditPost = ({ true, // isEditPost = true updateFileInPostFiles, ); - uploadErrorHandlers.current[file.clientId!] = DraftEditPostUploadManager.registerErrorHandler(file.clientId!, newUploadError); + uploadErrorHandlers.current[file.clientId!] = DraftEditPostUploadManager.registerErrorHandler(file.clientId!, setErrorLine); } - newUploadError(null); - }, [canUploadFiles, postFiles?.length, maxFileCount, newUploadError, intl, maxFileSize, serverUrl, post.channelId, post.rootId, updateFileInPostFiles]); + const currentMessageTooLong = postMessage.length > maxPostSize; + if (!currentMessageTooLong) { + setErrorLine(undefined); + } + }, [canUploadFiles, postFiles?.length, maxFileCount, setErrorLine, intl, maxFileSize, serverUrl, post.channelId, post.rootId, updateFileInPostFiles, postMessage, maxPostSize]); const handleFileRemoval = useCallback((id: string) => { @@ -321,20 +321,23 @@ const EditPost = ({ for (const file of loadingFiles) { if (file.clientId && !uploadErrorHandlers.current[file.clientId]) { - uploadErrorHandlers.current[file.clientId] = DraftEditPostUploadManager.registerErrorHandler(file.clientId, newUploadError); + uploadErrorHandlers.current[file.clientId] = DraftEditPostUploadManager.registerErrorHandler(file.clientId, setErrorLine); } } - }, [postFiles, postMessage, newUploadError, shouldEnableSaveButton, toggleSaveButton]); + }, [postFiles, postMessage, setErrorLine, shouldEnableSaveButton, toggleSaveButton]); const onChangeTextCommon = useCallback((message: string) => { - const tooLong = message.trim().length > maxPostSize; - setErrorLine(undefined); + const tooLong = message.length > maxPostSize; setErrorExtra(undefined); + if (tooLong) { const line = intl.formatMessage({id: 'mobile.message_length.message_split_left', defaultMessage: 'Message exceeds the character limit'}); - const extra = `${message.trim().length} / ${maxPostSize}`; + const extra = `${message.length} / ${maxPostSize}`; setErrorLine(line); setErrorExtra(extra); + } else { + // Clear any error when message is valid + setErrorLine(undefined); } }, [intl, maxPostSize]); @@ -461,7 +464,7 @@ const EditPost = ({ } diff --git a/app/screens/edit_post/edit_post_input/edit_post_input.test.tsx b/app/screens/edit_post/edit_post_input/edit_post_input.test.tsx index 80914b2c8..3a180356d 100644 --- a/app/screens/edit_post/edit_post_input/edit_post_input.test.tsx +++ b/app/screens/edit_post/edit_post_input/edit_post_input.test.tsx @@ -56,7 +56,6 @@ describe('EditPostInput', () => { onTextSelectionChange: jest.fn(), onChangeText: jest.fn(), addFiles: jest.fn(), - uploadFileError: undefined, }; beforeAll(async () => { diff --git a/app/screens/edit_post/edit_post_input/edit_post_input.tsx b/app/screens/edit_post/edit_post_input/edit_post_input.tsx index d830d098c..c719af976 100644 --- a/app/screens/edit_post/edit_post_input/edit_post_input.tsx +++ b/app/screens/edit_post/edit_post_input/edit_post_input.tsx @@ -47,7 +47,6 @@ type PostInputProps = { onChangeText: (text: string) => void; inputRef: React.MutableRefObject; addFiles: (file: FileInfo[]) => void; - uploadFileError: React.ReactNode; } const EditPostInput = ({ @@ -60,7 +59,6 @@ const EditPostInput = ({ version, inputRef, addFiles, - uploadFileError, }: PostInputProps) => { const intl = useIntl(); const theme = useTheme(); @@ -133,7 +131,7 @@ const EditPostInput = ({ channelId={post.channelId} currentUserId={post.userId} files={postFiles} - uploadFileError={uploadFileError} + uploadFileError={null} rootId={post.rootId} /> { + const hasError = Boolean(errorLine || errorExtra); + + if (!hasError) { + return null; + } + return ( - - {Boolean(errorLine) && ( - - )} - {Boolean(errorExtra) && ( - - )} - + <> + + {Boolean(errorLine) && ( + + )} + {Boolean(errorExtra) && ( + + )} + + + ); };