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

Chat - Attachment picker is briefly displayed after taking picture to upload in chat #2580

Closed
isagoico opened this issue Apr 26, 2021 · 13 comments · Fixed by #2656
Closed

Chat - Attachment picker is briefly displayed after taking picture to upload in chat #2580

isagoico opened this issue Apr 26, 2021 · 13 comments · Fixed by #2656
Assignees
Labels
External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@isagoico
Copy link

isagoico commented Apr 26, 2021

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Expected Result:

Upload Attachment screen displayed

Actual Result:

Attachment picker screen displayed for a moment before Upload attachment screen

Action Performed:

  1. Launch the app and login
  2. Initiate a chat > Tap +
  3. Tap Add Attachment > Take photo
  4. Take a photo and tap use photo

Workaround:

No need, visual issue.

Platform:

Where is this issue occurring?

Web
iOS ✔️
Android ✔️
Desktop App
Mobile Web

Version Number: 1.0.31-0

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Video

Expensify/Expensify Issue URL: https://github.com/Expensify/Expensify/issues/161920

View all open jobs on Upwork

@isagoico isagoico added the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Apr 26, 2021
@MelvinBot
Copy link

Triggered auto assignment to @sonialiap (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@MelvinBot MelvinBot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Apr 26, 2021
@roryabraham
Copy link
Contributor

I believe this might be a duplicate. @kidroca I think you were commenting about this here, right? Do you know if this is a duplicate issue / if this is already being worked on?

@kidroca
Copy link
Contributor

kidroca commented Apr 27, 2021

@roryabraham
No, this is not a duplicate

At the time of the PR, when a modal was closing the compose input would be auto focused which would interfere with the gallery/camera and dismiss it. The only way to prevent this was to keep the popup modal open

https://github.com/Expensify/Expensify.cash/blob/99b44b3dcbc8efe99e7ea7a0fe833382f025a0aa/src/components/AttachmentPicker/index.native.js#L220-L225

Now since compose autofocus no longer happen automatically on mobile devices this can be updated so that the 2nd modal, the attachment picker, closes while you take a picture or select something from gallery/documents

I can apply a proposal if you're interested

@roryabraham
Copy link
Contributor

@kidroca, thanks for the explanation! We don't have an upwork job for this yet, but when we have it posted I'll ping you and make sure you're first in line 🙂

@roryabraham roryabraham added the External Added to denote the issue can be worked on by a contributor label Apr 27, 2021
@MelvinBot
Copy link

Triggered auto assignment to @stephanieelliott (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@MelvinBot
Copy link

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

@stephanieelliott
Copy link
Contributor

Job post on Upwork: https://www.upwork.com/jobs/~01e62dbc72de7f7a3e

☝️ @kidroca

@stephanieelliott stephanieelliott removed their assignment Apr 28, 2021
@kidroca
Copy link
Contributor

kidroca commented Apr 28, 2021

Proposal

The logic here:

https://github.com/Expensify/Expensify.cash/blob/5ed9e261867aec89018d295fe67775b9d7d621b6/src/components/AttachmentPicker/index.native.js#L229-L237

Can be altered to something like:

<MenuItem
  key={text}
  icon={icon}
  title={text}
  onPress={() => {
    // first tell the popover to start closing
    this.close();

    // invoke the attachment picking action
    pickAttachment()
      .then(this.setResult)
      .catch(console.error)
      .finally(this.completeAttachmentSelection)
  }}
/>

completeAttachmentSelection would trigger the onPicked callback with the selected photo/document or do nothing if nothing was selected (cancelled from the gallery)

Right now completeAttachmentSelection would be invoked only after the modal hides resulting in the current issue

Alternatively instead of calling pickAttachment immediately after close it can be set to be called after the modal hides. E.g. if it's causing any transition issues.

@deetergp
Copy link
Contributor

@kidroca Your proposal sounds good, provided it doesn't somehow break the expected behavior of the app on Android. Go ahead and create a PR 👍

@kidroca
Copy link
Contributor

kidroca commented Apr 29, 2021

This happens on Android as well. Both will be addressed by the proposed changes

Android.Emulator.-.Pixel_2_API_29_5554.2021-04-29.15-05-20.mp4

@isagoico You might want to update the issue description to list Android as affected too

@kidroca
Copy link
Contributor

kidroca commented Apr 29, 2021

@deetergp
I have submitted a proposal on Upwork: https://www.upwork.com/ab/proposals/1387741889178193921

@stephanieelliott
Copy link
Contributor

Thanks @kidroca, I sent you the offer on Upwork!

@isagoico
Copy link
Author

isagoico commented May 9, 2021

Issue reproducible during today's KI retests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants