[HOLD for payment 2022-10-05] [$250] IOS - Chat - User get highlighted in grey when message is sent

[kavimuru]

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

---

Action Performed:

1. Launch the app
2. Log in with the same expensifail account
3. Select user and send message
4. Select another user and send message
5. Back to LHN

Expected Result:

User should not get highlighted in grey when message is sent

Actual Result:

User get highlighted in grey when message is sent

Workaround:

Unknown

Platform:

Where is this issue occurring?

• iOS

Version Number: 1.2.5.0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): applausetester+09162022abb@applause.expensifail.com / Feya86Katya
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Bug5744747_RPReplay_Final1663886502.mp4

Expensify/Expensify Issue URL:
Issue reported by: Applause internal team
Slack conversation:

View all open jobs on GitHub

[melvin-bot]

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

[neil-marcellini]

Yeah that's odd. Sending external.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[mollfpr]

Proposal

Root cause
We have option disableFocusOptions but not implemented on LHNOptionsList. The prop is used to determine should the option row has focused or not.

App/src/pages/home/sidebar/SidebarLinks.js

Line 165 in b75faf0

disableFocusOptions={this.props.isSmallScreenWidth}

It's already implemented on the OptionList component.

App/src/components/OptionsList/BaseOptionsList.js

Lines 176 to 177 in b75faf0

	optionIsFocused={!this.props.disableFocusOptions
	&& this.props.focusedIndex === (index + section.indexOffset)}

Solution
Use disableFocusOptions from SidebarLinks to determine the option row focused.

diff --git a/src/components/LHNOptionsList/LHNOptionsList.js b/src/components/LHNOptionsList/LHNOptionsList.js
index 941057579..1b39dc546 100644
--- a/src/components/LHNOptionsList/LHNOptionsList.js
+++ b/src/components/LHNOptionsList/LHNOptionsList.js
@@ -26,6 +26,9 @@ const propTypes = {

     /** Callback to execute when the SectionList lays out */
     onLayout: PropTypes.func.isRequired,
+
+    /** Disable focus options */
+    disableFocusOptions: PropTypes.bool,
 };

 class LHNOptionsList extends Component {
@@ -69,7 +72,7 @@ class LHNOptionsList extends Component {
             <OptionRowLHN
                 reportID={item}
                 viewMode={this.props.optionMode}
-                isFocused={this.props.focusedIndex === index}
+                isFocused={!this.props.disableFocusOptions && this.props.focusedIndex === index}
                 onSelectRow={this.props.onSelectRow}
             />
         );
@@ -100,5 +103,8 @@ class LHNOptionsList extends Component {
 }

 LHNOptionsList.propTypes = propTypes;
+LHNOptionsList.defaultProps = {
+    disableFocusOptions: false,
+};

 export default LHNOptionsList;

Result

Screen.Recording.2022-09-24.at.23.55.40.mov

[Puneet-here]

Proposal

isFocused is being used to know which option is focused and we apply styles to the focused option. We want this functionality to work for LHN at big screens but not for small screens.

We can use withWindowDimensions to know the screen width and we can just check if it's a small screen width device

Replace this line 72 with

isFocused={!this.props.isSmallScreenWidth && this.props.focusedIndex === index}

App/src/components/LHNOptionsList/LHNOptionsList.js

Line 72 in b75faf0

isFocused={this.props.focusedIndex === index}

This way we won't see the highlighted row in the mobiles

[rushatgabhane]

@luacmartins I like @mollfpr's proposal.
We most likely forgot to implement disableFocusOptions while refactoring LHNOptionsList in #10800

disableFocusOptions in SidebarLinks should take care of disabling it for small screens.

App/src/pages/home/sidebar/SidebarLinks.js

Line 146 in e33cfb3

disableFocusOptions={this.props.isSmallScreenWidth}

🎀 👀 🎀 C+ reviewed

[luacmartins]

@mollfpr your proposal looks good. Assigning you to the issue!

[melvin-bot]

📣 @mollfpr You have been assigned to this job by @luacmartins!
Please apply to this job in Upwork 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 📖

[mollfpr]

@luacmartins @rushatgabhane PR raised!

[melvin-bot]

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

• fix hightlighted option chat on mobile #11289

If no regressions arise, payment will be issued on 2022-10-05. 🎊

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[arielgreen]

@mollfpr @rushatgabhane sent Upwork invite, please accept and I will remit payment

[mollfpr]

@arielgreen Accepted, thank you!

[arielgreen]

Paid.