[$1000] iOS - Deepliink - Keyboard displayed upon splash screen if open new chat deeplink

[lanitochka17]

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

---

Issue found when executing PR #24126

Action Performed:

Precondition - Make sure that the app is completely closed (killed) and doesn't run in the background

1. Tap on https://staging.new.expensify.com/new/chat
   deeplink (e.g. in notes)

Expected Result:

"New chat" tab and keyboard appear after splash screen is dismissed

Actual Result:

Keyboard appears upon loading splash screen.
In Production the issue is reproducible when user interacts with staging deeplinks. If user taps on production deeplinks they will land in mWeb, not iOS app

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.53.1

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Notes/Photos/Videos: Any additional supporting documentation

Bug6161781_IMG_8918.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0100e6a84621057a16
• Upwork Job ID: 1691899029638578176
• Last Price Increase: 2023-10-18
• Automatic offers:
• jjcoffee | Reviewer | 27364778
• tienifr | Contributor | 27364780

[melvin-bot]

Triggered auto assignment to @isabelastisser (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[rayane-djouah]

Proposal

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

When a user taps on a deep link to open the "New Chat" page in the app, the keyboard appears upon loading the splash screen. The expected behavior is for the "New Chat" tab and keyboard to appear only after the splash screen is dismissed.

What is the root cause of that problem?

The shouldFocusOnSelectRow prop of the OptionsSelector component within the NewChatPage is set to true immediately upon rendering, triggering the keyboard to display.

App/src/pages/NewChatPage.js

Line 201 in a03a21d

shouldFocusOnSelectRow={props.isGroupChat && !Browser.isMobile()}

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

Use React's Context API to modify the shouldFocusOnSelectRow prop based on the isSplashHidden state:

Create a New Context:

import { createContext } from 'react';

const AppContext = createContext();

export default AppContext;

Provide the Context in Expensify.js:

import AppContext from './path_to_AppContext';

// ... other code ...

return (
    <AppContext.Provider value={{ isSplashHidden }}>
        {/* Your existing components, including navigators */}
    </AppContext.Provider>
);

Consume the Context in NewChatPage.js:

import React, { useContext } from 'react';
import AppContext from './path_to_AppContext';

function NewChatPage(props) {
    const { isSplashHidden } = useContext(AppContext);

    // ... rest of your component logic ...
}

Modify the shouldFocusOnSelectRow Prop:

Update the prop in the OptionsSelector component:

<OptionsSelector
    ...
    shouldFocusOnSelectRow={props.isGroupChat && !Browser.isMobile() && isSplashHidden}
    ...
/>

What alternative solutions did you explore? (Optional)

[isabelastisser]

I was assigned this when I was OOO. I will review this soon!

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0100e6a84621057a16

[melvin-bot]

Current assignee @isabelastisser is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[tienifr]

Proposal

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

Keyboard appears upon loading splash screen.

What is the root cause of that problem?

The issue happens for any screen that uses OptionsSelector without shouldDelayFocus. That's because we're focusing on the text input immediately via autofocusing here after the NewChatPage component is mounted.

At that time the Splash screen did not hide yet, causing the issue.

It doesn't happen for other screens like workspace members invite page because in those pages, we use SelectionList with shouldDelayFocus, so the input is only focused after a timeout, which is usually longer than what it takes for the splash screen to hide.

This is only happening for iOS, because for Android, the keyboard will not be displayed until the text input becomes visible (at that time the splash screen already hides), this is native platform behavior.

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

A potential solution is to use shouldDelayFocus on those OptionsSelector, so it's similar the pages that use SelectionList mentioned above and won't have the issue.

But I prefer the below since it doesn't rely on timeouts:

We need to standardise on the autofocusing of the TextInput so that:

• It autofocuses using the existing useAutoFocusInput hook
• It only autofocus after the splash screen hides

1. We should expose the ref of the input of OptionsSelector so that we can access the ref inside the pages that use OptionsSelector (for example the NewChatPage)
2. Provide the isSplashHidden via a context, so we can use that in components in the app
3. We can extend the useAutoFocusInput hook so that it will call inputRef.current.focus(); only if the isSplashHidden is true as well. (The isSplashHidden is retrieved via context)
4. Then use that hook for the NewChatPage and any other pages that have this issue

What alternative solutions did you explore? (Optional)

Another way is to get the isSplashHidden via context to inside the BaseOptionsSelector here and if it's false, do not auto focus. In componentDidUpdate of BaseOptionsSelector, if isSplashHidden becomes true, we'll focus there. The same thing can be done with the autofocusing behavior of TextInput

[jjcoffee]

@rayane-djouah I don't think your RCA is correct, if I set shouldFocusOnSelectRow to false, the text input is still focused (as you'd expect).

@tienifr Leaning towards your proposal, but I'd just like to understand the benefit of your main solution over the alternative, which sounds simpler to me at first glance! What is the benefit of using the useFocusOnTransitionEnd hook, over simply checking for isSplashHidden in componentDidUpdate?

Can either of you still replicate the issue, as these related issues are being reported as no longer occurring on staging or prod.

[tienifr]

@jjcoffee if we put isSplashHidden inside the components like OptionSelector and TextInput, we don't have very good separation of concern since now the generic OptionSelector and TextInput have the knowledge that we have a splash screen 😄. Also across the app we seems to converge to using the onEntryTransitionEnd event to manage the auto focus so my main solution will work for those places out of the box.

Can either of you still replicate the issue, as these related issues are being #22508 (comment) as no longer occurring on staging or prod.

I can still replicate it.

[jjcoffee]

@tienifr Thanks, that makes sense! I will come back for a full review on Monday.

[jjcoffee]

@tienifr Weird, I can't replicate on an iOS simulator (on latest main). Were you replicating on a physical device? Looks like there have been several changes to BaseOptionsSelector so that it no longer directly focuses in componentDidMount so maybe this is no longer an issue.

@lanitochka17 are you able to retest on latest main?

[tienifr]

Weird, I can't replicate on an iOS simulator (on latest main). Were you replicating on a physical device?

@jjcoffee No, I use Simulator (iPhone SE, iOS 16.2), still reproducible on latest main. You probably did but do make sure to turn on software keyboard 😄.

Simulator.Screen.Recording.-.iPhone.SE.3rd.generation.-.2023-08-22.at.17.46.00.mp4

[jjcoffee]

@tienifr Haha yes I did have the keyboard turned on!

I'm no longer sure if this is really a bug if it now always works the same as in your repro video. The keyboard is showing just as the splash screen is transitioning, rather than way before like in the video in the issue description. I feel like that's acceptable behaviour. Asking for feedback on Slack.

Separately, your RCA doesn't cover why the issue would be happening on iOS but not on Android. Do you have any thoughts on that?

[isabelastisser]

@jjcoffee I bumped your question in Slack.

[melvin-bot]

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

[jjcoffee]

@isabelastisser We got two votes for not a bug, not sure if that's enough to close? 😄

[lanitochka17]

Issue reproducible on Build 1.3.56-18
Iphone 14 Pro Max/ios16.6

0-02-01-2f1b86409e6fa4ea5ea75a4f312dffb317c736c5f6806629fa523e1ce06d80be_32da22cc25454b1b.mp4

[isabelastisser]

Closing this per slack discussion.

[melvin-bot]

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

[isabelastisser]

Bump @jjcoffee can you please review the updated proposal above? Thanks!

[jjcoffee]

Sorry higher priority PR got in the way, will review tomorrow!

[melvin-bot]

@jjcoffee, @isabelastisser Whoops! This issue is 2 days overdue. Let's get this updated quick!

[jjcoffee]

@tienifr I believe we may have settled on a standard pattern for this here. Can you comment on whether or not you think that's applicable for this case?

[tienifr]

@jjcoffee yes that pattern is applicable and my proposal already follows that pattern (The pattern was implemented in the useAutoFocusInput already)

[jjcoffee]

Great, thanks for clarifying!

I'm happy to go with @tienifr's proposal in that case!

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @jjcoffee 🎉 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

[melvin-bot]

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

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[jjcoffee]

Just a quick update - PR has been approved by me, so just waiting for @danieldoglas to approve.

[isabelastisser]

Bump @danieldoglas for the comment above. Thanks!

[jjcoffee]

@isabelastisser PR has been merged!

[danieldoglas]

interesting, this was deployed to production but it didn't post the summary yet.

It was deployed on the 9th, so this should be good for payment next tomorrow @isabelastisser

[jjcoffee]

• The PR that introduced the bug has been identified. Link to the PR: N/A - behaviour was always there
• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A
• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
• Determine if we should create a regression test for this bug. Yes
• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

1. Make sure that the app is completely closed (killed) and doesn't run in the background
2. Tap on a deeplink to https://staging.new.expensify.com/new/chat
3. Verify that the keyboard only opens after the splash screen is hidden

Do we agree 👍 or 👎

[jjcoffee]

@isabelastisser Friendly bump for payment, checklist is completed above!

[isabelastisser]

I'm sorry for the payment delay @jjcoffee and @tienifr. I've processed the payments in Upwork now.

@danieldoglas, can you please review the Regression Test Proposal above and let me know if one should be created? Thanks!

[danieldoglas]

@isabelastisser I think it should be created