[HOLD for payment 2023-05-29] [$1000] Contact suggestion focus/selection does not reset after selecting a contact #18844

kavimuru · 2023-05-12T16:29:24Z

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:

Open a chat and write “@” only to mention a contact
Use the down arrow key on the keyboard to navigate through the contact suggestion list and select the second contact in the suggestion list and hit enter key
Write "@" again

Expected behavior:

The contact suggestion selection should start from the first contact again

Actual behavior:

The contact suggestion selection remains on the previously selected contact

NOTE : This behavior is inconsistent with the emoji suggestion functionality (video attached)

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

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

2023-05-11.14.16.35.mp4

2023-05-11.14.10.18.mp4

Recording.575.mp4

Expensify/Expensify Issue URL:
Issue reported by: @Nathan-Mulugeta
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1683803719905009

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~013072759f95d2c09f
Upwork Job ID: 1657773872119492608
Last Price Increase: 2023-05-14

melvin-bot · 2023-05-12T16:29:29Z

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

melvin-bot · 2023-05-12T16:29:31Z

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

PrashantMangukiya · 2023-05-12T16:59:19Z

Proposal

Posting proposal early as per new guidelines

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

Contact suggestion focus/selection does not reset after selecting a contact.

What is the root cause of that problem?

When we press enter it will run this code.

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

Lines 749 to 758 in 101a3e2

    
           if ((e.key === CONST.KEYBOARD_SHORTCUTS.ENTER.shortcutKey || e.key === CONST.KEYBOARD_SHORTCUTS.TAB.shortcutKey) && suggestionsExist) { 
        
               e.preventDefault(); 
        
               if (this.state.suggestedEmojis.length > 0) { 
        
                   this.insertSelectedEmoji(this.state.highlightedEmojiIndex); 
        
               } 
        
               if (this.state.suggestedMentions.length > 0) { 
        
                   this.insertSelectedMention(this.state.highlightedMentionIndex); 
        
               } 
        
               return; 
        
           }

We can see we are not resetting the index after suggesting. This is the root cause of the problem.

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

We have to reset index as shown in code below i.e. this.setState({highlightedMentionIndex: 0});

if ((e.key === CONST.KEYBOARD_SHORTCUTS.ENTER.shortcutKey || e.key === CONST.KEYBOARD_SHORTCUTS.TAB.shortcutKey) && suggestionsExist) {
    e.preventDefault();
    if (this.state.suggestedEmojis.length > 0) {
        this.insertSelectedEmoji(this.state.highlightedEmojiIndex);
    }
    if (this.state.suggestedMentions.length > 0) {
        this.insertSelectedMention(this.state.highlightedMentionIndex);
        this.setState({highlightedMentionIndex: 0});     // ***  Add this
    }
    return;
}

This will solve the issue as shown in result video.

What alternative solutions did you explore? (Optional)

If we have to match code similar to emoji suggestion then we have to add highlightedMentionIndex: 0 for next state here.

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

Lines 541 to 545 in 101a3e2

    
           const nextState = { 
        
               suggestedMentions: [], 
        
               atSignIndex, 
        
               mentionPrefix: prefix, 
        
           };

So updated code looks as under

const nextState = {
    suggestedMentions: [],
    highlightedMentionIndex: 0, // *** ADD THIS ***
    atSignIndex,
    mentionPrefix: prefix,
};

This will also solve the issue and give same results.

Result

18844-MentionHighlight.mov

isabelastisser · 2023-05-12T19:56:21Z

I won't be able to act on this until next Tuesday (Compliance deadlines), reassigning!

melvin-bot · 2023-05-12T19:56:38Z

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

melvin-bot · 2023-05-12T19:56:39Z

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

alexpensify · 2023-05-13T16:20:17Z

I started an internal discussion here: https://expensify.slack.com/archives/C049HHMV9SM/p1683994795740649?thread_ts=1683803719.905009&cid=C049HHMV9SM

melvin-bot · 2023-05-14T15:44:27Z

Job added to Upwork: https://www.upwork.com/jobs/~013072759f95d2c09f

melvin-bot · 2023-05-14T15:44:30Z

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

melvin-bot · 2023-05-14T15:44:36Z

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

melvin-bot · 2023-05-14T15:44:40Z

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

alexpensify · 2023-05-14T15:45:31Z

I confirmed this is a bug and I can replicate it. With that said, the team confirmed the expectations:

the list selection should start at the top for a new mention input.

For now, I'm marking it as External.

mananjadhav · 2023-05-14T18:18:49Z

Thanks for confirming it @alexpensify. I think we should use @PrashantMangukiya's alternative approach of resetting it in nextState. It is inline with our current approach, we do for EmojiSuggestions.

@amyevans All yours.

dukenv0307 · 2023-05-15T04:24:28Z

@mananjadhav Sorry for my lateness but I see that the selected proposal is not exhaustive and it doesn't cover all cases

Proposal

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

Contact suggestion focus/selection does not reset after selecting a contact

What is the root cause of that problem?

We don't reset suggestion in both calculateMentionSuggestion function and insertSelectedMention function

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

For consistency with emoji suggestion
A. calculateEmojiSuggestion

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

Lines 497 to 505 in 6c10bb2

    
           const hasEnoughSpaceForLargeSuggestion = this.props.windowHeight / this.state.composerHeight >= 6.8; 
        
           const isAutoSuggestionPickerLarge = !this.props.isSmallScreenWidth || (this.props.isSmallScreenWidth && hasEnoughSpaceForLargeSuggestion); 
        
           const nextState = { 
        
               suggestedEmojis: [], 
        
               highlightedEmojiIndex: 0, 
        
               colonIndex, 
        
               shouldShowEmojiSuggestionMenu: false, 
        
               isAutoSuggestionPickerLarge,

B. calculateMentionSuggestion

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

Lines 544 to 548 in 6c10bb2

    
           const nextState = { 
        
               suggestedMentions: [], 
        
               atSignIndex, 
        
               mentionPrefix: prefix, 
        
           };

So that, on calculateMentionSuggestion function we should reset highlightedMentionIndex, shouldShowMentionSuggestionMenu, isAutoSuggestionPickerLarge in the next state

const hasEnoughSpaceForLargeSuggestion = this.props.windowHeight / this.state.composerHeight >= 6.8;
const isAutoSuggestionPickerLarge = !this.props.isSmallScreenWidth || (this.props.isSmallScreenWidth && hasEnoughSpaceForLargeSuggestion);

        const nextState = {
            suggestedMentions: [],
            atSignIndex,
            mentionPrefix: prefix,
            highlightedMentionIndex: 0, // ADD HERE
            shouldShowMentionSuggestionMenu: false, // ADD HERE
            isAutoSuggestionPickerLarge, // ADD HERE
        };

What alternative solutions did you explore? (Optional)

If we want to remain highlightedMentionIndex when the user changes the selection, we can set highlightedMentionIndex: 0 in insertSelectedMention function and do the same thing in the emoji suggestion

Additional Bug

I mentioned it here because it relates to the original issue and the original issue is so straightforward. So I think we should fix this issue
The mentioned suggestion still displays when the cursor is in the first place
https://github.com/Expensify/App/assets/129500732/82c16339-cd81-4020-ba0f-29bfd9c3117e

What is the root cause of that problem?

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

Lines 517 to 520 in 6c10bb2

    
           calculateMentionSuggestion() { 
        
               if (this.state.selection.end < 1) { 
        
                   return; 
        
               }

We don’t reset suggestions when this.state.selection.end < 1

Solution for Additional Bug

when this.state.selection.end < 1, we should call this.resetSuggestions()

Updated

I just see that the additional bug is created here: #18879
We can consider to fix in here or in that issue

PrashantMangukiya · 2023-05-15T14:05:09Z

My proposed solution and root cause analysis is according to the expected result of the issue and solves the problem. So technically my proposal is complete according to the expected result.

There could be many ways to suggest additional changes with proposal, but if I did not suggest additional changes it doesn't mean that my proposal is incomplete. During PR review time we can do other adjustment if decided by reviewer team.

Also if any additional bug is found then there is a procedure to report that bug on Slack and then move forward. By just mentioning that bug and solution in a proposal with current issue does not mean that other proposals are incomplete.

This is just my understanding. I am happy with whatever decision taken by C+ member.

cc: @mananjadhav

dukenv0307 · 2023-05-15T15:03:57Z

I mentioned an additional bug here because I see a similar thing in this issue
But it is my suggestion, the decision depends on the C+ contributor and internal team.

alexpensify · 2023-05-15T22:19:28Z

@amyevans and @mananjadhav - any feedback here?

amyevans · 2023-05-16T00:21:22Z

Hiring @PrashantMangukiya, you can get a PR started please!

amyevans · 2023-05-22T21:50:29Z

Hmm, shouldn't the BZ checklist and Payment Due items have populated?

I don't think so, that happens once the PR is deployed to production and this one is only on staging at the moment!

melvin-bot · 2023-05-22T23:16:59Z

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

melvin-bot · 2023-05-22T23:16:59Z

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

Corrected - Contact suggestion selection does not reset after selecting a contact #18996

If no regressions arise, payment will be issued on 2023-05-29. 🎊

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-05-22T23:17:01Z

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

alexpensify · 2023-05-24T21:08:16Z

@johncschuster and @dylanexpensify - I appreciate your help here! I'm back online and removing your assignment.

alexpensify · 2023-05-27T01:08:38Z

@mananjadhav - when you get a chance can you please fill out the checklist: #18844 (comment)

Thanks!

alexpensify · 2023-05-27T01:11:23Z

@mananjadhav, @PrashantMangukiya, and @Nathan-Mulugeta - To prepare for the payment date, please apply to the job in Upwork:

https://www.upwork.com/jobs/~013072759f95d2c09f

Thanks!

PrashantMangukiya · 2023-05-27T05:36:33Z

@alexpensify Can you please invite me for the job. I have not enough Upwork connect balance to apply. This is my Upwork profile https://www.upwork.com/freelancers/~01c718889ed7821f82 Thank you for your time.

mananjadhav · 2023-05-27T20:59:40Z

We added the Mentions support in this PR #18469, where we didn't reset the index. I don't see any need for a regression test, nor updating the checklist. This can be categorized as missing test case/mistake.

I've commented on the offending PR to let the contributors know.

@amyevans do you think we need a regression test here?

alexpensify · 2023-05-28T15:49:58Z

Thank you for that update @mananjadhav!

mananjadhav · 2023-05-29T23:10:41Z

@alexpensify this is ready for payout.

amyevans · 2023-05-30T14:44:48Z

@amyevans do you think we need a regression test here?

No I think we're fine without.

alexpensify · 2023-05-30T14:47:06Z

Sorry, I should have flagged that I was offline yesterday for the US Bank Holiday but most of the team was offline too. I will work on the payment process today.

alexpensify · 2023-05-30T20:52:41Z

Flagging the urgency bonus here: #18844 (comment)

alexpensify · 2023-05-30T21:27:01Z

Alright, it's payment time-- ring the bell!

@mananjadhav, @PrashantMangukiya, and @Nathan-Mulugeta - please accept in Upwork and I can complete the payment process. Thanks!

PrashantMangukiya · 2023-05-30T22:31:41Z

@alexpensify Offer accepted on Upwork.

Nathan-Mulugeta · 2023-05-31T04:42:32Z

Just accepted offer @alexpensify

alexpensify · 2023-05-31T19:47:09Z

Alright, @mananjadhav, @PrashantMangukiya, and @Nathan-Mulugeta have been paid via Upwork. Great work here, the job is closed now!

kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels May 12, 2023

melvin-bot bot assigned isabelastisser May 12, 2023

isabelastisser removed their assignment May 12, 2023

isabelastisser added Bug Something is broken. Auto assigns a BugZero manager. and removed Bug Something is broken. Auto assigns a BugZero manager. labels May 12, 2023

melvin-bot bot assigned alexpensify May 12, 2023

alexpensify added the External Added to denote the issue can be worked on by a contributor label May 14, 2023

melvin-bot bot changed the title ~~Contact suggestion focus/selection does not reset after selecting a contact~~ [$1000] Contact suggestion focus/selection does not reset after selecting a contact May 14, 2023

melvin-bot bot assigned mananjadhav May 14, 2023

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

melvin-bot bot assigned amyevans May 14, 2023

dylanexpensify self-assigned this May 22, 2023

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

melvin-bot bot changed the title ~~[$1000] Contact suggestion focus/selection does not reset after selecting a contact~~ [HOLD for payment 2023-05-29] [$1000] Contact suggestion focus/selection does not reset after selecting a contact May 22, 2023

melvin-bot bot removed the Reviewing Has a PR in review label May 22, 2023

alexpensify unassigned johncschuster and dylanexpensify May 24, 2023

mananjadhav mentioned this issue May 27, 2023

Add mention suggestions #18469

Merged

57 tasks

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels May 29, 2023

alexpensify closed this as completed May 31, 2023

[HOLD for payment 2023-05-29] [$1000] Contact suggestion focus/selection does not reset after selecting a contact #18844

[HOLD for payment 2023-05-29] [$1000] Contact suggestion focus/selection does not reset after selecting a contact #18844

Comments

kavimuru commented May 12, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected behavior:

Actual behavior:

Workaround:

Platforms:

melvin-bot bot commented May 12, 2023

melvin-bot bot commented May 12, 2023

Bug0 Triage Checklist (Main S/O)

PrashantMangukiya commented May 12, 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)

isabelastisser commented May 12, 2023

melvin-bot bot commented May 12, 2023

melvin-bot bot commented May 12, 2023

Bug0 Triage Checklist (Main S/O)

alexpensify commented May 13, 2023

melvin-bot bot commented May 14, 2023

melvin-bot bot commented May 14, 2023

melvin-bot bot commented May 14, 2023

melvin-bot bot commented May 14, 2023

alexpensify commented May 14, 2023

mananjadhav commented May 14, 2023

dukenv0307 commented May 15, 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?

What alternative solutions did you explore? (Optional)

Additional Bug

What is the root cause of that problem?

Solution for Additional Bug

Updated

PrashantMangukiya commented May 15, 2023

dukenv0307 commented May 15, 2023

alexpensify commented May 15, 2023

amyevans commented May 16, 2023

amyevans commented May 22, 2023

melvin-bot bot commented May 22, 2023

melvin-bot bot commented May 22, 2023

melvin-bot bot commented May 22, 2023

alexpensify commented May 24, 2023

alexpensify commented May 27, 2023

alexpensify commented May 27, 2023 • edited Loading

PrashantMangukiya commented May 27, 2023 • edited Loading

mananjadhav commented May 27, 2023

alexpensify commented May 28, 2023

mananjadhav commented May 29, 2023

amyevans commented May 30, 2023

alexpensify commented May 30, 2023

alexpensify commented May 30, 2023

alexpensify commented May 30, 2023

PrashantMangukiya commented May 30, 2023

Nathan-Mulugeta commented May 31, 2023

alexpensify commented May 31, 2023 • edited Loading

kavimuru commented May 12, 2023 •

edited by melvin-bot bot

Loading

dukenv0307 commented May 15, 2023 •

edited

Loading

alexpensify commented May 27, 2023 •

edited

Loading

PrashantMangukiya commented May 27, 2023 •

edited

Loading

alexpensify commented May 31, 2023 •

edited

Loading