Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue with option disappearing when selected for new contacts #30017

Merged
merged 11 commits into from
Oct 19, 2023
Merged

Fix issue with option disappearing when selected for new contacts #30017

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c6d70dc
add fix
thienlnam Oct 19, 2023
36d67a9
Add comment to help clarify what is happening
thienlnam Oct 19, 2023
7148df6
Duplicate changes to New Group
thienlnam Oct 19, 2023
f13440e
Add new method for formatting sections
thienlnam Oct 19, 2023
ba0990f
Replace the money request participants with the method
thienlnam Oct 19, 2023
42afc18
Replace new chat page with this method
thienlnam Oct 19, 2023
688df15
prettier
thienlnam Oct 19, 2023
89888a7
Return more items
thienlnam Oct 19, 2023
fbc9070
fix the throw on newChatPage
thienlnam Oct 19, 2023
a3d3afb
Remove the unneeded function param
thienlnam Oct 19, 2023
39afd7b
Clean up the utility method
thienlnam Oct 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions src/libs/OptionsListUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1585,6 +1585,73 @@ function shouldOptionShowTooltip(option) {
return (!option.isChatRoom || option.isThread) && !option.isArchivedRoom;
}

/**
* Handles the logic for displaying selected participants from the search term
* @param {String} searchTerm
* @param {Array} sectionList
* @param {Array} selectedOptions
* @param {Array} filteredRecentReports
* @param {Array} filteredPersonalDetails
* @param {Object} personalDetails
* @param {Boolean} shouldGetOptionDetails
* @param {Number} indexOffset
* @returns {Number} newIndexOffset
*/
function formatSectionsFromSearchTerm(
searchTerm,
sectionList,
thienlnam marked this conversation as resolved.
Show resolved Hide resolved
selectedOptions,
filteredRecentReports,
filteredPersonalDetails,
personalDetails = {},
shouldGetOptionDetails = false,
indexOffset,
) {
// We show the selected participants at the top of the list when there is no search term
// However, if there is a search term we remove the selected participants from the top of the list unless they are part of the search results
// This clears up space on mobile views, where if you create a group with 4+ people you can't see the selected participants and the search results at the same time
let newIndexOffset = indexOffset;
if (searchTerm === '') {
sectionList.push({
title: undefined,
data: shouldGetOptionDetails
? _.map(selectedOptions, (participant) => {
const isPolicyExpenseChat = lodashGet(participant, 'isPolicyExpenseChat', false);
return isPolicyExpenseChat ? getPolicyExpenseReportOption(participant) : getParticipantsOption(participant, personalDetails);
})
: selectedOptions,
shouldShow: !_.isEmpty(selectedOptions),
indexOffset,
});
newIndexOffset += selectedOptions.length;
} else {
// If you select a new user you don't have a contact for, they won't get returned as part of a recent report or personal details
// This will add them to the list of options, deduping them if they already exist in the other lists
const selectedParticipantsWithoutDetails = _.filter(selectedOptions, (participant) => {
const accountID = lodashGet(participant, 'accountID', null);
const isPartOfSearchTerm = participant.searchText.toLowerCase().includes(searchTerm.trim().toLowerCase());
const isReportInRecentReports = _.some(filteredRecentReports, (report) => report.accountID === accountID);
const isReportInPersonalDetails = _.some(filteredPersonalDetails, (personalDetail) => personalDetail.accountID === accountID);
return isPartOfSearchTerm && !isReportInRecentReports && !isReportInPersonalDetails;
});

sectionList.push({
title: undefined,
data: shouldGetOptionDetails
? _.map(selectedParticipantsWithoutDetails, (participant) => {
const isPolicyExpenseChat = lodashGet(participant, 'isPolicyExpenseChat', false);
return isPolicyExpenseChat ? getPolicyExpenseReportOption(participant) : getParticipantsOption(participant, personalDetails);
})
: selectedParticipantsWithoutDetails,
shouldShow: !_.isEmpty(selectedParticipantsWithoutDetails),
indexOffset,
});
newIndexOffset += selectedParticipantsWithoutDetails.length;
}

return newIndexOffset;
}

export {
addSMSDomainIfPhoneNumber,
getAvatarsForAccountIDs,
Expand All @@ -1610,4 +1677,5 @@ export {
hasEnabledOptions,
getCategoryOptionTree,
formatMemberForList,
formatSectionsFromSearchTerm,
};
31 changes: 1 addition & 30 deletions src/pages/NewChatPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import {View} from 'react-native';
import PropTypes from 'prop-types';
import {withOnyx} from 'react-native-onyx';
import lodashGet from 'lodash/get';

Check failure on line 6 in src/pages/NewChatPage.js

View workflow job for this annotation

GitHub Actions / lint 

'lodashGet' is defined but never used
import OptionsSelector from '../components/OptionsSelector';
import * as OptionsListUtils from '../libs/OptionsListUtils';
import Permissions from '../libs/Permissions';
Expand Down Expand Up @@ -72,36 +72,7 @@
const sectionsList = [];
let indexOffset = 0;

// We show the selected participants at the top of the list when there is no search term
// However, if there is a search term we remove the selected participants from the top of the list unless they are part of the search results
// This clears up space on mobile views, where if you create a group with 4+ people you can't see the selected participants and the search results at the same time
if (searchTerm === '') {
sectionsList.push({
title: undefined,
data: selectedOptions,
shouldShow: !_.isEmpty(selectedOptions),
indexOffset,
});
indexOffset += selectedOptions.length;
} else {
// If you select a new user you don't have a contact for, they won't get returned as part of a recent report or personal details
// This will add them to the list of options, deduping them if they already exist in the other lists
const selectedParticipantsWithoutDetails = _.filter(selectedOptions, (participant) => {
const accountID = lodashGet(participant, 'accountID', null);
const isPartOfSearchTerm = participant.searchText.toLowerCase().includes(searchTerm.trim().toLowerCase());
const isReportInRecentReports = _.some(filteredRecentReports, (report) => report.accountID === accountID);
const isReportInPersonalDetails = _.some(filteredPersonalDetails, (personalDetail) => personalDetail.accountID === accountID);
return isPartOfSearchTerm && !isReportInRecentReports && !isReportInPersonalDetails;
});

sectionsList.push({
title: undefined,
data: selectedParticipantsWithoutDetails,
shouldShow: !_.isEmpty(selectedParticipantsWithoutDetails),
indexOffset,
});
indexOffset += selectedParticipantsWithoutDetails.length;
}
indexOffset = OptionsListUtils.formatSectionsFromSearchTerm(searchTerm, sectionsList, selectedOptions, filteredRecentReports, filteredPersonalDetails, {}, false, indexOffset);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of silently mutating the sectionsList, I think it would be better to return the list along with the index offset. Let me know what you think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I was thinking this might be odd just returning the index offset, I can update this


if (maxParticipantsReached) {
return sectionsList;
Expand Down
46 changes: 10 additions & 36 deletions src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsSelector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,42 +104,16 @@ function MoneyRequestParticipantsSelector({
const newSections = [];
let indexOffset = 0;

// We show the selected participants at the top of the list when there is no search term
// However, if there is a search term we remove the selected participants from the top of the list unless they are part of the search results
// This clears up space on mobile views, where if you split with 4+ people you can't see the selected participants and the search results at the same time
if (searchTerm === '') {
newSections.push({
title: undefined,
data: _.map(participants, (participant) => {
const isPolicyExpenseChat = lodashGet(participant, 'isPolicyExpenseChat', false);
return isPolicyExpenseChat ? OptionsListUtils.getPolicyExpenseReportOption(participant) : OptionsListUtils.getParticipantsOption(participant, personalDetails);
}),
shouldShow: true,
indexOffset,
});
indexOffset += participants.length;
} else {
// If you select a new user you don't have a contact for, they won't get returned as part of a recent report or personal details
// This will add them to the list of options, deduping them if they already exist in the other lists
const selectedParticipantsWithoutDetails = _.filter(participants, (participant) => {
const accountID = lodashGet(participant, 'accountID', null);
const isPartOfSearchTerm = participant.searchText.toLowerCase().includes(searchTerm.trim().toLowerCase());
const isReportInRecentReports = _.some(newChatOptions.recentReports, (report) => report.accountID === accountID);
const isReportInPersonalDetails = _.some(newChatOptions.personalDetails, (personalDetail) => personalDetail.accountID === accountID);
return isPartOfSearchTerm && !isReportInRecentReports && !isReportInPersonalDetails;
});

newSections.push({
title: undefined,
data: _.map(selectedParticipantsWithoutDetails, (participant) => {
const isPolicyExpenseChat = lodashGet(participant, 'isPolicyExpenseChat', false);
return isPolicyExpenseChat ? OptionsListUtils.getPolicyExpenseReportOption(participant) : OptionsListUtils.getParticipantsOption(participant, personalDetails);
}),
shouldShow: !_.isEmpty(selectedParticipantsWithoutDetails),
indexOffset,
});
indexOffset += selectedParticipantsWithoutDetails.length;
}
indexOffset = OptionsListUtils.formatSectionsFromSearchTerm(
searchTerm,
newSections,
participants,
newChatOptions.recentReports,
newChatOptions.personalDetails,
personalDetails,
true,
indexOffset,
);

if (maxParticipantsReached) {
return newSections;
Expand Down
Loading