Skip to content

Commit 0de1c55

Browse files
authored
[♻️][Select]: refactor to improve search mechanic and hidden input rendering (#102)
* Remove get options utility usage * Improve use-combobox-base hook * Update version * Update tests
1 parent 8a80d2c commit 0de1c55

File tree

8 files changed

+84
-160
lines changed

8 files changed

+84
-160
lines changed

lib/Select/Select.test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1005,7 +1005,7 @@ describe("Select", () => {
10051005
});
10061006

10071007
expect(handleSubmit.mock.calls.length).toBe(2);
1008-
expect(getFormData().getAll("n")).toEqual(["1"]);
1008+
expect(getFormData().getAll("n")).toEqual(["0", "1"]);
10091009

10101010
rerender1(
10111011
<form
@@ -1164,7 +1164,7 @@ describe("Select", () => {
11641164
});
11651165

11661166
expect(handleSubmit.mock.calls.length).toBe(2);
1167-
expect(getFormData().get("n")).toBe(null);
1167+
expect(getFormData().get("n")).toBe("0");
11681168

11691169
rerender2(
11701170
<form

lib/Select/Select.tsx

Lines changed: 9 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,7 @@ import {
1818
} from "../utils";
1919
import { SelectContext, type SelectContextValue } from "./context";
2020
import { Root as RootSlot } from "./slots";
21-
import {
22-
getOptions as getOptionsUtil,
23-
noValueSelected,
24-
normalizeValues,
25-
useElementsRegistry,
26-
} from "./utils";
21+
import { noValueSelected, normalizeValues, useElementsRegistry } from "./utils";
2722

2823
export type RenderProps = {
2924
/**
@@ -415,8 +410,6 @@ const SelectBase = (props: Props, ref: React.Ref<HTMLDivElement>) => {
415410
closeList();
416411
}
417412

418-
const getOptions = () => getOptionsUtil(React.Children.toArray(children));
419-
420413
const context: SelectContextValue = {
421414
readOnly,
422415
disabled,
@@ -433,7 +426,6 @@ const SelectBase = (props: Props, ref: React.Ref<HTMLDivElement>) => {
433426
closeListAndMaintainFocus,
434427
setFilteredEntities,
435428
setActiveDescendant,
436-
getOptions,
437429
openList,
438430
closeList,
439431
toggleList,
@@ -491,29 +483,14 @@ const SelectBase = (props: Props, ref: React.Ref<HTMLDivElement>) => {
491483
if (selectedValues.length === 0) return null;
492484

493485
const renderOptions = () => {
494-
const disabledOptions = getOptions().filter(o => o.disabled);
495-
496-
const isOptionDisabled = (optionValue: string) =>
497-
disabledOptions.some(o => o.value === optionValue);
498-
499-
if (!multiple) {
500-
const optionValue = selectedValues as string;
501-
502-
if (isOptionDisabled(optionValue)) return null;
503-
504-
return <option value={optionValue} />;
505-
}
506-
507-
return (selectedValues as string[]).map(value => {
508-
if (isOptionDisabled(value)) return null;
509-
510-
return (
511-
<option
512-
key={value}
513-
value={value}
514-
/>
515-
);
516-
});
486+
if (!multiple) return <option value={selectedValues as string} />;
487+
488+
return (selectedValues as string[]).map(value => (
489+
<option
490+
key={value}
491+
value={value}
492+
/>
493+
));
517494
};
518495

519496
return (

lib/Select/components/Controller/Controller.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ const ControllerBase = (props: Props, ref: React.Ref<HTMLInputElement>) => {
8383
searchable: ctx?.searchable ?? false,
8484
onInputChange: onChange,
8585
onKeyDown,
86-
getOptionsInfo: ctx?.getOptions ?? (() => []),
8786
getOptionElements: () => {
8887
const listId = ctx?.elementsRegistry.getElementId("list");
8988
const listNode = document.getElementById(listId ?? "");

lib/Select/components/Controller/utils.ts

Lines changed: 45 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ type Props<T extends HTMLElement> = {
2828
onBackspaceKeyDown?: React.KeyboardEventHandler<T>;
2929
onInputChange?: React.ChangeEventHandler<HTMLInputElement>;
3030
getOptionElements: () => HTMLElement[];
31-
getOptionsInfo: SelectContextValue["getOptions"];
3231
onFilteredEntities: (
3332
entities: SelectContextValue["filteredEntities"],
3433
) => void;
@@ -49,7 +48,6 @@ export const useComboboxBase = <T extends HTMLElement>(props: Props<T>) => {
4948
onEscapeKeyDown,
5049
onBackspaceKeyDown,
5150
getOptionElements,
52-
getOptionsInfo,
5351
onActiveDescendantChange,
5452
onListOpenChange,
5553
onFilteredEntities,
@@ -70,15 +68,11 @@ export const useComboboxBase = <T extends HTMLElement>(props: Props<T>) => {
7068
ref: focusVisibleRef,
7169
} = useIsFocusVisible<T>();
7270

73-
const jumpToChar = useJumpToChar({
74-
activeDescendantElement: activeDescendant,
75-
getListItems: getOptionElements,
76-
onActiveDescendantElementChange: onActiveDescendantChange,
77-
});
78-
7971
const ref = React.useRef<T>();
8072
const handleRef = useForkedRefs(ref, focusVisibleRef);
8173

74+
const cachedOptionElementsRef = React.useRef<HTMLElement[]>([]);
75+
8276
const isSelectOnly = !searchable;
8377

8478
const [isFocusedVisible, setIsFocusedVisible] = React.useState(() =>
@@ -97,12 +91,33 @@ export const useComboboxBase = <T extends HTMLElement>(props: Props<T>) => {
9791
ref.current?.focus();
9892
}, []);
9993

94+
const getCachedItems = () => {
95+
if (cachedOptionElementsRef.current.length === 0) {
96+
cachedOptionElementsRef.current = getOptionElements();
97+
}
98+
99+
return cachedOptionElementsRef.current;
100+
};
101+
102+
const jumpToChar = useJumpToChar({
103+
activeDescendantElement: activeDescendant,
104+
getListItems: getOptionElements,
105+
onActiveDescendantElementChange: onActiveDescendantChange,
106+
});
107+
100108
useOnChange(listOpenState, currentOpenState => {
101109
if (disabled || readOnly) return;
102-
if (currentOpenState) return;
103110
if (!(ref.current instanceof HTMLInputElement)) return;
104111

112+
if (currentOpenState) {
113+
cachedOptionElementsRef.current = getOptionElements();
114+
115+
return;
116+
}
117+
105118
ref.current.value = "";
119+
cachedOptionElementsRef.current = [];
120+
106121
onFilteredEntities(null);
107122
});
108123

@@ -169,7 +184,8 @@ export const useComboboxBase = <T extends HTMLElement>(props: Props<T>) => {
169184
if (
170185
item.getAttribute("aria-disabled") === "true" ||
171186
item.hasAttribute("data-hidden") ||
172-
item.getAttribute("aria-hidden") === "true"
187+
item.getAttribute("aria-hidden") === "true" ||
188+
item.hasAttribute("data-hidden")
173189
) {
174190
const newIdx =
175191
(forward ? idx + 1 : idx - 1 + items.length) % items.length;
@@ -184,9 +200,20 @@ export const useComboboxBase = <T extends HTMLElement>(props: Props<T>) => {
184200
items: (HTMLElement | null)[],
185201
forward: boolean,
186202
) => {
187-
const selectedItems = items.filter(item =>
188-
item?.hasAttribute("data-selected"),
189-
);
203+
const selectedItems = items.filter(item => {
204+
if (!item) return false;
205+
206+
if (
207+
item.getAttribute("aria-disabled") === "true" ||
208+
item.hasAttribute("data-hidden") ||
209+
item.getAttribute("aria-hidden") === "true" ||
210+
item.hasAttribute("data-hidden")
211+
) {
212+
return false;
213+
}
214+
215+
return item.hasAttribute("data-selected");
216+
});
190217

191218
return getAvailableItem(
192219
selectedItems.length > 0 ? selectedItems : items,
@@ -365,15 +392,15 @@ export const useComboboxBase = <T extends HTMLElement>(props: Props<T>) => {
365392

366393
const query = target.value;
367394

368-
const options = getOptionsInfo();
395+
const items = getCachedItems();
369396

370-
const entities = options
371-
.filter(option => {
372-
const text = option.valueLabel.toLowerCase();
397+
const entities = items
398+
.filter(item => {
399+
const text = item.textContent?.trim().toLowerCase() ?? "";
373400

374401
return text.includes(query.toLowerCase());
375402
})
376-
.map(option => option.value);
403+
.map(item => item.getAttribute("data-entity") ?? "");
377404

378405
onFilteredEntities(entities);
379406
onInputChange?.(event);

lib/Select/components/Group.tsx

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,13 @@ import {
55
resolvePropWithRenderContext,
66
} from "../../internals";
77
import type { MergeElementProps, PropWithRenderContext } from "../../types";
8-
import { componentWithForwardedRef, useDeterministicId } from "../../utils";
8+
import {
9+
componentWithForwardedRef,
10+
useDeterministicId,
11+
useIsServerHandoffComplete,
12+
} from "../../utils";
913
import { SelectContext } from "../context";
1014
import { GroupRoot as GroupRootSlot } from "../slots";
11-
import { getOptions } from "../utils";
1215

1316
export type RenderProps = {
1417
/**
@@ -65,6 +68,8 @@ const GroupBase = (props: Props, ref: React.Ref<HTMLDivElement>) => {
6568

6669
const id = useDeterministicId(idProp, "styleless-ui__select__group");
6770

71+
const isServerHandoffComplete = useIsServerHandoffComplete();
72+
6873
const labelInfo = getLabelInfo(label, "Select.Group", {
6974
customErrorMessage: [
7075
"Invalid `label` property.",
@@ -75,30 +80,32 @@ const GroupBase = (props: Props, ref: React.Ref<HTMLDivElement>) => {
7580

7681
const ctx = React.useContext(SelectContext);
7782

83+
const getOptionElements = () => {
84+
const group = document.getElementById(id);
85+
86+
if (!group) return [];
87+
88+
return Array.from(group.querySelectorAll<HTMLElement>(`[role='option']`));
89+
};
90+
7891
const isHidden = React.useMemo(() => {
79-
let hidden = false;
92+
if (!isServerHandoffComplete) return false;
8093

8194
const filtered = ctx?.filteredEntities;
8295

83-
if (filtered != null) {
84-
if (filtered.length === 0) hidden = true;
85-
else {
86-
const options = getOptions(
87-
React.Children.toArray(
88-
resolvePropWithRenderContext(childrenProp, { hidden: false }),
89-
),
90-
true,
91-
);
92-
93-
hidden = options.every(
94-
option => !filtered.some(value => value === option.value),
95-
);
96-
}
97-
}
96+
if (filtered == null) return false;
97+
if (filtered.length === 0) return true;
98+
99+
const optionElements: HTMLElement[] = getOptionElements();
98100

99-
return hidden;
101+
return optionElements.every(
102+
optionElement =>
103+
!filtered.some(
104+
value => value === optionElement.getAttribute("data-entity"),
105+
),
106+
);
100107
// eslint-disable-next-line react-hooks/exhaustive-deps
101-
}, [ctx?.filteredEntities]);
108+
}, [ctx?.filteredEntities, isServerHandoffComplete]);
102109

103110
if (!ctx) {
104111
logger("You have to use this component as a descendant of <Select.Root>.", {

lib/Select/context.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
import * as React from "react";
22
import { type LabelInfo } from "../internals";
3-
import type { PickAsMandatory } from "../types";
43
import type { RegisteredElementsKeys } from "./Select";
5-
import type { OptionProps } from "./components";
64
import type { ElementsRegistry } from "./utils";
75

86
type ContextValue = {
@@ -21,9 +19,6 @@ type ContextValue = {
2119
closeListAndMaintainFocus: () => void;
2220
setActiveDescendant: React.Dispatch<React.SetStateAction<HTMLElement | null>>;
2321
setFilteredEntities: React.Dispatch<React.SetStateAction<null | string[]>>;
24-
getOptions: () => Array<
25-
PickAsMandatory<OptionProps, "disabled" | "value" | "valueLabel">
26-
>;
2722
openList: () => void;
2823
closeList: () => void;
2924
toggleList: () => void;

0 commit comments

Comments
 (0)