Conversation
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-06-26.at.6.40.52.in.the.evening.movAndroid: mWeb ChromeScreen.Recording.2025-06-26.at.6.48.51.in.the.evening.moviOS: HybridAppScreen.Recording.2025-06-26.at.9.03.05.at.night.moviOS: mWeb Safariscreen-recording-2025-06-26-at-64950-in-the-evening_zUYlFvlR.mp4MacOS: Chrome / SafariScreen.Recording.2025-06-25.at.4.06.20.in.the.afternoon.movMacOS: DesktopScreen.Recording.2025-06-26.at.6.55.54.in.the.evening.online-video-cutter.com.mp4 |
| [], | ||
| ); | ||
|
|
||
| const isDirectoryCheck = useCallback((data: FileObject) => { |
There was a problem hiding this comment.
Out of scope but either the name is misleading or is not working as expected.
Doesn't prevent folder upload.
There was a problem hiding this comment.
yeah, I also noticed that - but as you mentioned - out of scope
src/languages/en.ts
Outdated
| tooManyFiles: ({fileLimit}: FileLimitParams) => `You can only upload up to ${fileLimit} files at a time.`, | ||
| sizeExceededWithValue: ({maxUploadSizeInMB}: SizeExceededParams) => `Files exceeds ${maxUploadSizeInMB} MB. Please try again.`, | ||
| someFilesCantBeUploaded: "Some files can't be uploaded", | ||
| sizeLimitExceeded: "Files must be under 10 MB. Any larger files won't be uploaded.", |
There was a problem hiding this comment.
it's a limit for the receipts, it will be used later
| maxFileLimitExceeded: "You can upload up to 30 receipts at a time. Any extras won't be uploaded.", | ||
| unsupportedFileType: ({fileType}: FileTypeParams) => `${fileType} files aren't supported. Only supported file types will be uploaded.`, | ||
| learnMoreAboutSupportedFiles: 'Learn more about supported formats.', | ||
| passwordProtected: "Password-protected PDFs aren't supported. Only supported files will be uploaded.", |
There was a problem hiding this comment.
Looks like we don't use this. plus password-protected PDFs are supported.
There was a problem hiding this comment.
We will use it for the receipts
| confirmText={translate(validFilesToUpload.length ? 'common.continue' : 'common.close')} | ||
| shouldShowCancelButton={!!validFilesToUpload.length} | ||
| cancelText={translate('common.cancel')} | ||
| onModalHide={() => { |
There was a problem hiding this comment.
Lets reset fileError state here, since if we rely on useEffect to show the modal.
There was a problem hiding this comment.
I will check it - but probably I made it this way, because when you clear the file error too early, the text will disappear before the modal closes (because modal closing is animated). Same reason the text appears a bit longer and the cancel button disappears earlier here #64323 (comment)
src/components/AttachmentModal.tsx
Outdated
| setFile(originalFileName ? {name: originalFileName} : undefined); | ||
| }, [originalFileName]); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
Why are we using useEffect? Can't we just set it where we set fileError?
There was a problem hiding this comment.
@getusha - same as above #64323 (comment)
|
Bug: A file size limit error briefly appears after pressing "Continue."
Screen.Recording.2025-06-23.at.10.58.04.at.night.mov |
@getusha You mean when all the files dropped are larger than 10MB? (should be 24MB for attachments, I know) I think this copy is descriptive enough for this case, we don't really care how much of the files are exceeding the limit, but I wonder what @Expensify/design Team think? |
|
Personally I think it's probably fine as-is. I think this copy already went through our design doc process, right? |
yep, the copy is from the design doc |
|
@getusha I've made fix for ios, can you check it? |
|
I am still able to reproduce @koko57 Screen.Recording.2025-06-27.at.11.24.18.at.night.movLMK if you need the assets |
|
@getusha yes, please, can you send me the assets? |
|
@getusha I've changed the limit of max size to make it easier to throw an error - I cannot reproduce Simulator.Screen.Recording.-.iPhone.15.-.2025-06-30.at.10.35.07.mp4 |
|
Can't upload multiple file attachments after a multi file error
Screen.Recording.2025-07-01.at.10.45.29.in.the.morning.movDoesn't have to be a blocker |
|
For the iOS issue there is a noticeable delay that could be relevant Screen.Recording.2025-07-01.at.11.40.42.in.the.morning.movWill again test it after the refactor |
|
yeah, for the ios, I think we should add a loader (like it's for selecting receipts in the IOU flow), it could be done later, now I will fix the issue with the modal on the web |
|
@getusha can you check once again #64323 (comment) and send me the attachments if it's still reproducible? |
|
@getusha also please retest this one: #64323 (comment) (fixed) |
Couldn't reproduce but i think it's a potential race condition which boils down to the delay Screen.Recording.2025-07-01.at.1.03.53.in.the.afternoon.movYou can replicate the freeze by trying to open the create popover before the error modal appears |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/lakchote in version: 9.1.74-2 🚀
|
|
We have reverted to fix this blocker #65317, please add those QA steps to the next attempt 🙌 Thank you! |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.1.74-10 🚀
|




Explanation of Change
🚨 IMPORTANT NOTE 🚨
This PR introduces the ability to send multiple file attachments in a single message.
Key Changes
Fixed Issues
$ #59443
PROPOSAL:
Tests
PREREQUISITES: all betas / newDotMultiFilesDragAndDrop beta enabled
[WEB / DESKTOP]
[MOBILE]
[CHECK THE ERROR HANDLING]
[CHECK THE SINGLE FILE FLOW]
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
Uploading Simulator Screen Recording - iPhone 15 Pro - 2025-06-17 at 15.04.24.mp4…
iOS: mWeb Safari
Uploading Simulator Screen Recording - iPhone 15 Pro - 2025-06-17 at 14.58.05.mp4…
MacOS: Chrome / Safari
Screen.Recording.2025-06-17.at.17.26.33.mp4
Screen.Recording.2025-06-17.at.17.25.18.mp4
MacOS: Desktop
Screen.Recording.2025-06-17.at.17.32.54.mp4
Screen.Recording.2025-06-17.at.17.32.16.mp4