Skip to content

Commit 5b43009

Browse files
MelvinBotmkhutornyi
andcommitted
Simplify code comments for readability
Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
1 parent 8d837f7 commit 5b43009

2 files changed

Lines changed: 29 additions & 46 deletions

File tree

src/components/Search/SearchFiltersParticipantsSelector.tsx

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,7 @@ import ROUTES from '@src/ROUTES';
2020
import type {Attendee} from '@src/types/onyx/IOU';
2121
import SearchFilterPageFooterButtons from './SearchFilterPageFooterButtons';
2222

23-
/**
24-
* Creates an OptionData object from a name-only attendee (attendee without a real accountID in personalDetails)
25-
*/
23+
// Builds an OptionData row for a name-only attendee — one without a real accountID in personalDetails.
2624
function getOptionDataFromAttendee(attendee: Attendee): OptionData {
2725
return {
2826
text: attendee.displayName,
@@ -67,7 +65,7 @@ function SearchFiltersParticipantsSelector({initialAccountIDs, onFiltersUpdate,
6765
const currentUserEmail = currentUserPersonalDetails.email ?? '';
6866
const [recentAttendees] = useOnyx(ONYXKEYS.NVP_RECENT_ATTENDEES);
6967

70-
// Transform raw recentAttendees into Option[] format for use with getValidOptions (only for attendee filter)
68+
// Only the attendee filter feeds recentAttendees into the picker; other filters use empty list.
7169
const recentAttendeeLists = shouldAllowNameOnlyOptions ? getFilteredRecentAttendees(personalDetails, [], recentAttendees ?? [], currentUserEmail, currentUserAccountID) : [];
7270

7371
const {searchTerm, debouncedSearchTerm, setSearchTerm, availableOptions, selectedOptions, setSelectedOptions, toggleSelection, areOptionsInitialized, onListEndReached} =
@@ -85,8 +83,8 @@ function SearchFiltersParticipantsSelector({initialAccountIDs, onFiltersUpdate,
8583
shouldKeepSelectedInAvailableOptions: true,
8684
});
8785

88-
// Set once the hydration effect runs so the snapshot waits for it. Without this, a fully stale
89-
// initialAccountIDs list would let the snapshot fire on the first toggled row and pin it.
86+
// Flip to true once the hydration effect runs. Without this, a stale `initialAccountIDs` could let
87+
// the pinning snapshot fire on the first toggled row and pin it by mistake.
9088
const [hasAttemptedHydration, setHasAttemptedHydration] = useState(initialAccountIDs.length === 0);
9189

9290
const trimmedSearchTerm = debouncedSearchTerm.trim().toLowerCase();
@@ -95,7 +93,7 @@ function SearchFiltersParticipantsSelector({initialAccountIDs, onFiltersUpdate,
9593
const currentUserOption = areOptionsInitialized ? availableOptions.currentUserOption : null;
9694
const isCurrentUserSelected = !!currentUserAccountID && selectedOptions.some((option) => option.accountID === currentUserAccountID);
9795

98-
// Drop the current user from Recents / Contacts so they only show in their dedicated section (or get pinned at top by the hook).
96+
// Hide the current user from Recents / Contacts they get their own row (or get pinned at the top).
9997
const recentReportsWithoutCurrentUser =
10098
areOptionsInitialized && currentUserOption?.accountID
10199
? availableOptions.recentReports.filter((report) => report.accountID !== currentUserOption.accountID)
@@ -105,8 +103,8 @@ function SearchFiltersParticipantsSelector({initialAccountIDs, onFiltersUpdate,
105103
? availableOptions.personalDetails.filter((detail) => detail.accountID !== currentUserOption.accountID)
106104
: (availableOptions.personalDetails ?? []);
107105

108-
// Selected items that don't show up in Recents / Contacts and aren't the current user — surface them but respect the search term.
109-
// Dedupe by accountID for real users and by login for name-only attendees (which share DEFAULT_NUMBER_ID).
106+
// Selected items not visible in Recents / Contacts and not the current user — show them in a section above, but only if they match the search term.
107+
// Dedupe by accountID for real users and by login for name-only attendees (which all share DEFAULT_NUMBER_ID).
110108
const visibleAccountIDs = new Set<number>(
111109
[...personalDetailsWithoutCurrentUser, ...recentReportsWithoutCurrentUser].map((option) => option.accountID).filter((id): id is number => !!id && id !== CONST.DEFAULT_NUMBER_ID),
112110
);
@@ -119,7 +117,7 @@ function SearchFiltersParticipantsSelector({initialAccountIDs, onFiltersUpdate,
119117
matchesSearchTerm(option),
120118
);
121119

122-
// Render-ready current user row with the "(you)" suffix; falls back to personalDetails if pagination dropped them.
120+
// Current user row with the "(you)" suffix. Falls back to personalDetails when pagination drops them from availableOptions.
123121
let currentUserRow: OptionData | undefined;
124122
if (areOptionsInitialized) {
125123
let candidate = currentUserOption ?? undefined;
@@ -152,9 +150,9 @@ function SearchFiltersParticipantsSelector({initialAccountIDs, onFiltersUpdate,
152150
baseSections.push({title: '', data: personalDetailsWithoutCurrentUser, sectionIndex: 4});
153151
}
154152

155-
// `initialAccountIDs` holds accountIDs (or, for the attendee filter, accountID || displayName ||
156-
// login for name-only entries). The default `keyForList` match doesn't fit: for any contact with
157-
// a 1:1 DM, `keyForList` is the reportID, so accountID-based matching needs an explicit `getKey`.
153+
// `initialAccountIDs` holds accountIDs (or displayName / login for name-only attendees), but for any
154+
// contact with a 1:1 DM, the default `keyForList` is the reportID. Use an explicit getKey so the hook
155+
// matches on the right identifier.
158156
const getKey = (option: OptionData) => {
159157
if (shouldAllowNameOnlyOptions) {
160158
if (option.accountID && option.accountID !== CONST.DEFAULT_NUMBER_ID && personalDetails?.[option.accountID]) {
@@ -166,12 +164,11 @@ function SearchFiltersParticipantsSelector({initialAccountIDs, onFiltersUpdate,
166164
return option.accountID ? option.accountID.toString() : undefined;
167165
};
168166

169-
// Pinned rows may not be in current sections when list is lazy-loaded.
170-
// The hook keeps them from the snapshot; we filter them through `matchesSearchTerm` so the pinned
171-
// section respects the current search term.
167+
// The list is lazy-loaded, so pinned rows aren't always present in baseSections — the hook keeps them
168+
// from the snapshot. Pass `matchesSearchTerm` so the pinned section still respects the search term.
172169
const sections = useFrozenPreSelection<OptionData>(baseSections, {
173170
initialSelectedValues: initialAccountIDs,
174-
// Wait for hydration so a toggled row isn't mistaken for pre-selection.
171+
// Wait for hydration so a toggled row isn't mistaken for a pre-selection.
175172
canCapture: areOptionsInitialized && hasAttemptedHydration,
176173
shouldRenderPinned: matchesSearchTerm,
177174
getKey,
@@ -190,12 +187,10 @@ function SearchFiltersParticipantsSelector({initialAccountIDs, onFiltersUpdate,
190187
if (shouldAllowNameOnlyOptions) {
191188
selectedIdentifiers = selectedOptions
192189
.map((option) => {
193-
// For real users (with valid accountID in personalDetails), use accountID
190+
// Real users accountID; name-only attendees → displayName or login.
194191
if (option.accountID && option.accountID !== CONST.DEFAULT_NUMBER_ID && personalDetails?.[option.accountID]) {
195192
return option.accountID.toString();
196193
}
197-
198-
// For name-only attendees, use displayName or login as identifier
199194
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing -- need || to handle empty string
200195
return option.displayName || option.login;
201196
})
@@ -208,7 +203,7 @@ function SearchFiltersParticipantsSelector({initialAccountIDs, onFiltersUpdate,
208203
Navigation.goBack(ROUTES.SEARCH_ADVANCED_FILTERS.getRoute());
209204
};
210205

211-
// This effect handles setting initial selectedOptions based on accountIDs (or displayNames for attendee filter)
206+
// Hydrate selectedOptions from initialAccountIDs (or displayNames for the attendee filter).
212207
useEffect(() => {
213208
if (!initialAccountIDs || initialAccountIDs.length === 0 || !personalDetails) {
214209
return;
@@ -219,7 +214,7 @@ function SearchFiltersParticipantsSelector({initialAccountIDs, onFiltersUpdate,
219214
if (shouldAllowNameOnlyOptions) {
220215
preSelectedOptions = initialAccountIDs
221216
.map((identifier) => {
222-
// First, try to look up as accountID in personalDetails
217+
// Look up the identifier as an accountID first.
223218
const participant = personalDetails[identifier];
224219
if (participant) {
225220
const optionData = {
@@ -229,15 +224,14 @@ function SearchFiltersParticipantsSelector({initialAccountIDs, onFiltersUpdate,
229224
return optionData as OptionData;
230225
}
231226

232-
// If not found in personalDetails, this might be a name-only attendee
233-
// Search in recentAttendees by displayName or email
227+
// Fall back to a name-only attendee match (by displayName or email).
234228
const attendee = recentAttendees?.find((recentAttendee) => recentAttendee.displayName === identifier || recentAttendee.email === identifier);
235229
if (attendee) {
236230
return getOptionDataFromAttendee(attendee);
237231
}
238232

239-
// Fallback: construct a minimal option from the identifier string to preserve
240-
// name-only filters across sessions (e.g., after cache clear or on another device)
233+
// Last resort: build a minimal option from the identifier so name-only filters survive
234+
// a cache clear or a switch to another device.
241235
return {
242236
text: identifier,
243237
alternateText: identifier,
@@ -269,7 +263,7 @@ function SearchFiltersParticipantsSelector({initialAccountIDs, onFiltersUpdate,
269263
}
270264

271265
setSelectedOptions(preSelectedOptions);
272-
// eslint-disable-next-line react-hooks/set-state-in-effect -- one-shot flag so the snapshot waits for hydration; derivable state isn't enough since the effect can resolve to an empty array.
266+
// eslint-disable-next-line react-hooks/set-state-in-effect -- one-shot flag the pinning snapshot waits on; derivable state doesn't work because hydration can resolve to an empty array.
273267
setHasAttemptedHydration(true);
274268
// eslint-disable-next-line react-hooks/exhaustive-deps -- this should react only to changes in form data
275269
}, [initialAccountIDs, personalDetails, recentAttendees, shouldAllowNameOnlyOptions]);

src/hooks/useFrozenPreSelection.ts

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,40 +4,29 @@ import type {Section} from '@components/SelectionList/SelectionListWithSections/
44
import CONST from '@src/CONST';
55

66
type UseFrozenPreSelectionOptions<TItem extends ListItem> = {
7-
/** Identifiers (matched against the result of `getKey`, or `item.keyForList` when `getKey` is omitted) of items that should be pinned at the top on first capture. */
7+
/** Item identifiers to pin on first capture. Matched against `getKey(item)`, or `item.keyForList` if `getKey` is omitted. */
88
initialSelectedValues: string[];
99

10-
/** Whether the input sections are ready to be inspected. Capture happens on the first render where this becomes true. */
10+
/** Set to `true` once the sections are ready to inspect — capture fires on the next render. */
1111
canCapture: boolean;
1212

13-
/**
14-
* Optional predicate to keep a captured item in the pinned section. Required for lazy-loaded
15-
* lists where pinned rows must outlive the current section data (e.g. to apply the search filter
16-
* to pinned rows that aren't otherwise present in the visible sections).
17-
*/
13+
/** Optional filter for pinned rows (e.g. to honor a search term). Needed when pinned rows may not appear in the input `sections`. */
1814
shouldRenderPinned?: (item: TItem) => boolean;
1915

20-
/**
21-
* Optional extractor for the identity used to match items against `initialSelectedValues`.
22-
* Defaults to `item.keyForList`. Use this when the caller's identifier scheme doesn't match
23-
* `keyForList` — e.g. participants are passed in by accountID but their `keyForList` is the
24-
* 1:1 DM's reportID.
25-
*/
16+
/** Optional identity extractor. Defaults to `item.keyForList`. Override when the caller's identifier doesn't match `keyForList`. */
2617
getKey?: (item: TItem) => string | undefined;
2718
};
2819

2920
/**
30-
* Pins pre-selected rows to a new top section on first ready render, then locks the order.
31-
* Items are matched against `initialSelectedValues` using `getKey` (or `keyForList` when `getKey`
32-
* is omitted) and removed from their original sections; toggling a pinned row updates `isSelected`
33-
* in place without moving it. Returns `sections` unchanged while `canCapture` is false or when the
34-
* combined item count is below `STANDARD_LIST_ITEM_LIMIT`.
21+
* Pins pre-selected rows to a top section on first ready render, then locks them in place.
22+
* Toggling a pinned row updates `isSelected` without moving it.
23+
* Returns `sections` unchanged while not ready, or when the list is shorter than `STANDARD_LIST_ITEM_LIMIT`.
3524
*/
3625
function useFrozenPreSelection<TItem extends ListItem>(sections: Array<Section<TItem>>, options: UseFrozenPreSelectionOptions<TItem>): Array<Section<TItem>> {
3726
const {initialSelectedValues, canCapture, shouldRenderPinned, getKey} = options;
3827
const resolveKey = (item: TItem) => (getKey ? getKey(item) : item.keyForList);
3928

40-
// null = not captured yet; empty Map = captured but list was too short to pin.
29+
// null = not captured yet; empty Map = captured but the list was too short to pin.
4130
const [frozenData, setFrozenData] = useState<Map<string, TItem> | null>(null);
4231

4332
if (frozenData === null && canCapture) {

0 commit comments

Comments
 (0)