Skip to content

Commit

Permalink
refactor: change selection type to string[] and adjust usages (init…
Browse files Browse the repository at this point in the history 
…ial)
  • Loading branch information
@TheAlmightyCrumb
TheAlmightyCrumb committed Dec 8, 2024
1 parent 4858365 commit 7e20c88
Show file tree
Hide file tree
Showing 13 changed files with 80 additions and 58 deletions.
10 changes: 5 additions & 5 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@
"packages/*"
],
"scripts": {
"clean": "rimraf -g ./packages/*/dist",
"build": "npm run build:boards && npm run build:typescript && npm run build:stylable",
"build:typescript": "tsc --build",
"build:stylable": "stc",
"build:boards": "node ./build-board-index.js",
"build:stylable": "stc",
"build:typescript": "tsc --build",
"clean": "rimraf -g ./packages/*/dist",
"lint": "eslint",
"pretest": "npm run lint && npm run build",
"prettify": "prettier . --write",
"test": "npm run test:spec",
"test:spec": "mocha-web \"packages/*/dist/test/**/*.spec.js\"",
"prettify": "prettier . --write"
"test:spec": "mocha-web \"packages/*/dist/test/**/*.spec.js\""
},
"devDependencies": {
"@playwright/browser-chromium": "^1.49.0",
Expand Down
18 changes: 11 additions & 7 deletions packages/components/src/auto-complete/auto-complete.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export type AutoComplete<T, EL extends HTMLElement = HTMLDivElement> = (props: A
export function AutoComplete<T, EL extends HTMLElement = HTMLDivElement>(props: AutoCompleteProps<T, EL>): JSX.Element {
const { searchControl, getTextContent, items, focusControl, selectionControl, getId, ...listProps } = props;
const [focused, setFocused] = useStateControls(focusControl, undefined);
const [selected, setSelected] = useStateControls(selectionControl, undefined);
const [selected, setSelected] = useStateControls(selectionControl, []);
const inputRef = useRef<HTMLInputElement>(null);
const scrollListRef = useRef<HTMLDivElement>(null);
const [searchText, updateSearchText] = useStateControls(searchControl, undefined);
Expand Down Expand Up @@ -48,13 +48,17 @@ export function AutoComplete<T, EL extends HTMLElement = HTMLDivElement>(props:
}, [getTextContent, items, match, searchText]);

const onListSelect = useCallback(
(selectedId?: string) => {
const item = items.find((item) => getId(item) === selectedId);
updateSearchText(item ? getTextContent(item) : '');
setSelected(selectedId);
(selectedIds: string[]) => {
const selectedItems = items.filter((item) => selectedIds.includes(getId(item)));

This comment has been minimized.

Sign in to view

Copy link
@alissawix

alissawix Dec 8, 2024

Contributor

without reading the autocomplete code, that is a bit risky...
the easy way out, if it uses the list is to assume that the array is of length 0 | 1 at this point

This comment has been minimized.

Sign in to view

Copy link
@TheAlmightyCrumb

TheAlmightyCrumb Dec 8, 2024

Author

Why is it risky?
updateSearchText(
selectedItems.length
? selectedItems.map((selectedItem) => getTextContent(selectedItem)).join(', ')
: '',
);
setSelected(selectedIds);
close();
},
[close, getId, getTextContent, items, setSelected, updateSearchText]
[close, getId, getTextContent, items, setSelected, updateSearchText],
);
const scrollListRoot = createListRoot(Area, {
className: classes.scrollListRoot,
Expand All @@ -74,7 +78,7 @@ export function AutoComplete<T, EL extends HTMLElement = HTMLDivElement>(props:
onKeyPress(ev);
}
},
[close, onKeyPress, open]
[close, onKeyPress, open],
);
useEffect(() => {
if (filteredData[0]) {
Expand Down
4 changes: 2 additions & 2 deletions packages/components/src/hooks/use-id-based-event.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, { useCallback } from 'react';
import { getElementWithId } from '../common';

export function useIdListener<EVType extends React.KeyboardEvent | React.MouseEvent>(
idSetter: (id: string | undefined, element?: Element) => void
idSetter: (id: string | undefined, element?: Element) => void,
): (ev: EVType) => any {
return useCallback(
(ev: EVType) => {
Expand All @@ -12,6 +12,6 @@ export function useIdListener<EVType extends React.KeyboardEvent | React.MouseEv
const res = getElementWithId(ev.target as Element, ev.currentTarget as unknown as Element);
idSetter(res?.id || undefined, res?.element);
},
[idSetter]
[idSetter],
);
}
4 changes: 2 additions & 2 deletions packages/components/src/hooks/use-keyboard-nav.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export const getHandleKeyboardNav = (
elementsParent: React.RefObject<HTMLElement>,
focusedId: string | undefined,
setFocusedId: (id: string) => void,
setSelectedId: (id: string) => void,
setSelectedIds: (ids: string[]) => void,
) => {
const onKeyPress = (ev: React.KeyboardEvent) => {
if (
Expand Down Expand Up @@ -117,7 +117,7 @@ export const getHandleKeyboardNav = (
break;
case KeyCodes.Space:
case KeyCodes.Enter:
setSelectedId(focusedId);
setSelectedIds([focusedId]);
break;
default:
}
Expand Down
6 changes: 3 additions & 3 deletions packages/components/src/hooks/use-tree-view-keyboard-interaction.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export interface TreeViewKeyboardInteractionsParams {
open: (itemId: string) => void;
close: (itemId: string) => void;
focus: (itemId: string) => void;
select: ProcessedControlledState<string, KeyboardSelectMeta>[1];
select: ProcessedControlledState<string[], KeyboardSelectMeta>[1];
isOpen: (itemId: string) => boolean;
isEndNode: (itemId: string) => boolean;
getPrevious: (itemId: string) => string | undefined;
Expand Down Expand Up @@ -60,7 +60,7 @@ export const useTreeViewKeyboardInteraction = ({
if (!itemId) return;

if (selectionFollowsFocus) {
select(itemId, 'keyboard');
select([itemId], 'keyboard');
} else {
focus(itemId);
}
Expand All @@ -72,7 +72,7 @@ export const useTreeViewKeyboardInteraction = ({
if (!focusedItemId) {
return;
}
select(focusedItemId);
select([focusedItemId]);
}, [focusedItemId, select]);

const handleArrowRight = useCallback(() => {
Expand Down
29 changes: 18 additions & 11 deletions packages/components/src/list/list.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export interface ListItemProps<T> {
isFocused: boolean;
isSelected: boolean;
focus: (id?: string) => void;
select: (id?: string) => void;
select: (id: string[]) => void;
}

export interface ListProps<T> {
Expand All @@ -51,7 +51,8 @@ export interface ListProps<T> {
items: T[];
ItemRenderer: React.ComponentType<ListItemProps<T>>;
focusControl?: StateControls<string | undefined>;
selectionControl?: StateControls<string | undefined>;
// selectionControl?: StateControls<string | undefined>;
selectionControl?: StateControls<string[]>;
transmitKeyPress?: UseTransmit<React.KeyboardEventHandler>;
onItemMount?: (item: T) => void;
onItemUnmount?: (item: T) => void;
Expand All @@ -72,21 +73,27 @@ export function List<T, EL extends HTMLElement = HTMLDivElement>({
onItemUnmount,
disableKeyboard,
}: ListProps<T>): JSX.Element {
const [selectedId, setSelectedId] = useStateControls(selectionControl, undefined);
const [selectedIds, setSelectedIds] = useStateControls(selectionControl, []);
const [focusedId, setFocusedId] = useStateControls(focusControl, undefined);
const [prevSelectedId, setPrevSelectedId] = useState(selectedId);
if (selectedId !== prevSelectedId) {
setFocusedId(selectedId);
setPrevSelectedId(selectedId);
const [prevSelectedId, setPrevSelectedId] = useState(selectedIds);
if (selectedIds !== prevSelectedId) {
setFocusedId(selectedIds[-1]);

This comment has been minimized.

Sign in to view

Copy link
@alissawix

alissawix Dec 8, 2024

Contributor

I tried the trick with -1. it doesn't work
setPrevSelectedId(selectedIds);
}
const defaultRef = useRef<EL>();
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const actualRef = listRoot?.props?.ref || defaultRef;

const onClick = useIdListener(setSelectedId);
const onClick = useIdListener((id) => {
if (selectedIds.findIndex((selectedId) => selectedId === id) !== -1) {
return;
}
setSelectedIds(id ? [id] : []);
});

const onKeyPress = disableKeyboard
? () => {}
: getHandleKeyboardNav(actualRef as React.RefObject<HTMLElement>, focusedId, setFocusedId, setSelectedId);
: getHandleKeyboardNav(actualRef as React.RefObject<HTMLElement>, focusedId, setFocusedId, setSelectedIds);
if (transmitKeyPress) {
transmitKeyPress(callInternalFirst(onKeyPress, listRoot?.props?.onKeyPress));
}
Expand All @@ -113,8 +120,8 @@ export function List<T, EL extends HTMLElement = HTMLDivElement>({
data={item}
focus={setFocusedId}
isFocused={focusedId === id}
isSelected={selectedId === id}
select={setSelectedId}
isSelected={selectedIds.findIndex((selectedId) => selectedId === id) !== -1}
select={setSelectedIds}
/>
);
})}
Expand Down
6 changes: 3 additions & 3 deletions packages/components/src/scroll-list/boards/scroll-to-selection.board.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export default createBoard({
name: 'ScrollList — scroll to selected item',
Board: () => {
const initialSelectedIndex = 442;
const [selectedItem, setSelectedItem] = useState(`a${initialSelectedIndex}`);
const [selectedItems, setSelectedItems] = useState([`a${initialSelectedIndex}`]);
const [input, setInput] = useState(initialSelectedIndex);

return (
Expand All @@ -60,7 +60,7 @@ export default createBoard({
/>
</label>

<button id={selectItemButton} onClick={() => setSelectedItem(`a${input}`)}>
<button id={selectItemButton} onClick={() => setSelectedItems([`a${input}`])}>
Select
</button>
</div>
Expand All @@ -71,7 +71,7 @@ export default createBoard({
items={items}
itemSize={() => 50}
getId={getId}
selectionControl={[selectedItem, noop]}
selectionControl={[selectedItems, noop]}
scrollToSelection={true}
scrollListRoot={{
el: 'div',
Expand Down
19 changes: 14 additions & 5 deletions packages/components/src/scroll-list/hooks/use-scroll-list-scroll-to-selected.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export const useScrollListScrollToSelected = <T, EL extends HTMLElement>({
scrollListRef: RefObject<EL>;
items: ListProps<T>['items'];
getId: ListProps<T>['getId'];
selected: string | undefined;
selected: string[];
averageItemSize: number;
itemsDimensions: MutableRefObject<DimensionsById>;
mountedItems: MutableRefObject<Set<string>>;
Expand All @@ -34,7 +34,7 @@ export const useScrollListScrollToSelected = <T, EL extends HTMLElement>({
const loadingTimeout = useRef(0);
const timeout = useRef(0);
const isScrollingToSelection = useRef(false);
const selectedIndex = useMemo(() => items.findIndex((i) => getId(i) === selected), [items, getId, selected]);
const selectedIndex = useMemo(() => items.findIndex((i) => getId(i) === selected[0]), [items, getId, selected]);
const calculateDistance = useCallback(
({ itemIndex, direction }: { itemIndex: number; direction: 'up' | 'down' }) => {
let distance = 0;
Expand All @@ -59,7 +59,16 @@ export const useScrollListScrollToSelected = <T, EL extends HTMLElement>({

return Math.floor((direction === 'down' ? 1 : -1) * distance);
},
[averageItemSize, extraRenderSize, getId, isHorizontal, items, itemsDimensions, scrollWindowSize, selectedIndex]
[
averageItemSize,
extraRenderSize,
getId,
isHorizontal,
items,
itemsDimensions,
scrollWindowSize,
selectedIndex,
],
);
const cleanUp = () => {
isScrollingToSelection.current = false;
Expand All @@ -78,7 +87,7 @@ export const useScrollListScrollToSelected = <T, EL extends HTMLElement>({
const node = scrollListRef.current?.querySelector(`[data-id='${getId(items[selected]!)}']`);
if (!node) {
timeout.current = window.setTimeout(
() => isScrollingToSelection.current && scrollTo(selected, true)
() => isScrollingToSelection.current && scrollTo(selected, true),

This comment has been minimized.

Sign in to view

Copy link
@alissawix

alissawix Dec 8, 2024 •
edited
Loading

Contributor

it probably should scroll to focused...
it isn't a problem yet, but we'll need to go over this one
);
} else {
scrollIntoViewIfNeeded(node, {
Expand Down Expand Up @@ -108,7 +117,7 @@ export const useScrollListScrollToSelected = <T, EL extends HTMLElement>({

timeout.current = window.setTimeout(() => scrollIntoView(selectedIndex, repeated ? 'center' : position));
},
[scrollListRef, scrollWindow, mountedItems, items, getId, calculateDistance]
[scrollListRef, scrollWindow, mountedItems, items, getId, calculateDistance],
);

useEffect(() => {
Expand Down
6 changes: 3 additions & 3 deletions packages/components/src/scroll-list/scroll-list.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,14 +169,14 @@ export function ScrollList<T, EL extends HTMLElement = HTMLDivElement>({
});
const scrollWindowSize = useElementSize(scrollWindow, !isHorizontal);
const mountedItems = useRef(new Set(''));
const [selected, setSelected] = useStateControls(selectionControl, undefined);
const [selected, setSelected] = useStateControls(selectionControl, []);
const [focused, setFocused] = useStateControls(focusControl, undefined);

const getItemInfo = useCallback(
(data: T): ScrollListItemInfo<T> => ({
data,
isFocused: focused === getId(data),
isSelected: selected === getId(data),
isSelected: selected.some((id) => getId(data) === id),
}),
[getId, focused, selected],
);
Expand Down Expand Up @@ -283,7 +283,7 @@ export function ScrollList<T, EL extends HTMLElement = HTMLDivElement>({
() => [focused, setFocused],
[focused, setFocused],
);
const selectionControlMemoized: ProcessedControlledState<string | undefined> = useMemo(
const selectionControlMemoized: ProcessedControlledState<string[]> = useMemo(
() => [selected, setSelected],
[selected, setSelected],
);
Expand Down
7 changes: 4 additions & 3 deletions packages/components/src/tree/boards/tree-focus.board.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ export default createBoard({
Board: () => {
const openItemsControl = useState<string[]>([]);
const scrollRef = useRef<HTMLDivElement>(null);
const selectionControl = useState<string | undefined>();
const selectionControl = useState<string[]>([]);
const [, setSelection] = selectionControl;
return (
<div>
<Tree<typeof data>
Expand All @@ -50,10 +51,10 @@ export default createBoard({
}}
eventRoots={[scrollRef]}
/>
<button id="clear" onClick={() => selectionControl[1](undefined)}>
<button id="clear" onClick={() => setSelection([])}>
clear selection
</button>
<button id="select" onClick={() => selectionControl[1]('5')}>
<button id="select" onClick={() => setSelection(['5'])}>
select 5
</button>
</div>
Expand Down
24 changes: 12 additions & 12 deletions packages/components/src/tree/boards/tree-with-lanes.board.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,12 @@ const mockedLane = lane(
el('p', [
lane(
[laneKinds.repeater],
[el('span')]
[el('span')],
),
]),
]),
]),
]
],
),
el('span', [
el('Comp', [
Expand All @@ -109,32 +109,32 @@ const mockedLane = lane(
el('p', [
lane(
[laneKinds.repeater],
[el('span')]
[el('span')],
),
]),
]),
]),
]
],
),
]
],
),
]),
]),
]),
]
],
),
el('span', [
el('Comp', [marker('children'), marker('header', [el('div')])]),
el('div', [el('p', [lane([laneKinds.repeater], [el('span')])])]),
]),
]
],
),
]
],
),
]),
]),
]),
]
],
);

const data: TreeItemWithLaneData = el('div', [mockedLane, mockedLane]);
Expand Down Expand Up @@ -170,7 +170,7 @@ const treeOverlay = createTreeOverlay(OverlayRenderer, {});
export default createBoard({
name: 'Tree with lanes',
Board: () => {
const [selection, updateSelection] = useState<string | undefined>(undefined);
const [selection, updateSelection] = useState<string[]>([]);
const [openItems, updateOpen] = useState<string[]>(allIds);

return (
Expand All @@ -180,10 +180,10 @@ export default createBoard({
getIndent,
getParents,
selectItem: (item) => {
updateSelection(item.id);
updateSelection([item.id]);
},
}),
[]
[],
)}
>
<Tree<TreeItemWithLaneData>
Expand Down
Loading

0 comments on commit 7e20c88

Please sign in to comment.