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

iPad - Chat - Chats are not accessible on iPad in Portrait mode (Pay on June 15th) #2447

Closed
isagoico opened this issue Apr 17, 2021 · 45 comments · Fixed by #2814
Closed

iPad - Chat - Chats are not accessible on iPad in Portrait mode (Pay on June 15th) #2447

isagoico opened this issue Apr 17, 2021 · 45 comments · Fixed by #2814
Assignees
Labels
Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Weekly KSv2

Comments

@isagoico
Copy link

isagoico commented Apr 17, 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:

Chat modal should open

Actual Result:

Nothing happens on tapping, chat blinks in list but doesn't open

Action Performed:

  1. Open app in Landscape
  2. Open any chat
  3. Rotate device to Portrait
  4. Try to open any chat

Workaround:

Unknown

Platform:

Where is this issue occurring?

Web
iOS iPad ✔️
Android
Desktop App
Mobile Web

Version Number: 1.0.24-0

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

Notes/Photos/Videos: Any additional supporting documentation

Found on iPad 6th iOS 14.3

Issue is not reproducible in production.

portrair.mp4

Expensify/Expensify Issue URL:

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

I'm not able to reproduce this. Are there more specific steps?
If not, I'm not sure we should make this a blocker.

@isagoico
Copy link
Author

I asked a tester to check. He was able to reproduce following these (will update the main comment steps to these ones):

  1. Open app in Landscape
  2. Open any chat
  3. Rotate device to Portrait
  4. Try to open any chat

Here's a vid

RPReplay_Final1618877156.MP4

@isagoico
Copy link
Author

He just confirmed the issue is reproducible in production so removing the blocker label

@marcaaron marcaaron removed the DeployBlockerCash This issue or pull request should block deployment label Apr 20, 2021
@jasperhuangg jasperhuangg self-assigned this Apr 26, 2021
@jasperhuangg jasperhuangg added Weekly KSv2 Improvement Item broken or needs improvement. Engineering labels Apr 26, 2021
@jasperhuangg
Copy link
Contributor

@isagoico I am unable to reproduce this issue on my iPad simulator, and since I don't have an actual iPad to test on I'm unsure as to how I can help. I'm going to unassign myself and throw this back into the pool. Sorry for the confusion!

2021-04-27_10-37-44.mp4

@jasperhuangg jasperhuangg removed their assignment Apr 27, 2021
@tugbadogan
Copy link
Contributor

I encountered this issue while testing another code. This is reproducible on older/smaller iPads and this issue is caused by openByDefault prop is set for Drawer.Navigator in MainDrawerNavigator component. When device orientation is changed while a chat is open, this prop is opening the drawer while the route is set for a chat.

I can work on a fix/proposal if you're planning to export this issue for external contributors.

@marcaaron
Copy link
Contributor

Sounds good @tugbadogan I think it can be External

@marcaaron marcaaron added the External Added to denote the issue can be worked on by a contributor label May 5, 2021
@MelvinBot
Copy link

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

@bfitzexpensify
Copy link
Contributor

Posted on Upwork.

@MelvinBot
Copy link

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

@bfitzexpensify bfitzexpensify removed the Daily KSv2 label May 5, 2021
@mallenexpensify
Copy link
Contributor

Interesting that the issue is specific to older iPads, I'm unable to reproduce on v 1.0.38 an iPad Pro (11-inch) on software version 14.4.2

@marcaaron
Copy link
Contributor

It might also only affect smaller sizes.

@tugbadogan
Copy link
Contributor

Yes, this is a problem for devices where isSmallScreenWidth is true on Portrait mode. I could reproduce it on iPad 5th Generation and iPhone 11.

Before the fix

Screen.Recording.2021-05-06.at.00.46.06.480p.mov

After the fix

Screen.Recording.2021-05-06.at.00.46.41.Trimmed.480p.mov

@tugbadogan
Copy link
Contributor

I submitted a proposal on Upwork as well.

@marcaaron
Copy link
Contributor

Interesting find @tugbadogan. To clarify, is the fix is to disable openByDefault? If so, I don't think it's acceptable as this is the behavior we want. But maybe we can use some other information to conditionally allow this prop to be set?

@tugbadogan
Copy link
Contributor

Hmm, that's true. We can use route prop that's passed by react navigation and disable openByDefault if route has a reportID.

@Jag96
Copy link
Contributor

Jag96 commented May 7, 2021

@tugbadogan what route prop values will you be using for this? I did a quick test and it looks like the values are pretty much the same on mobile when looking at the chatList and a specific chat, so I'd want to ensure that this proposal won't create a regression there.

iOS on chatList

joetest route: {"key":"Home-BT6nqocs5-Ma2ymwxFBv6","name":"Home","state":{"stale":false,"type":"drawer","key":"drawer-N6n6bcEY0W42ZJUYCXaip","index":0,"routeNames":["Report"],"history":[{"type":"route","name":"Report"},{"type":"route","name":"Report"},{"type":"route","name":"Report"},{"type":"drawer"}],"routes":[{"name":"Report","params":{"reportID":"66"},"key":"Report-POrWS0f4kvwe65FCWmDKG"}]}}

iOS on a specific chat

joetest route: {"key":"Home-BT6nqocs5-Ma2ymwxFBv6","name":"Home","state":{"stale":false,"type":"drawer","key":"drawer-N6n6bcEY0W42ZJUYCXaip","index":0,"routeNames":["Report"],"history":[{"type":"route","name":"Report"},{"type":"route","name":"Report"},{"type":"route","name":"Report"},{"type":"route","name":"Report"},{"type":"route","name":"Report"}],"routes":[{"name":"Report","params":{"reportID":"66"},"key":"Report-MMVQ2tUd9R8EXZ7kODxH8"}]}}

@Jag96 Jag96 reopened this May 12, 2021
@Jag96 Jag96 changed the title iPad - Chat - Chats are not accessible on iPad in Portrait mode [HOLD for payment on May 19th] iPad - Chat - Chats are not accessible on iPad in Portrait mode May 12, 2021
@isagoico
Copy link
Author

Issue reproducible during today's KI retests

@Jag96
Copy link
Contributor

Jag96 commented May 17, 2021

I can confirm this issue still occurs on the iPad Pro (9.7 inch). @tugbadogan can you have a look please?

@Jag96 Jag96 changed the title [HOLD for payment on May 19th] iPad - Chat - Chats are not accessible on iPad in Portrait mode iPad - Chat - Chats are not accessible on iPad in Portrait mode May 17, 2021
@tugbadogan
Copy link
Contributor

Hmm, let me check this. Are you using the same steps to reproduce the issue?

@Jag96
Copy link
Contributor

Jag96 commented May 17, 2021

Yes, using the same steps on the latest main (1.0.47-0)

@tugbadogan
Copy link
Contributor

Yeah, this fix didn't fix the actual issue in react-navigation library. It just fixed one of the reproduction paths. I'm trying to fix this issue by upgrading the version of react-navigation packages to be able to use Reanimated 2 to resolve the underlying issues.

@isagoico
Copy link
Author

Issue not reproducible during today's KI retests (First week)

@isagoico
Copy link
Author

Issue reproducible today during KI retests

@bfitzexpensify
Copy link
Contributor

Hey @tugbadogan, any updates on this one?

@tugbadogan
Copy link
Contributor

Yes @bfitzexpensify , I've updated the open PR today to fix this issue. I'm waiting for code review now.

#3078

@isagoico
Copy link
Author

isagoico commented Jun 7, 2021

Issue reproducible today during KI retests

@tugbadogan
Copy link
Contributor

This PR should solve this issue.
#3078

@bfitzexpensify
Copy link
Contributor

Thanks @tugbadogan! Once 7 days have passed after merging (which was 2 days ago) without any regressions, I'll finalise the contract and pay it out on Upwork.

@bfitzexpensify bfitzexpensify changed the title iPad - Chat - Chats are not accessible on iPad in Portrait mode iPad - Chat - Chats are not accessible on iPad in Portrait mode (Pay on June 15th) Jun 10, 2021
@tugbadogan
Copy link
Contributor

@bfitzexpensify I checked Upwork and it looks like the milestone is automatically closed by Upwork.

@bfitzexpensify
Copy link
Contributor

@tugbadogan — it does look that way. It looks like the payment was automatically sent — can you confirm receipt?

@tugbadogan
Copy link
Contributor

@bfitzexpensify Yes, I've received the payment automatically.

@mallenexpensify
Copy link
Contributor

I think this issue and #2180 might be the same. Checking with @tugbadogan in the other issue (so confusing, can't wait for both to be closed)

@isagoico
Copy link
Author

Issue reproducible today during KI retests

@tugbadogan
Copy link
Contributor

I'm not able reproduce this issue anymore on iPad Pro 9.7-inch iOS 12.0 with Expensify.cash v1.0.69-0 build

Which device/version are you using to reproduce this issue and can you record the screen with reproduction steps when you're able to reproduce?

@isagoico
Copy link
Author

@tugbadogan We were still able to reproduce this issue during KI retests.
Version: 1.0.72-0 (latest in staging)
In these 2 devices:

  • Apple iPad 6 - 9.7 (2018) - iOS 13.6
  • Apple iPad Pro 9.7-inch - iPadOS 14.6

@tugbadogan
Copy link
Contributor

@tugbadogan We were still able to reproduce this issue during KI retests.

Version: 1.0.72-0 (latest in staging)

In these 2 devices:

  • Apple iPad 6 - 9.7 (2018) - iOS 13.6

  • Apple iPad Pro 9.7-inch - iPadOS 14.6

I tried a couple of times, but could not reproduce this issue on a simulator. Are you using a physical device or a simulator? Do you follow the same reproduction steps? Can you record a video by any chance, so I can apply the exact same steps?

@isagoico
Copy link
Author

@tugbadogan This week the issue was not reproducible in the same devices (that were reproducible last week) 🎉

@isagoico
Copy link
Author

isagoico commented Jun 28, 2021

Issue not reproducible during KI retests. (First week)

@Jag96
Copy link
Contributor

Jag96 commented Jul 6, 2021

I'm unable to reproduce, since this wasn't reproducible by QA I'm going to close, feel free to reopen if this happens again!

@Jag96 Jag96 closed this as completed Jul 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Weekly KSv2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants