[HOLD for payment 2023-02-28] [$1000] Screen is flashing when changing focus between edit input

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Login with any account
2. Try to select any report, send few messages, try to edit them but doesn’t press Save. Try to change focus
3. Notice that screen is flashing when switching focus between edit field.

Expected Result:

Screen is not flashing when switching focus between edit fields.

Actual Result:

Screen is flashing when switching focus between edit field.

Workaround:

unknown

Platforms:

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

• iOS / native

Version Number: 1.2.70-1
Reproducible in staging?: y
Reproducible in production?: y
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:

RPReplay_Final1676115090.MP4

BVYQ7397.1.MP4

Expensify/Expensify Issue URL:
Issue reported by: @hungvu193
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1676115306084349

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01f0ed90a9d3684e1e
• Upwork Job ID: 1625310511301693440
• Last Price Increase: 2023-02-14

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

Triggered auto assignment to @thienlnam (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~01f0ed90a9d3684e1e

[MelvinBot]

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

[davidcardoza]

Sending out to engineering and external. I was able to reproduce on iOS.

[MelvinBot]

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

[MelvinBot]

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

[tienifr]

Proposal

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

The screen is flashing when switching focus between edit message fields.

What is the root cause of that problem?

When we move from 1 edit message input to another, the toggleReportActionComposeView in

App/src/pages/home/report/ReportActionItemMessageEdit.js

Line 230 in ff5723c

toggleReportActionComposeView(true, VirtualKeyboard.shouldAssumeIsOpen());

is called, causing the ReportActionCompose to become visible, pushing the view upward and causing the glitch.

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

We should only call toggleReportActionComposeView when the keyboard is completely hidden. So when we switch between edit message inputs, it will not trigger (and it should not because we're still on the edit inputs so there's no need to make the ReportActionCompose visible). If we tap outside of the edit message inputs, the keyboard will be hidden and we can show the ReportActionCompose in this case.

We should only call toggleReportActionComposeView when the keyboard is completely hidden

We can do something like this (only on iOS and Android):

this.keyboardDidHideListener = Keyboard.addListener(
                                'keyboardDidHide',
                                () => {
                                    toggleReportActionComposeView(true, this.props.isSmallScreenWidth);
                                },
                            );

There'll be cleanups of the listener, ... as well but the above is the main change to fix it.

For mWeb we have a related but different issue compared to this, I'm keen to post it in another bug report but I'll post here for the sake of completeness first. For mWeb we will not run toggleReportActionComposeView if the relatedTarget that causes the blur is another message edit input:

if (lodashGet(event, 'nativeEvent.relatedTarget.id') === "MessageEditInput") return;
toggleReportActionComposeView(true, this.props.isSmallScreenWidth);
                           

(MessageEditInput is the id that we can set to the input via nativeID)

What alternative solutions did you explore? (Optional)

NA

Result

Working well after the fix
https://user-images.githubusercontent.com/113963320/218640179-e1adca95-32f4-4e3c-bd5c-74621c812738.mov

[abhishek-2m]

The toggleReportActionComposeView method is likely responsible for showing or hiding the report action compose view, and the VirtualKeyboard.shouldAssumeIsOpen() method is used to check whether the virtual keyboard is currently open or not.

Assuming that the flashing issue is related to the interaction between the report action compose view and the virtual keyboard, we can try the following technical solutions:

1. Use a different keyboard event listener: Instead of relying on VirtualKeyboard.shouldAssumeIsOpen(), we can try using a different keyboard event listener, such as Keyboard.addListener('keyboardDidShow', callback) and Keyboard.addListener('keyboardDidHide', callback). These event listeners can be used to detect when the keyboard is shown or hidden, and we can adjust the layout or behavior of the report action compose view accordingly.
2. Optimize report action compose view rendering: The flashing issue could be related to the way the report action compose view is rendered. We can review the code that renders the report action compose view and optimize it by using a more efficient rendering method, such as PureComponent or React.memo, reducing unnecessary re-renders, or using shouldComponentUpdate to prevent unnecessary updates.
3. Use a different animation method: If the flashing issue is related to the animation of the report action compose view, we can try using a different animation method, such as the Animated API or a third-party animation library. These methods can provide more fine-grained control over the animation and can be optimized for performance.

[bernhardoj]

Proposal

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

The screen is blinking when the focus changes between edit composer.

What is the root cause of that problem?

When we switch the focus to another edit composer, the onBlur will be called on the first composer and inside it, we call toggleReportActionComposeView(true, this.props.isSmallScreenWidth) which will show the main composer. Then the onFocus on the second (that we switched to) edit composer will be called which will call toggleReportActionComposeView(false, this.props.isSmallScreenWidth) to hide the main composer.

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

The simplest solution is to debounce the function call, so we only receive the latest value. Tested with 100ms wait time and it works fine.

[thienlnam]

Removing @MonilBhavsar since it auto-assigned for both Engineering and External (not sure why)

[mananjadhav]

Both the proposals identified the root cause correctly. But I think we should avoid using debounce.

I checked @tienifr's proposal and it makes sense to implement both the KeyboardListener for the native and inputId to the composer. @tienifr's proposal looks good to me.

@thienlnam All yours.

[thienlnam]

Yeah, the proposal looks good to me 👍

[MelvinBot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[MelvinBot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.74-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix/15086: Screen is flashing when changing focus between edit input #15201

If no regressions arise, payment will be issued on 2023-02-28. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[MelvinBot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@mananjadhav / @thienlnam] The PR that introduced the bug has been identified. Link to the PR:
• [@mananjadhav / @thienlnam] 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:
• [@mananjadhav / @thienlnam] 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:
• [@tienifr] Propose regression test steps to ensure the same bug will not reach production again.
• [@davidcardoza] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[mananjadhav]

I'll try to update the checklist by today/tomorrow. Meanwhile @tienifr can you please post a regression test step for this one?

[MelvinBot]

📣 @mananjadhav! 📣

Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.

Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[tienifr]

@mananjadhav @thienlnam I just posted the regression test in the above comment

Regression Test Proposal

Bug: Screen is flashing when changing focus between edit input

Proposed Test Steps:

1. Login with any account
2. Try to select any report, send few messages, try to edit them but don’t press Save. Try to change the focus
3. Verify that the screen isn't flashing when switching focus between the edit fields.

Do we 👍 or 👎

[mananjadhav]

I think we're good with the regression steps. Not sure if we want to split step 2 into multiple steps.

Also I couldn't pinpoint to a specific PR for this regression. I think it exists since the time we've got these Report Edit/Chat features.

@thienlnam What do you think?

[thienlnam]

Not sure if we want to split step 2 into multiple steps.

I agree, let's try to make this more clear for the QA testers.
Can we clarify 'Try to change the focus' to be more specific like Press on the compose input, or add more steps for testing?

Also I couldn't pinpoint to a specific PR for this regression

No worries on not being able to pinpoint the specific PR

[mananjadhav]

@tienifr quick bump on the previous comments to update the regression test.

[tienifr]

Regression Test Proposal

Bug: Screen is flashing when changing focus between edit input

Proposed Test Steps:

1. Login with any account
2. Try to select any report
3. Send few messages
4. Try to edit them but don’t press Save.
5. Press on the compose input
6. Verify that the screen isn't flashing when switching focus between the edit fields.

Do we 👍 or 👎

[tienifr]

@mananjadhav Thanks for pointing that out. I've just updated the regression test proposal, please help check again.

[mananjadhav]

@davidcardoza Can you help with the payout here?

[davidcardoza]

Payments sent apologies for the wait.

[mananjadhav]

Thanks @davidcardoza I have accepted the offer. Can you help with the payout? Also this is eligible for the timeline bonus.

[hungvu193]

@davidcardoza can you send me the offer as well? I'm the reporter.

[mananjadhav]

Thanks @davidcardoza for the payment here. This issue is eligible for 500$ timeline bonus.

and the same amount also needs to be paid to @tienifr for the contribution and a reporting bonus of 250$ to @hungvu193.

[davidcardoza]

Both were sent.

[mananjadhav]

Here's a summary of the payout @davidcardoza.

@tienifr Contributor for creating the fix ($1000) + timeline bonus ($500) = Total $1500
@mananjadhav C+ Review ($1000) + timeline bonus ($500) = Total $1500
@hungvu193 Reporting bonus $250

Hope this helps. Thanks once again for the help here.

[davidcardoza]

All payments have been sent.