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

[$250] The mobile attachment picker on Android web is giving option to record audio and video #46336

Closed
1 of 6 tasks
m-natarajan opened this issue Jul 26, 2024 · 34 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@m-natarajan
Copy link

m-natarajan commented Jul 26, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 9.0.13-3
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @quinthar
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1721965815025499

Action Performed:

  1. Go to staging.new.expensify.com
  2. Tap Global +
  3. Tap Track > Scan > Attachment picker

Expected Result:

  1. On Android native, we don't offer "Take photo" when opening the attachment menu from the camera
  2. On Android web, let's really try to just follow the exact same flow as Android native
  3. On Android web, let's not ask to take a video or record audio

Actual Result:

Not matching with Android native and giving option to record audio and video for attachment picker

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~014aa6bba3d11a7600
  • Upwork Job ID: 1818599086364884619
  • Last Price Increase: 2024-08-07
  • Automatic offers:
    • eh2077 | Reviewer | 103436535
Issue OwnerCurrent Issue Owner: @eh2077
@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 26, 2024
Copy link

melvin-bot bot commented Jul 26, 2024

Triggered auto assignment to @miljakljajic (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@melvin-bot melvin-bot bot added the Overdue label Jul 29, 2024
Copy link

melvin-bot bot commented Jul 30, 2024

@miljakljajic Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@miljakljajic miljakljajic added the External Added to denote the issue can be worked on by a contributor label Jul 31, 2024
@melvin-bot melvin-bot bot changed the title The mobile attachment picker on Android web is giving option to record audio and video [$250] The mobile attachment picker on Android web is giving option to record audio and video Jul 31, 2024
Copy link

melvin-bot bot commented Jul 31, 2024

Job added to Upwork: https://www.upwork.com/jobs/~014aa6bba3d11a7600

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 31, 2024
Copy link

melvin-bot bot commented Jul 31, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @eh2077 (External)

@melvin-bot melvin-bot bot removed the Overdue label Jul 31, 2024
@daledah
Copy link
Contributor

daledah commented Jul 31, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

Not matching with Android native and giving option to record audio and video for attachment picker.

What is the root cause of that problem?

When using AttachmentPicker for scanning here

, we don't limit the file types, so by default it will accept any file types
accept={getAcceptableFileTypes(type)}

What changes do you think we should make in order to solve the problem?

Allow the users of AttachmentPicker to customize the file types that the input will accept, that means the AttachmentPicker will have a new params acceptedFileTypes, which it will pass to input. If acceptedFileTypes exists it will be used, otherwise it will fallback to the existing logic

accept={getAcceptableFileTypes(type)}

Then we can pass acceptedFileTypes to AttachmentPicker here

. The list of file types supported is here
ALLOWED_RECEIPT_EXTENSIONS: ['jpg', 'jpeg', 'gif', 'png', 'pdf', 'htm', 'html', 'text', 'rtf', 'doc', 'tif', 'tiff', 'msword', 'zip', 'xml', 'message'],

We need to construct a string of accepted file types that is compatible with accept prop of the input. Reference: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/file#unique_file_type_specifiers

Or a slightly better code design is we pass this list

ALLOWED_RECEIPT_EXTENSIONS: ['jpg', 'jpeg', 'gif', 'png', 'pdf', 'htm', 'html', 'text', 'rtf', 'doc', 'tif', 'tiff', 'msword', 'zip', 'xml', 'message'],
as is, as acceptedFileTypes, then we have a transformer in AttachmentPicker that will convert it to accept-compatible string

What alternative solutions did you explore? (Optional)

NA

@daledah
Copy link
Contributor

daledah commented Jul 31, 2024

Proposal has a minor update for clarification

@eh2077
Copy link
Contributor

eh2077 commented Jul 31, 2024

@daledah Thanks for your proposal!

Does the inconsistency only happen with mobile Chrome? Or both mobile web platforms have same issue?

As your solution suggests to pass accepted files types in App/src/components/AttachmentPicker/index.tsx, I'm not sure if it would break some feature on web platforms.

@daledah
Copy link
Contributor

daledah commented Aug 2, 2024

Does the inconsistency only happen with mobile Chrome? Or both mobile web platforms have same issue?

@eh2077 Both mobile platforms have the issue, Safari is also allowing users to record/select videos.

As your solution suggests to pass accepted files types in App/src/components/AttachmentPicker/index.tsx, I'm not sure if it would break some feature on web platforms.

@eh2077 It won't break any feature because we only use it when selecting files for Scan, and accept is a standard web feature for input, the OS will allow selecting according to the file formats specified by accept. From my testing everything looks fine.

@melvin-bot melvin-bot bot added the Overdue label Aug 2, 2024
@eh2077
Copy link
Contributor

eh2077 commented Aug 4, 2024

@daledah 's proposal looks good to me. Coding details can be discussed in PR and we need to ensure mobile platforms have consistent behaviour.

🎀👀🎀 C+ reviewed

@melvin-bot melvin-bot bot removed the Overdue label Aug 4, 2024
Copy link

melvin-bot bot commented Aug 4, 2024

Triggered auto assignment to @danieldoglas, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

Copy link

melvin-bot bot commented Aug 7, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Aug 7, 2024
@eh2077
Copy link
Contributor

eh2077 commented Aug 7, 2024

Not overdue, we're waiting for @danieldoglas 's review

@melvin-bot melvin-bot bot removed the Overdue label Aug 7, 2024
@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 7, 2024
Copy link

melvin-bot bot commented Aug 7, 2024

📣 @eh2077 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

Copy link

melvin-bot bot commented Aug 7, 2024

📣 @daledah You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Aug 8, 2024
@daledah
Copy link
Contributor

daledah commented Aug 8, 2024

@eh2077 This PR is ready for review.

@miljakljajic
Copy link
Contributor

Deployed to prod two days ago. Will update the payment schedule header.

@miljakljajic miljakljajic changed the title [$250] The mobile attachment picker on Android web is giving option to record audio and video Ready for payment on 26/08/24 [$250] The mobile attachment picker on Android web is giving option to record audio and video Aug 21, 2024
@eh2077
Copy link
Contributor

eh2077 commented Aug 21, 2024

@daledah Can you take a look at this comment #47028 (comment) please?

@miljakljajic
Copy link
Contributor

@daledah please apply to this job too and we'll get you paid

@eh2077
Copy link
Contributor

eh2077 commented Aug 27, 2024

@daledah Can you take a look at this comment #47028 (comment) please?

@daledah Friendly bump

@daledah
Copy link
Contributor

daledah commented Aug 27, 2024

@eh2077 I'm working on this PR

@miljakljajic miljakljajic changed the title Ready for payment on 26/08/24 [$250] The mobile attachment picker on Android web is giving option to record audio and video [$250] The mobile attachment picker on Android web is giving option to record audio and video Aug 27, 2024
@miljakljajic
Copy link
Contributor

Seems like this one isn't ready to be paid yet

@daledah
Copy link
Contributor

daledah commented Aug 27, 2024

@daledah please apply to this job too and we'll get you paid

@miljakljajic I don't have any Upwork Connect so I could not apply. Could you send the offer directly to my profile here https://www.upwork.com/freelancers/~0138d999529f34d33f? Thx

@miljakljajic
Copy link
Contributor

Offer for @daledah is here: https://www.upwork.com/nx/wm/offer/103795902

@eh2077
Copy link
Contributor

eh2077 commented Sep 3, 2024

Hi @miljakljajic, sorry this issue is not ready yet for payment. Because we haven't fully get the expected behaviors. We have active discussions on the closed PR #47028

@miljakljajic
Copy link
Contributor

Don't worry, I have just extended the offer but I will not pay til payment is due. Thank you @eh2077

@eh2077
Copy link
Contributor

eh2077 commented Sep 16, 2024

I'll post a summary of our discussions from the closed PR #47028 soon

@miljakljajic
Copy link
Contributor

Did we decide to close without moving forward, or will payment still be due?

@eh2077
Copy link
Contributor

eh2077 commented Sep 20, 2024

Did we decide to close without moving forward, or will payment still be due?

@miljakljajic Thanks for checking this. Sorry for the late update, I think it's better to hold it for a while to hear feedback from internal engineering team.

@eh2077
Copy link
Contributor

eh2077 commented Sep 20, 2024

Here's the summary of the status on this issue

  1. We have a PR fix: Android web is giving option to record audio and video #47028 merged but only managed to remove the record audio option
  2. On mobile Chrome, we still have the Camera Camcorder option - the record video option
The screenshot of what's look like after the PR merged

@daledah and I spent sometime to find a solution to remove the Camera Camcorder option but we ended up a conclusion that, on mobile Chrome, we can't control options to show on the attachment picker modal. Instead, it's the default behaviour of mobile Browser. There's a demonstration video #47028 (comment) by @daledah

See also

  1. Discussions from the merged PR fix: Android web is giving option to record audio and video #47028 (comment)
  2. https://stackoverflow.com/questions/48503345/how-to-disable-camera-option-for-file-upload-from-mobile-browser
  3. https://stackoverflow.com/questions/21523544/html-file-input-control-with-capture-and-accept-attributes-works-wrong

That said, I tend to leave the record video option, Camera Camcorder, as it is.

@danieldoglas What do you think?

cc @quinthar @daledah

@danieldoglas
Copy link
Contributor

I see. So, that's more of a limitation on Android, not on our side. If that's not something we can control, I think it's fine to keep it as is. @miljakljajic are you OK with that too?

@miljakljajic
Copy link
Contributor

I agree - it doesn't look like a bug. Closing. Thank you for investigating!

@eh2077
Copy link
Contributor

eh2077 commented Sep 20, 2024

@miljakljajic This should be eligible for payment right? We have worked on a PR #47028 here.

@danieldoglas danieldoglas reopened this Sep 20, 2024
@danieldoglas
Copy link
Contributor

Yep, this is eligible for payment - we indeed solved the situation with the audio recording.

@miljakljajic
Copy link
Contributor

Sorry, paid!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

5 participants