-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Add document picking support to mobile #689
Conversation
CC @Jag96. My thinking is that the previous ImagePicker should be instead called AttachmentPicker -- as it's responsible for more than just images. After that, it made sense to move both the Image and Document picking logic into the AttachmentPicker. Would be great to get your thoughts at this stage and to confirm the overall structure makes sense. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of moving the Document picker inside the newly renamed AttachmentPicker. Left a couple of comments but looks good so far.
2df85b5
to
8c2b9bd
Compare
8c2b9bd
to
686029c
Compare
Document upload is working, but I'm unsure about a certain behaviour: When uploading a csv file from web, it is a downloadable file, but when I re attach it from mobile it is parsed and displayed as an image. I imagine this was desired for parsing expense reports, but should chat behave differently? |
I would think it should upload as a file. It would be awesome if we had file previews at some point, but that's a different feature. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, just a couple of questions/recommendations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just a nab
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, good stuff!
CC @Jag96
Add ability to add document attachments to chats on mobile. This already exists for web and desktop, but mobile users were only able to upload images.
Fixed Issues
Fixes https://github.com/Expensify/Expensify/issues/142733
Tests
1) Take Photo
2) Upload photo from gallery
3)
4)
5)
6)
Screenshots
Android