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

Fix keyboard bug when opening attachment modal #2460

Merged
merged 4 commits into from
Apr 20, 2021

Conversation

roryabraham
Copy link
Contributor

@roryabraham roryabraham commented Apr 19, 2021

Fixed Issues

Fixes #2424 (comment)

Tests / QA Steps (iOS/Android)

  1. Focus the main text input on a report.
  2. Click on the +, verify that the create menu opens.
  3. Close the modal, verify that the text input regains focus.
  4. Click on the +, and select Add Attachment
  5. Verify that the CreateMenu modal closes and is replaced by the AttachmentPickerMenu
  6. Close the modal, verify that the text input regains focus.

Tests / QA Steps (web/desktop/mWeb)

Just perform regression testing of the report action compose and attachment modals.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

n/a

Mobile Web

n/a

Desktop

n/a

iOS

keyboardFocusIOS.mov

Android

keyboardFocusAndroid.mov

@roryabraham roryabraham requested a review from a team as a code owner April 19, 2021 18:42
@roryabraham roryabraham self-assigned this Apr 19, 2021
@MelvinBot MelvinBot requested review from MariaHCD and removed request for a team April 19, 2021 18:42
@roryabraham roryabraham mentioned this pull request Apr 19, 2021
5 tasks
@roryabraham roryabraham changed the title Fix keyboard bug when opening attachment modal [HOLD] Fix keyboard bug when opening attachment modal Apr 19, 2021
@roryabraham
Copy link
Contributor Author

HOLD: not sure why yet but this seems to cause an app-crashing failure on android only.

@roryabraham roryabraham changed the title [HOLD] Fix keyboard bug when opening attachment modal Fix keyboard bug when opening attachment modal Apr 19, 2021
@roryabraham
Copy link
Contributor Author

Okay, fixed and retested on iOS and Android.

@roryabraham
Copy link
Contributor Author

Okay, just tested on web, desktop, and mWeb and I think we are good 👍

Copy link
Contributor

@MariaHCD MariaHCD left a comment

Choose a reason for hiding this comment

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

LGTM! Just one question.

src/pages/home/report/ReportActionCompose.js Show resolved Hide resolved
Copy link
Contributor

@Beamanator Beamanator left a comment

Choose a reason for hiding this comment

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

I like it 👍 and tests went quite well! LGTM

@Julesssss Julesssss merged commit 7960dd8 into main Apr 20, 2021
@Julesssss Julesssss deleted the Rory-FixKeyboardFocusModalOpen branch April 20, 2021 10:49
@OSBotify
Copy link
Contributor

🚀 Deployed to staging 🚀

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

@isagoico
Copy link

mWeb - Chat - Unable to add attachment on mweb Safari

Expected Result:

Able to add attachment on mweb Safari

Actual Result:

Unable to add attachment on mweb Safari

Action Performed:

  1. Go to https://staging.expensify.cash and log in
  2. Navigate to a conversation
  3. Click on the attachment button
  4. Click on add atachment.

Build version : 1.0.27-0

Notes/Images/Videos
Issue not reproducible in production.

Video

@isagoico isagoico added the DeployBlockerCash This issue or pull request should block deployment label Apr 20, 2021
@isagoico
Copy link

isagoico commented Apr 20, 2021

Chat - Focus not regained in text input field after closing AttachmentPicker

Expected Result:

verify that the text input regains focus.

Actual Result:

Cursor focus not regained in text input field

Action Performed:

  1. Focus the main text input on a report.
  2. Click on the +, and select Add Attachment
  3. Verify that the CreateMenu modal closes and is replaced by the AttachmentPickerMenu
  4. Close the modal, verify that the text input regains focus.

Build version : 1.0.27-0

Notes/Images/Videos
Issue not reproducible in production.

Bug5030758_Focus.mp4

@roryabraham
Copy link
Contributor Author

mWeb - Chat - Unable to add attachment on mweb Safari

Did this ever work? I'm just now realizing that there isn't really such a thing as a file picker on iOS like there is on macOS/chrome. I think mWeb needs an interface more like iOS has where one can choose between the camera, image picker, or document picker. In short I don't think this is a regression, but should be treated as a new feature with its own issue.

@roryabraham
Copy link
Contributor Author

Chat - Focus not regained in text input field after closing AttachmentPicker

This happened because the ReportActionCompose wasn't focused when the modals were opened. Just retested on iOS v1.0.27-3 and it works as expected. If the ReportActionCompose is focused when the modal is opened, it regains that focus when the modal is closed. If not, it does not gain focus when the modal is closed.

@roryabraham
Copy link
Contributor Author

mWeb - Chat - Unable to add attachment on mweb Safari

Okay, apparently this did work, so it is a real regression. I will continue to look into it

@roryabraham
Copy link
Contributor Author

roryabraham commented Apr 21, 2021

Got the fix for this regression here.

@roryabraham
Copy link
Contributor Author

@isagoico Please retest the deploy blocker from this comment during tomorrow's QA. Should be fixed on 1.0.30

@isagoico
Copy link

#2460 (comment) was not reproducible 🎉

#2460 (comment) this is still reproducible

@roryabraham roryabraham removed the DeployBlockerCash This issue or pull request should block deployment label Apr 26, 2021
@roryabraham
Copy link
Contributor Author

See this comment ... the second "deploy blocker" listed on this PR was a miscommunication of expectations. I can confirm that it's working as expected so I'm removing the deploy blocker label.

@OSBotify
Copy link
Contributor

OSBotify commented May 8, 2021

🚀 Deployed to production in version: 1.0.39-5🚀

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants