LHN - Display chats with an existing draft under pinned chats in LHN

[isagoico]

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. Navigate to a conversation that's not on the top of LHN
2. Write something in the compose box.
3. Navigate away from the chat

Expected Result:

Conversation with a draft message should be displayed on the top of LHN for visibility

Actual Result:

Conversation is not displayed on the top and can be easily lost in the LHN conversation list.

Workaround:

User can manually look for the conversation with the drafted message.

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.0.85-0

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos:

Expensify/Expensify Issue URL:

View all open jobs on Upwork

---

From @rafecolton https://expensify.slack.com/archives/C01GTK53T8Q/p1628786564265600

Chats with draft messages no longer show at the top of the LHN below pinned chats. Unless I missed a conversation somewhere, this is a regression. Edit: I'm on macOS desktop 1.0.82-7

[MelvinBot]

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

[aldo-expensify]

By looking at the code in

App/src/libs/OptionsListUtils.js

Lines 387 to 428 in ff62078

	if (includeRecentReports) {
	for (let i = 0; i < allReportOptions.length; i++) {
	// Stop adding options to the recentReports array when we reach the maxRecentReportsToShow value
	if (recentReportOptions.length > 0 && recentReportOptions.length === maxRecentReportsToShow) {
	break;
	}
	const reportOption = allReportOptions[i];
	// Skip if we aren't including multiple participant reports and this report has multiple participants
	if (!includeMultipleParticipantReports && !reportOption.login) {
	continue;
	}
	// Check the report to see if it has a single participant and if the participant is already selected
	if (reportOption.login && _.some(loginOptionsToExclude, option => option.login === reportOption.login)) {
	continue;
	}
	// Finally check to see if this option is a match for the provided search string if we have one
	const {searchText, participantsList, isDefaultChatRoom} = reportOption;
	const participantNames = getParticipantNames(participantsList);
	if (searchValue && !isSearchStringMatch(searchValue, searchText, participantNames, isDefaultChatRoom)) {
	continue;
	}
	// If the report is pinned and we are using the option to display pinned reports on top then we need to
	// collect the pinned reports so we can sort them alphabetically once they are collected
	if (prioritizePinnedReports && reportOption.isPinned) {
	pinnedReportOptions.push(reportOption);
	} else if (prioritizeIOUDebts && reportOption.hasOutstandingIOU && !reportOption.isIOUReportOwner) {
	iouDebtReportOptions.push(reportOption);
	} else {
	recentReportOptions.push(reportOption);
	}
	// Add this login to the exclude list so it won't appear when we process the personal details
	if (reportOption.login) {
	loginOptionsToExclude.push({login: reportOption.login});
	}
	}
	}

I can see that we have to choose one option to prioritize: iou debt or pinned, and now we are adding with draft comment. The way it is implemented now, looks like the prioritized is exclusive, if I prioritize by pinned, then 'iou debt' won't be prioritized.

How do we want the reports with draft comment to appear? if it is prioritized by 'iou debt', then the chat with draft comments should be right below the 'iou debt'? or on top?

Wouldn't it be better to be able to have also a way of keeping things in a prioritization like:

1. pinned
2. iou debt
3. drafts
4. all the rest

And then the user, in the settings, can check or uncheck if they want to have categories like iou debt prioritized or not. If a user uncheck iou debt as a priority, then these reports get mixed git 'all the rest', leaving the reports in the following order:

1. pinned
2. drafts
3. all the rest

[MelvinBot]

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

[aldo-expensify]

In my opinion, this can be performed by an external because it seems to be only UI stuff

[aldo-expensify]

Forgot to un-assign myself

[JmillsExpensify]

Upwork job is here: https://www.upwork.com/jobs/~01b019518a683d87cf. Please post your solution in this issue and someone from our team will approve before we move forward. Thank you!

[MelvinBot]

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

[iwiznia]

@aldo-expensify but I think the prioritization really happens here and it can indeed prioritize many things.

[meetmangukiya]

Yes, I think the current order is Pinned > IOUs > everything else if respective prioritizing is set.

Proposal

1. Add a new argument prioritizeReportsWithDraftComments
   

   App/src/libs/OptionsListUtils.js

   Lines 321 to 343 in 0f446f0

   	function getOptions(reports, personalDetails, draftComments, activeReportID, {
   	betas = [],
   	selectedOptions = [],
   	maxRecentReportsToShow = 0,
   	excludeConcierge = false,
   	excludeChronos = false,
   	excludeReceipts = false,
   	excludeDefaultRooms = false,
   	includeMultipleParticipantReports = false,
   	includePersonalDetails = false,
   	includeRecentReports = false,
   	prioritizePinnedReports = false,
   	prioritizeDefaultRoomsInSearch = false,
   	sortByLastMessageTimestamp = false,
   	searchValue = '',
   	showChatPreviewLine = false,
   	showReportsWithNoComments = false,
   	showReportsWithDrafts = false,
   	hideReadReports = false,
   	sortByAlphaAsc = false,
   	forcePolicyNamePreview = false,
   	prioritizeIOUDebts = false,
   	}) {

2. Add a new array of reports with draft comments here draftReportOptions by filtering with hasDraftComment
   

   App/src/libs/OptionsListUtils.js

   Lines 455 to 461 in 0f446f0

   	if (prioritizePinnedReports && reportOption.isPinned) {
   	pinnedReportOptions.push(reportOption);
   	} else if (prioritizeIOUDebts && reportOption.hasOutstandingIOU && !reportOption.isIOUReportOwner) {
   	iouDebtReportOptions.push(reportOption);
   	} else {
   	recentReportOptions.push(reportOption);
   	}

3. Conditionally concat that array after IOUs, Pinned so the order becomes Pinned > IOUs > Drafts > everything else(or whatever is the new priority we want to set).
   

   App/src/libs/OptionsListUtils.js

   Lines 470 to 486 in 0f446f0

   	// If we are prioritizing IOUs the user owes, add them before the normal recent report options
   	if (prioritizeIOUDebts) {
   	const sortedIOUReports = lodashOrderBy(iouDebtReportOptions, ['iouReportAmount'], ['desc']);
   	recentReportOptions = sortedIOUReports.concat(recentReportOptions);
   	}
   	// If we are prioritizing our pinned reports then shift them to the front and sort them by report name
   	if (prioritizePinnedReports) {
   	const sortedPinnedReports = lodashOrderBy(pinnedReportOptions, ['text'], ['asc']);
   	recentReportOptions = sortedPinnedReports.concat(recentReportOptions);
   	}
   	// If we are prioritizing default rooms in search, do it only once we started something
   	if (prioritizeDefaultRoomsInSearch && searchValue !== '') {
   	const reportsSplitByDefaultChatRoom = _.partition(recentReportOptions, option => option.isDefaultChatRoom);
   	recentReportOptions = reportsSplitByDefaultChatRoom[0].concat(reportsSplitByDefaultChatRoom[1]);
   	}

[iwiznia]

Ah, yes, that looks better. I am missing:

• What will the default of the new param be? I assume false
• From what call are we going to pass it as true?

[meetmangukiya]

1. I think that needs to be discussed and decided
2. I believe we will pass the param from getSidebarOptions calling getOptions.
   

   App/src/libs/OptionsListUtils.js

   Lines 707 to 715 in 0f446f0

   	return getOptions(reports, personalDetails, draftComments, activeReportID, {
   	betas,
   	includeRecentReports: true,
   	includeMultipleParticipantReports: true,
   	maxRecentReportsToShow: 0, // Unlimited
   	sortByLastMessageTimestamp: true,
   	showChatPreviewLine: true,
   	...sideBarOptions,
   	});

   
   I am not sure what GSD stands for, but we could conditionally change that with that priorityMode or hardcode the priority value
   

   App/src/libs/OptionsListUtils.js

   Lines 695 to 705 in 0f446f0

   	let sideBarOptions = {
   	prioritizePinnedReports: true,
   	prioritizeIOUDebts: true,
   	};
   	if (priorityMode === CONST.PRIORITY_MODE.GSD) {
   	sideBarOptions = {
   	hideReadReports: true,
   	sortByAlphaAsc: true,
   	showReportsWithDrafts: true,
   	};
   	}

[iwiznia]

I think that needs to be discussed and decided

Let's do that then 😄 I think since the only place we want to prioritize the drafts is the LHN, the default should be false.

I am not sure what GSD stands for, but we could conditionally change that with that priorityMode or hardcode the priority value

That's the Get Shit Done mode (renamed to #focus mode in the UI), which you can toggle from the preferences. I think that the proper behavior for GSD mode is to also show any chat with a draft.

[aldo-expensify]

@aldo-expensify but I think the prioritization really happens here and it can indeed prioritize many things.

It looks like it will prioritize all those there, but only one of those list will contain items at the same time because of the if..else if...else in here

Scratch that.. you are right

[mountiny]

@meetmangukiya Gert job on the PR. We have already merged it 🎉

Since this is your first PR for Expensify/App (I hope first of many :), I just want to double check, if you have submitted a proposal to this job posting in Upwork? The job posting is here.

[meetmangukiya]

Yes, I have @mountiny

[mountiny]

Perfect! You should be hired today by Jason (he is in US, West Coast).

[mallenexpensify]

Hired @meetmangukiya in Upwork, assigned issue to me as Contributor Manager

[botify]

🚀 Deployed to staging by @iwiznia in version: 1.0.88-3 🚀

platform	result
🤖 android 🤖	cancelled 🔪
🖥 desktop 🖥	cancelled 🔪
🍎 iOS 🍎	cancelled 🔪
🕸 web 🕸	success ✅

[botify]

🚀 Deployed to production by @roryabraham in version: 1.0.90-2 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

[mallenexpensify]

@meetmangukiya, on Desktop Version 1.0.92-0, this is what I'm experiencing.

• Navigate to new chat that's not currently at the top of Left Hand Nav (LHN)
• Start typing a message
• Once paused for 1-2 seconds, the new draft chat populates at the top of LHN.

I'm assuming this is performing 'as expected', can you confirm?

Personally... I find it to be a bit distracting that, when I'm typing a message, I see a 'flash' in the LHN and new chat loaded at top, I often/always look over at the chat thinking I have a new one coming it, but it's just my current draft that I'm actively working on. Are there other options you considered or that you know are available for how to manage this?

Can we do something like:

1. When someone is composing a text and there are characters in the compose box, 'do nothing'.
2. If a users leaves a chat and there are characters still in the box (aka a draft message), then move it up to the top of LHN. That way it wouldn't 'flash' and distract a user when they're in the middle of composing a message. I don't think there's much value in seeing a draft message while you're composing, only once you're navigated away from the chat .

What do you think @meetmangukiya ?
@aldo-expensify , @iwiznia, @mountiny 👀 too please, in case I'm missing something on the engineering/code side

[MelvinBot]

@iwiznia, @meetmangukiya, @mallenexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[mallenexpensify]

Waiting for feedback from my previous comment

[mountiny]

@mallenexpensify Yes, that is expected behaviour I would say.

Honestly, I am not totally sure what would be the best way to approach this. Let's say we would reorder the LHN on choosing some other chat. If it would be chat which is located above the draft chat, we would see shift at time of clicking to the new message (because the one we clicked on would be shifted one below).

I think this would be the best to ask in #expensify-open-source to get some thoughts and ideally ideas on how this is treated in other apps.

I have tried Facebook Messenger and Whatsapp desktop apps and they dont even do drafts. In Signal desktop App, it is the way as I described above, they reorder the LHN once user clicks on other chat causing it shift at that moment. Maybe that is a good compromise.

[meetmangukiya]

If we were to to want to implement this I suppose we could add a shouldComponentUpdate to SidebarLinks that will do some smart diffing in previous and new props to see if any new drafts, and if they are for current report, etc. and respectively return.

[mallenexpensify]

Posted in Slack, let's see what others think
https://expensify.slack.com/archives/C01GTK53T8Q/p1631062473055500

[mallenexpensify]

@mountiny and I agree we should update this to:

1. When someone is composing a text and there are characters in the compose box, 'do nothing'.
2. If a users leaves a chat and there are characters still in the box (aka a draft message), then move it up to the top of LHN. That way it wouldn't 'flash' and distract a user when they're in the middle of composing a message. I don't think there's much value in seeing a draft message while you're composing, only once

@meetmangukiya are you able to do that? If so, do you want to do as part of this GH issue or would a new GH issue be better? Either way, I think you should be compensated for the update since it wasn't clearly stated that was what we originally were asking for and it will require additional work.

Thanks @kidroca for the :yes: on the Slack post too.

[meetmangukiya]

@mallenexpensify yes, I can do it. I am fine with either continuing this issue or creating a new one. Is it possible for the part 1(?) payment to be made while we implement, review and test the update?

[mallenexpensify]

@meetmangukiya

1. Do you want to work on the update?
2. If so, do you have a preference between working off this existing issue or creating a brand new one? The pay will be the same, $250.

@vitHoracek it's easy to add a bonus to a job and/or create a new job in Upwork

[mallenexpensify]

Paid @meetmangukiya for the original job in Upwork. Job is still open in Upwork til we decide next best step

[mallenexpensify]

ooooof, completely missed this comment above

@mallenexpensify yes, I can do it. I am fine with either continuing this issue or creating a new one. Is it possible for the part 1(?) payment to be made while we implement, review and test the update?

I created a new issue (which is almost always 'best practice') #5166
It will go through our normal process, I added a note that you have 'first dibs' to fix the issue @meetmangukiya so other contributors shouldn't propose fixes.
Closing this now and also the upwork job (there will be a new upwork job for the other with separate payment)

[mountiny]

Thank you for handling and clarifying the process @mallenexpensify 🙌

[meetmangukiya]

Thank you!