[HOLD for payment 2023-07-03] [$1000] Feature Request: Make the mentions auto-complete menu scrollable

[JmillsExpensify]

Problem: I have too many friends on NewDot! :sonriente: :champán: More to the point, I know too many contacts with the name Tom, and the mentions auto-complete menu only returns the first five results, which means I don’t have a way to actually mention the Tom I’m chatting with.

Solution: Make the mentions auto-complete menu scrollable. This way, if the top five results aren’t the one you’re looking for, I still have a way to find and select the right person.
Example in thread.

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0125202f31d3cf1b51
• Upwork Job ID: 1666898796516528128
• Last Price Increase: 2023-06-15

[melvin-bot]

Current assignee @JmillsExpensify is eligible for the NewFeature assigner, not assigning anyone new.

[JmillsExpensify]

Opening this one up externally. More discussion here and please clarify any questions you might have.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0125202f31d3cf1b51

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[agalyan]

Contributor details
Your Expensify account email: arman.galyan@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~019cd393456c38df2c

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[agalyan]

@JmillsExpensify, could you please provide some guidance? I sent an email to contributors@expensify.com a week ago with the subject Slack Channel Invites, but I haven't received an invite yet. Is there anything I can do to expedite the contribution process?

[Nikhil-Vats]

Proposal

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

Mention Suggestions are limited to 5 and not scrollable.

What is the root cause of that problem?

We are limiting suggestions to 5 here -

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

Lines 478 to 480 in 38ba7c8

	const sortedPersonalDetails = _.sortBy(filteredPersonalDetails, (detail) => detail.displayName || detail.login);
	_.each(_.first(sortedPersonalDetails, CONST.AUTO_COMPLETE_SUGGESTER.MAX_AMOUNT_OF_ITEMS - suggestions.length), (detail) => {
	suggestions.push({

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

1. To show all suggestions (not just 5) - We need to remove the use of first function and change the line above to this so we only use _.each function -

 _.each(sortedPersonalDetails, (detail) => {

2. To use scroll with a fixed maxHeight - The height of the container of suggestions is based on the number of suggestions. We also need to change how we calculate height in measureHeightOfSuggestionRows function -
   

   App/src/components/AutoCompleteSuggestions/BaseAutoCompleteSuggestions.js

   Lines 16 to 25 in 38ba7c8

   	const measureHeightOfSuggestionRows = (numRows, isSuggestionPickerLarge) => {
   	if (isSuggestionPickerLarge) {
   	return numRows * CONST.AUTO_COMPLETE_SUGGESTER.ITEM_HEIGHT;
   	}
   	if (numRows > 2) {
   	// On small screens, we display a scrollable window with a height of 2.5 items, indicating that there are more items available beyond what is currently visible
   	return CONST.AUTO_COMPLETE_SUGGESTER.SMALL_CONTAINER_HEIGHT_FACTOR * CONST.AUTO_COMPLETE_SUGGESTER.ITEM_HEIGHT;
   	}
   	return numRows * CONST.AUTO_COMPLETE_SUGGESTER.ITEM_HEIGHT;
   	};

   If numRows are more than 5 (MAX_AMOUNT_OF_ITEMS in const) then we will use MAX_AMOUNT_OF_ITEMS i.e. 5 to calculate height so that the maximum height is fixed and container is scrollable. We only need to do this isSuggestionPickerLarge is true since the other case is already handled by line 20 in function above.
   Note - We should also change name of MAX_AMOUNT_OF_ITEMS const because it is no more the max no of suggestions rather a height factor.

   We change this function because it is used to calculate the top position and height of container -
   itemsHeight parameter in the below function is passed the output of the function measureHeightOfSuggestionRows -
   

   App/src/styles/StyleUtils.js

   Lines 1017 to 1032 in 38ba7c8

   	function getAutoCompleteSuggestionContainerStyle(itemsHeight, shouldIncludeReportRecipientLocalTimeHeight) {
   	'worklet';
   	const optionalPadding = shouldIncludeReportRecipientLocalTimeHeight ? CONST.RECIPIENT_LOCAL_TIME_HEIGHT : 0;
   	const padding = CONST.AUTO_COMPLETE_SUGGESTER.SUGGESTER_PADDING - optionalPadding;
   	const borderWidth = 2;
   	const height = itemsHeight + 2 * CONST.AUTO_COMPLETE_SUGGESTER.SUGGESTER_INNER_PADDING + borderWidth;
   	// The suggester is positioned absolutely within the component that includes the input and RecipientLocalTime view (for non-expanded mode only). To position it correctly,
   	// we need to shift it by the suggester's height plus its padding and, if applicable, the height of the RecipientLocalTime view.
   	return {
   	overflow: 'hidden',
   	top: -(height + padding),
   	height,
   	};
   	}

   
   The output of getAutoCompleteSuggestionContainerStyle is used for container styles.

3. We also need to show the scroll bar when list is scrollable -
   For that, we just need to fix this regression from fix: prevent scrolling in suggestions popup propagating to layers below #19621 -
   

   App/src/components/AutoCompleteSuggestions/BaseAutoCompleteSuggestions.js

   Line 71 in 28ad1da

   showsVerticalScrollIndicator={innerHeight > rowHeight}

   rowHeight here is an object not a number -
   

   App/src/components/AutoCompleteSuggestions/BaseAutoCompleteSuggestions.js

   Line 53 in 28ad1da

   rowHeight.value = withTiming(measureHeightOfSuggestionRows(props.suggestions.length, props.isSuggestionPickerLarge), {

   we need to pass innerHeight > rowHeight.value as value

Result

Screen.Recording.2023-06-13.at.2.16.55.AM.mov

[Ayush-004]

Contributor details
Your Expensify account email: ayush2003malik@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~016cb0d3ee1d88e304

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[puneetlath]

@JmillsExpensify if @abdulrahuman5196 chooses a proposal next week while I'm out, please un-assign me and use the Engineering label to get another internal engineer assigned. Thanks!

[abdulrahuman5196]

Will check on proposals today

[abdulrahuman5196]

@Nikhil-Vats I like the proposal on high level. Just some questions or changes with the expectation.

We should also change name of MAX_AMOUNT_OF_ITEMS const because it is no more the max no of suggestions rather a height factor.

I don't think we should not have a limit. IMO, We should have a max limit like 20 or something. So that if I have 1000 contacts and if i just type 'a' we don't end up showing lots of contacts in the suggestions.

2. We should show scrollbar in the suggestion popup then right when user is able to scroll? or not required?

@JmillsExpensify What do you think of the above questions?

[Nikhil-Vats]

@abdulrahuman5196 the scroll bar did come up in my mind and I think we should show it too! But I noticed it was missing in most places like the LHN, search chats page, any of the request/send/split money participant selection steps, etc.
I will add it to the alternative solution section of my proposal so we can add it if all of you agree.

And regarding the limit, sure we can adjust it to whatever the team wants. @JmillsExpensify was the one who suggested this feature so maybe he can suggest or we can discuss it in the original Slack thread.

[Nikhil-Vats]

@abdulrahuman5196 Proposal updated to show scrollbar. It was a regression from a PR otherwise it should have been visible in the first place.

[Nikhil-Vats]

@abdulrahuman5196 @JmillsExpensify confirmed on Slack that we should go with 20 mentions max like you suggested. Are we good to go?

[melvin-bot]

🎯 ⚡️ Woah @abdulrahuman5196 / @Nikhil-Vats, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

• when @Nikhil-Vats got assigned: 2023-06-20 16:21:41 Z
• when the PR got merged: 2023-06-23 16:05:54 UTC

On to the next one 🚀

[melvin-bot]

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

[melvin-bot]

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

• Feature: Make the mentions auto-complete menu scrollable #21196

If no regressions arise, payment will be issued on 2023-07-03. 🎊

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 n/a
• Contributor that fixed the issue @Nikhil-Vats
• Contributor+ that helped on the issue and/or PR @abdulrahuman5196

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]

@JmillsExpensify, @puneetlath, @Nikhil-Vats, @abdulrahuman5196 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[puneetlath]

@JmillsExpensify I think this is just waiting for payment and then we're all done.

[melvin-bot]

@JmillsExpensify, @puneetlath, @Nikhil-Vats, @abdulrahuman5196 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[JmillsExpensify]

Thanks, sounds great! Let's summarize where we ended up:

• External issue reporter: NA
• Contributor: @Nikhil-Vats $1,500
• Contributor+: @abdulrahuman5196 $1,500

Everyone has been paid out via Upwork. That said, what about a regression test for this new feature?

[abdulrahuman5196]

Since melvin bot didn't add BZ checklist(I think it doesn't add for new feature labels), I missed to add.
Will anyways suggest a regression test before tomorrow morning

[abdulrahuman5196]

@JmillsExpensify
Regression test:

1. Login using an account with more than 20 contacts.
2. Open a chat and type @.
3. Verify 20 mentions are present in the list and maximum 5 suggestions are shown at once (maximum height of menu is 5 items, we need to scroll to see the rest).
   4 Verify that mention menu is scrollable and scrollbar is visible.
4. Start typing further and mentions should be filtered further.
5. Now type :he in composer. Verify 20 emojis are shown and menu is scrollable.
6. Type further (eg - :heart_) and emojis should be filtered further.

[abdulrahuman5196]

Not overdue,
We have to create test for this #20481 (comment) and then we should be good to close it out @JmillsExpensify

[melvin-bot]

@JmillsExpensify, @puneetlath, @Nikhil-Vats, @abdulrahuman5196 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[puneetlath]

@JmillsExpensify that regression test looks good to me. Do you agree?

[JmillsExpensify]

Yeah agreed! I'll get an issue created for this shortly and then close this issue out.

[JmillsExpensify]

Test created. Closing this one out!