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.

Sign up for GitHub

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

Jump to bottom

Fix: Attachment Picker briefly displayed after taking a picture #2656

Merged
merged 3 commits into from
May 13, 2021
Merged

Fix: Attachment Picker briefly displayed after taking a picture #2656

merged 3 commits into from
May 13, 2021

Conversation

kidroca
Copy link
Contributor

@kidroca kidroca commented Apr 30, 2021 •
edited
Loading

@Julesssss

Details

When an attachment option is selected, first close the modal and then open camera/gallery/documents

Fixed Issues

Fixes #2580

Tests

  1. Open a conversation
  2. Select the (+) Button in the compose box
  3. 1st popup -> Select "Add Attachment"
  4. 2nd popup -> Select one of the available options to pick an attachment
  5. Gallery | Camera | Document picker opens
  6. Pick an attachment

QA Steps

Same as above

Tested On

The changes made are only to the index.native.js file the rest of the Platforms are unaffected and their code was not modified

  • iOS
  • Android
  • Web
  • mWeb
  • Desktop

Screenshots

iOS

Screen.Recording.2021-04-30.at.21.49.04.mov

Android

Android.Emulator.-.Pixel_2_API_29_5554.2021-04-30.20-10-21.mp4

Web

image

mWeb

image

Desktop

image

@kidroca kidroca requested a review from a team as a code owner April 30, 2021 18:50
@MelvinBot MelvinBot requested review from joelbettner and removed request for a team April 30, 2021 18:50
@Julesssss
Copy link
Contributor

Bumping @joelbettner for a review.

I'm unable to take on additional code reviews this week, but will try to take a look if things clear up towards the end of the week. Thanks for tagging me anyway 👍

@kidroca kidroca requested a review from joelbettner May 5, 2021 20:05
@kidroca kidroca mentioned this pull request May 6, 2021
Closed
@kidroca
Copy link
Contributor Author

kidroca commented May 6, 2021

Updated

@kidroca kidroca requested a review from tgolen May 7, 2021 11:21
tgolen
tgolen previously approved these changes May 7, 2021
View reviewed changes
@kidroca
Copy link
Contributor Author

kidroca commented May 10, 2021

Bumping @joelbettner for a review.
I've left the last thread unresolved so you can check my reply

joelbettner
joelbettner previously approved these changes May 10, 2021
View reviewed changes
Copy link
Contributor

@joelbettner joelbettner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Unfortunately there are conflicts now. Probably just need to merge master.

@kidroca
Merge remote-tracking branch 'e.cash/main' into kidroca/attachment-pi…
94ddc07 
…cker-briefly-displayed-after-taking-a-picture

# Conflicts:
#	src/components/AttachmentPicker/index.native.js
@kidroca kidroca dismissed stale reviews from joelbettner and tgolen via 94ddc07 May 12, 2021 07:54
@kidroca
Copy link
Contributor Author

kidroca commented May 12, 2021

@tgolen @joelbettner
Synced with master and resolved merge conflicts in AttachmentPicker/index.native.js

@tgolen
Copy link
Contributor

tgolen commented May 13, 2021 •
edited
Loading

Code changes approved, but I need to go through and test this again. @kidroca I know that you're only changing a native platform file, but can you please also test all other platforms and add screenshots for them? That needs to be done on every PR (we've been experiencing quality issues and need to step up the PR testing).

@tgolen
Copy link
Contributor

tgolen commented May 13, 2021

All my tests pass 👍

@kidroca
Copy link
Contributor Author

kidroca commented May 13, 2021

Updated with screenshots from the other platforms

@tgolen
Copy link
Contributor

tgolen commented May 13, 2021

Thanks!

@tgolen tgolen merged commit 89667d7 into Expensify:main May 13, 2021
@OSBotify
Copy link
Contributor

🚀 Deployed to staging in version: 1.0.43-9🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to production in version: 1.0.44-0🚀

platform result
🤖 android 🤖 failure ❌
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joelbettner joelbettner joelbettner approved these changes

@tgolen tgolen tgolen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Chat - Attachment picker is briefly displayed after taking picture to upload in chat
5 participants
@kidroca @Julesssss @tgolen @OSBotify @joelbettner