[HOLD for payment 2023-06-28] [$1000] mweb - Chat - Reaction list does not appear after navigating to chat from member detail #20513

kbecciv · 2023-06-09T12:50:00Z

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 #19612

Action Performed:

Go to https://staging.new.expensify.com/
Go to any group chat with reactions to messages.
Long press on the reaction.
Tap on the group chat header.
Tap on any member > Message member.
Return to group chat in Step 2.
Long press on the reaction.

Expected Result:

Reaction list shows up.

Actual Result:

Reaction list does not show up.

Workaround:

Unknown

Platforms:

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

Version Number: 1.3.26.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

Bug6086568_Screen_Recording_20230609_193946_Chrome.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/~0162c97fe6a772efa5
Upwork Job ID: 1667199561562185728
Last Price Increase: 2023-06-09

melvin-bot · 2023-06-09T12:50:05Z

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

melvin-bot · 2023-06-09T12:50:08Z

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

melvin-bot · 2023-06-09T15:58:47Z

Job added to Upwork: https://www.upwork.com/jobs/~0162c97fe6a772efa5

melvin-bot · 2023-06-09T15:58:50Z

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

melvin-bot · 2023-06-09T15:58:55Z

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

melvin-bot · 2023-06-09T15:59:00Z

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

maxim-abuzarov · 2023-06-09T19:53:50Z

Proposal

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

Reaction list does not appear after navigating to chat from member detail

What is the root cause of that problem?

After re-render we have null instead of ref and get in this part of logic:

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

Lines 14 to 16 in d5374be

    
           if (!reactionListRef.current) { 
        
               return; 
        
           }

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

add in constructor
this.ref = ReactionList.reactionListRef;
this.setTextInputRef = this.setTextInputRef.bind(this);

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

Lines 60 to 66 in d5374be

    
           constructor(props) { 
        
               super(props); 
        
               this.didLayout = false; 
        
               this.didSubscribeToReportTypingEvents = false; 
        
               this.unsubscribeVisibilityListener = null; 
        
               this.hasCachedActions = _.size(props.reportActions) > 0;

add in shoudComponentUpdate
if (ReactionList.reactionListRef !== this.ref) {return true}

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

Lines 129 to 133 in d5374be

    
           shouldComponentUpdate(nextProps, nextState) { 
        
               if (!_.isEqual(nextProps.reportActions, this.props.reportActions)) { 
        
                   this.mostRecentIOUReportActionID = ReportActionsUtils.getMostRecentIOURequestActionID(nextProps.reportActions); 
        
                   return true; 
        
               }

add function in ReportActionsView class component
setTextInputRef(el) { ReactionList.reactionListRef.current = el; this.ref = el }
change this

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

Line 357 in d5374be

ref={ReactionList.reactionListRef}

to this:
ref={(ref) => ref === null ? this.ref : this.setTextInputRef(ref)}

What alternative solutions did you explore? (Optional)

jeet-dhandha · 2023-06-09T20:52:04Z

Proposal

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

The emoji picker stops working when you switch from a group chat to a member's chat using the group's details page, and then return to the group chat.

What is the root cause of that problem?

Here we have two issues:

When ReportScreen unmounts the ref for the EmojiPicker is set to null, and when the ReportScreen is re-rendered the ref is not updated. This causes the EmojiPicker to not be able to be used.
When ReportActionsView unmounts the ref for the PopoverReactionList is set to null, and when the ReportActionsView is re-rendered the ref is not updated. This causes the PopoverReactionList to not be able to be used.

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

To resolve these two issue we need to do the following:

Adding a way to update the ref for the EmojiPicker when the ReportScreen is re-rendered.

class ReportScreen extends React.Component {
  constructor(props) {
      ... rest code
      this.updateEmojiPickerRef = this.updateEmojiPickerRef.bind(this);
      ... rest code
      this.emojiPickerRef = React.createRef();
  }

  /**
  * Function - Resets Ref for Emoji Picker
  * @explanation :
  * When the component unmounts the ref sets to null for the Emoji Picker,
  * this function resets the ref to the current Emoji Picker's Ref.
  */ 
  updateEmojiPickerRef() {
      if(!this.emojiPickerRef.current || EmojiPickerAction.emojiPickerRef.current ) { return; }
      EmojiPickerAction.emojiPickerRef.current = this.emojiPickerRef.current;
  }

  ... rest functions

  render() {
    ... rest code 
    
    // Function will update the ref if not already set (when the component is re-rendered)
    this.updateEmojiPickerRef()

    return (
      ... rest code 
      // Updated ref will be passed to the EmojiPicker component
      <EmojiPicker ref={this.emojiPickerRef} />
      ... rest code 
    )
  }
}

Adding a way to update the ref for the PopoverReactionList when the ReportActionsView is re-rendered.

class ReportActionsView extends React.Component {
  constructor(props) {
      ... rest code
      this.updatePopoverReactionListRef = this.updatePopoverReactionListRef.bind(this);
      this.popoverReactionListRef = React.createRef();
  }

  ... rest functions

  /**
  * Function - Resets Ref for PopoverReactionList
  * @explanation :
  * When the component unmounts the ref sets to null for the PopoverReactionList,
  * this function resets the ref to the current PopoverReactionList's Ref.
  */ 
  updatePopoverReactionListRef() {
    if(!this.popoverReactionListRef.current || ReactionList.reactionListRef.current) { return; }
    ReactionList.reactionListRef.current = this.popoverReactionListRef.current;
  }

  render() {
    ... rest code 
    
    // Function will update the ref if not already set (when the component is re-rendered)
    this.updatePopoverReactionListRef()

    return (
      ... rest code 
      // Updated ref will be passed to the EmojiPicker component
      <PopoverReactionList
        ref={ReactionList.reactionListRef}
        ref={this.popoverReactionListRef}
        reportID={this.props.report.reportID}
      />
      ... rest code 
    )
  }
}

Before Fix:

Screen.Recording.2023-06-10.at.2.23.44.AM.mov

After Fix:

Screen.Recording.2023-06-10.at.2.22.53.AM.mov

Thank You,
Jeet Dhandha.

bernhardoj · 2023-06-10T08:16:23Z

Proposal

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

Reaction list modal does not show up anymore after going to another chat from participants list and go back to current chat.

What is the root cause of that problem?

When we long press the reaction, it will call ReactionList.showReactionList.

App/src/components/Reactions/ReportActionItemReactions.js

Lines 69 to 71 in 9f1c0e0

    
           const onReactionListOpen = (event) => { 
        
               ReactionList.showReactionList(event, popoverReactionListAnchor.current, reaction.emoji, props.reportActionID); 
        
           };

As the others explained, the reactionListRef.current is null after went back from other user chat.

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

Lines 13 to 19 in 9f1c0e0

    
           function showReactionList(event, reactionListPopoverAnchor, emojiName, reportActionID) { 
        
               if (!reactionListRef.current) { 
        
                   return; 
        
               } 
        
               reactionListRef.current.showReactionList(event, reactionListPopoverAnchor, emojiName, reportActionID); 
        
           }

The ref we have (reactionListRef) currently is global and being set here.

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

Lines 356 to 357 in 9f1c0e0

    
           <PopoverReactionList 
        
               ref={ReactionList.reactionListRef}

If ReportActionsView unmounts, reactionListRef will become null. So, in this issue case:

Open a group chat, ReportActionsView is mounted and reactionListRef is set.
Open participants list and start a chat with different member of the group.
Once again, ReportActionsView is mounted and reactionListRef is set.
When we go back, ReportActionsView is unmounted and reactionListRef is now null

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

We should replace the global ref with just a local ref.

Remove ReactionList.js (this is the file that store the global ref).
Create a new local ref in ReportActionsView called reactionListRef and pass it to PopoverReactionList ref.

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

Lines 356 to 357 in 9f1c0e0

<PopoverReactionList

ref={ReactionList.reactionListRef}
To pass the ref deep down to ReportActionItemReactions, we can use react context. We can name it ReactionListRefContext. Then, use the provider to provide the value by wrapping ReportActionsList.

<ReactionListRefContext.Provider value={this.reactionListRef}>

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

Lines 346 to 347 in 9f1c0e0

    
           <ReportActionsList 
        
               report={this.props.report}

4. In ReportActionItemReactions, receive the context with useContext.
const reactionListRef = useContext(ReactionListRefContext);
5. We can use this new ref now to open the reaction list
reactionListRef.current.showReactionList

App/src/components/Reactions/ReportActionItemReactions.js

Lines 69 to 71 in 9f1c0e0

    
           const onReactionListOpen = (event) => { 
        
               ReactionList.showReactionList(event, popoverReactionListAnchor.current, reaction.emoji, props.reportActionID); 
        
           };

thesahindia · 2023-06-12T17:59:01Z

I think I prefer @bernhardoj's proposal.

C+ reviewed 🎀👀🎀

cc: @francoisl

francoisl · 2023-06-13T18:43:07Z

👍 yeah I like the idea of making the ref local instead, let's go with that proposal.

melvin-bot · 2023-06-13T18:43:18Z

📣 @bernhardoj You have been assigned to this job by @francoisl!
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 📖

bernhardoj · 2023-06-14T04:22:59Z

PR is ready for review.

melvin-bot · 2023-06-15T23:11:06Z

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

when @bernhardoj got assigned: 2023-06-13 18:43:18 Z
when the PR got merged: 2023-06-15 23:11:06 UTC
days elapsed: 3

On to the next one 🚀

bernhardoj · 2023-06-16T10:15:41Z

I think it's still within 3 days, right?

slafortune · 2023-06-19T17:02:42Z

@bernhardoj - yep, I think Melvin messed up math there!

melvin-bot · 2023-06-21T01:55:57Z

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

melvin-bot · 2023-06-21T01:55:58Z

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

Fix unable to open reaction list after coming back from another chat #20739

If no regressions arise, payment will be issued on 2023-06-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

melvin-bot · 2023-06-21T01:56:00Z

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:

[@thesahindia] The PR that introduced the bug has been identified. Link to the PR:
[@thesahindia] 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:
[@thesahindia] 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:
[@thesahindia] Determine if we should create a regression test for this bug.
[@thesahindia] 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.
[@slafortune] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

melvin-bot · 2023-07-04T20:11:29Z

@francoisl, @slafortune, @bernhardoj, @thesahindia Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

slafortune · 2023-07-05T15:05:05Z

@bernhardoj @thesahindia offers sent!

@thesahindia can you please complete the checklist?

thesahindia · 2023-07-10T06:24:21Z

@thesahindia can you please complete the checklist?

We can skip it. We shouldn't call it a regression. It was an edge case that we couldn't have thought about.

If we want we can add a test case for this. Steps -

Open a group chat or create one if you don't have
Add an emoji reaction to a chat message
In Web/Desktop, right click the reaction. In Android/iOS/mWeb, long press the reaction
Verify a reaction list is displayed
Close the reaction list
Open the group participants
Select any participant
Press/click "Message {participant name}"
Verify you are navigated to the chat page with the user you selected
Go back to the previous chat
Repeat step 3-4

thesahindia · 2023-07-10T06:25:10Z

@slafortune, I am going to request money on new dot, so no need to pay me on Upwork.

slafortune · 2023-07-11T21:41:48Z

https://github.com/Expensify/Expensify/issues/299211

JmillsExpensify · 2023-08-24T05:28:00Z

@slafortune Can you please leave a payment summary for this issue so that I can issue payment via NewDot?

thesahindia · 2023-08-29T19:28:10Z

@bernhardoj - yep, I think Melvin messed up math there!

Hey @slafortune can you please confirm the C+ compensation on this issue? I believe it was $1500 since it was eligible for the speed bonus.

slafortune · 2023-08-29T21:45:24Z

Oh yes, you are correct - and Melvin had a hard time with math - #20513 (comment)

@JmillsExpensify
C+ @thesahindia also qualifies for the bonus within 3 days - payment should be $1500

JmillsExpensify · 2023-08-30T04:14:09Z

Reviewed the details for @thesahindia. $1,500 approved for payment via NewDot based on BZ summary above.

kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 9, 2023

melvin-bot bot assigned slafortune Jun 9, 2023

This was referenced Jun 9, 2023

17013 - migrate OptionRow to PressableWithFeedback #19612

Merged

Deploy Checklist: New Expensify 2023-06-08 #20466

Closed

slafortune added the External Added to denote the issue can be worked on by a contributor label Jun 9, 2023

melvin-bot bot changed the title ~~mweb - Chat - Reaction list does not appear after navigating to chat from member detail~~ [$1000] mweb - Chat - Reaction list does not appear after navigating to chat from member detail Jun 9, 2023

melvin-bot bot assigned thesahindia Jun 9, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 9, 2023

melvin-bot bot assigned francoisl Jun 9, 2023

melvin-bot bot added the Overdue label Jun 12, 2023

melvin-bot bot removed the Overdue label Jun 12, 2023

francoisl assigned bernhardoj Jun 13, 2023

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 13, 2023

bernhardoj mentioned this issue Jun 14, 2023

Fix unable to open reaction list after coming back from another chat #20739

Merged

57 tasks

melvin-bot bot added the Reviewing Has a PR in review label Jun 14, 2023

bernhardoj mentioned this issue Jun 16, 2023

[HOLD for payment 2023-07-05] [$1000] Chat - App crashes when edit comment clicked for mark down message #20847

Closed

6 tasks

bernhardoj mentioned this issue Jun 20, 2023

[HOLD for payment 2023-07-17] [$1000] mWeb - Emoji modal stops working in the chat if the user opens it in the IOU report. #21101

Closed

6 tasks

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels Jun 21, 2023

melvin-bot bot changed the title ~~[$1000] mweb - Chat - Reaction list does not appear after navigating to chat from member detail~~ [HOLD for payment 2023-06-28] [$1000] mweb - Chat - Reaction list does not appear after navigating to chat from member detail Jun 21, 2023

melvin-bot bot removed the Reviewing Has a PR in review label Jun 21, 2023

melvin-bot bot added Reviewing Has a PR in review Daily KSv2 and removed Weekly KSv2 labels Jun 21, 2023

slafortune closed this as completed Jul 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2023-06-28] [$1000] mweb - Chat - Reaction list does not appear after navigating to chat from member detail #20513

[HOLD for payment 2023-06-28] [$1000] mweb - Chat - Reaction list does not appear after navigating to chat from member detail #20513

kbecciv commented Jun 9, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Jun 9, 2023

melvin-bot bot commented Jun 9, 2023 •

edited by slafortune

Loading

melvin-bot bot commented Jun 9, 2023

melvin-bot bot commented Jun 9, 2023

melvin-bot bot commented Jun 9, 2023

melvin-bot bot commented Jun 9, 2023

maxim-abuzarov commented Jun 9, 2023

jeet-dhandha commented Jun 9, 2023 •

edited

Loading

bernhardoj commented Jun 10, 2023

thesahindia commented Jun 12, 2023

francoisl commented Jun 13, 2023

melvin-bot bot commented Jun 13, 2023

bernhardoj commented Jun 14, 2023

melvin-bot bot commented Jun 15, 2023

bernhardoj commented Jun 16, 2023

slafortune commented Jun 19, 2023

melvin-bot bot commented Jun 21, 2023

melvin-bot bot commented Jun 21, 2023

melvin-bot bot commented Jun 21, 2023

melvin-bot bot commented Jul 4, 2023

slafortune commented Jul 5, 2023

thesahindia commented Jul 10, 2023

thesahindia commented Jul 10, 2023

slafortune commented Jul 11, 2023

JmillsExpensify commented Aug 24, 2023

thesahindia commented Aug 29, 2023

slafortune commented Aug 29, 2023

JmillsExpensify commented Aug 30, 2023

[HOLD for payment 2023-06-28] [$1000] mweb - Chat - Reaction list does not appear after navigating to chat from member detail #20513

[HOLD for payment 2023-06-28] [$1000] mweb - Chat - Reaction list does not appear after navigating to chat from member detail #20513

Comments

kbecciv commented Jun 9, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

melvin-bot bot commented Jun 9, 2023

melvin-bot bot commented Jun 9, 2023 • edited by slafortune Loading

Bug0 Triage Checklist (Main S/O)

melvin-bot bot commented Jun 9, 2023

melvin-bot bot commented Jun 9, 2023

melvin-bot bot commented Jun 9, 2023

melvin-bot bot commented Jun 9, 2023

maxim-abuzarov commented Jun 9, 2023

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

jeet-dhandha commented Jun 9, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

Before Fix:

After Fix:

bernhardoj commented Jun 10, 2023

Proposal

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

What is the root cause of that problem?

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

thesahindia commented Jun 12, 2023

francoisl commented Jun 13, 2023

melvin-bot bot commented Jun 13, 2023

bernhardoj commented Jun 14, 2023

melvin-bot bot commented Jun 15, 2023

bernhardoj commented Jun 16, 2023

slafortune commented Jun 19, 2023

melvin-bot bot commented Jun 21, 2023

melvin-bot bot commented Jun 21, 2023

melvin-bot bot commented Jun 21, 2023

melvin-bot bot commented Jul 4, 2023

slafortune commented Jul 5, 2023

thesahindia commented Jul 10, 2023

thesahindia commented Jul 10, 2023

slafortune commented Jul 11, 2023

JmillsExpensify commented Aug 24, 2023

thesahindia commented Aug 29, 2023

slafortune commented Aug 29, 2023

JmillsExpensify commented Aug 30, 2023

kbecciv commented Jun 9, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Jun 9, 2023 •

edited by slafortune

Loading

jeet-dhandha commented Jun 9, 2023 •

edited

Loading