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.

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

[TreeView] Clean up usage of term node in internals #12655

Merged
merged 7 commits into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/data/tree-view/rich-tree-view/headless/headless.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ We probably need a new abstraction here so that a plugin is always responsible f
TODO

:::warning
Once `focusedNodeId` becomes a model, we could consider removing the notion of state and just let each plugin define its state and provide methods in the instance to access / update it.
Once `focusedItemId` becomes a model, we could consider removing the notion of state and just let each plugin define its state and provide methods in the instance to access / update it.
:::

### Populate the Tree View instance
Expand Down
2 changes: 1 addition & 1 deletion packages/x-tree-view/src/internals/models/treeView.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { TreeViewAnyPluginSignature } from './plugin';
import type { MergePluginsProperty } from './helpers';

export interface TreeViewNode {
export interface TreeViewItemMeta {
id: string;
idAttribute: string | undefined;
index: number;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const useTreeViewExpansion: TreeViewPlugin<UseTreeViewExpansionSignature>
);

const isItemExpandable = React.useCallback(
(itemId: string) => !!instance.getNode(itemId)?.expandable,
(itemId: string) => !!instance.getItemMeta(itemId)?.expandable,
[instance],
);

Expand Down Expand Up @@ -52,8 +52,8 @@ export const useTreeViewExpansion: TreeViewPlugin<UseTreeViewExpansionSignature>
);

const expandAllSiblings = (event: React.KeyboardEvent, itemId: string) => {
const node = instance.getNode(itemId);
const siblings = instance.getChildrenIds(node.parentId);
const item = instance.getItemMeta(itemId);
noraleonte marked this conversation as resolved.
Show resolved Hide resolved
const siblings = instance.getChildrenIds(item.parentId);

const diff = siblings.filter(
(child) => instance.isItemExpandable(child) && !instance.isItemExpanded(child),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ const useTabbableItemId = (
selectedItems: string | string[] | null,
) => {
const isItemVisible = (itemId: string) => {
const node = instance.getNode(itemId);
return node && (node.parentId == null || instance.isItemExpanded(node.parentId));
const item = instance.getItemMeta(itemId);
return item && (item.parentId == null || instance.isItemExpanded(item.parentId));
};

let tabbableItemId: string | null | undefined;
Expand Down Expand Up @@ -62,13 +62,13 @@ export const useTreeViewFocus: TreeViewPlugin<UseTreeViewFocusSignature> = ({
);

const isItemVisible = (itemId: string) => {
const node = instance.getNode(itemId);
return node && (node.parentId == null || instance.isItemExpanded(node.parentId));
const item = instance.getItemMeta(itemId);
return item && (item.parentId == null || instance.isItemExpanded(item.parentId));
};

const innerFocusItem = (event: React.SyntheticEvent | null, itemId: string) => {
const node = instance.getNode(itemId);
const itemElement = document.getElementById(instance.getTreeItemId(itemId, node.idAttribute));
const item = instance.getItemMeta(itemId);
const itemElement = document.getElementById(instance.getTreeItemId(itemId, item.idAttribute));
if (itemElement) {
itemElement.focus();
}
Expand Down Expand Up @@ -106,10 +106,10 @@ export const useTreeViewFocus: TreeViewPlugin<UseTreeViewFocusSignature> = ({
return;
}

const node = instance.getNode(state.focusedItemId);
if (node) {
const item = instance.getItemMeta(state.focusedItemId);
if (item) {
const itemElement = document.getElementById(
instance.getTreeItemId(state.focusedItemId, node.idAttribute),
instance.getTreeItemId(state.focusedItemId, item.idAttribute),
);
if (itemElement) {
itemElement.blur();
Expand Down Expand Up @@ -148,7 +148,7 @@ export const useTreeViewFocus: TreeViewPlugin<UseTreeViewFocusSignature> = ({
}
};

const focusedItem = instance.getNode(state.focusedItemId!);
const focusedItem = instance.getItemMeta(state.focusedItemId!);
const activeDescendant = focusedItem
? instance.getTreeItemId(focusedItem.id, focusedItem.idAttribute)
: null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { populateInstance, populatePublicAPI } from '../../useTreeView/useTreeVi
import {
UseTreeViewItemsSignature,
UseTreeViewItemsDefaultizedParameters,
TreeViewNodeMap,
TreeViewItemMetaMap,
TreeViewItemIdAndChildren,
UseTreeViewItemsState,
TreeViewItemMap,
Expand All @@ -21,7 +21,7 @@ const updateItemsState = ({
UseTreeViewItemsDefaultizedParameters<TreeViewBaseItem>,
'items' | 'isItemDisabled' | 'getItemLabel' | 'getItemId'
>): UseTreeViewItemsState<any>['items'] => {
const nodeMap: TreeViewNodeMap = {};
const itemMetaMap: TreeViewItemMetaMap = {};
const itemMap: TreeViewItemMap<any> = {};

const processItem = (
Expand All @@ -42,7 +42,7 @@ const updateItemsState = ({
);
}

if (nodeMap[id] != null) {
if (itemMetaMap[id] != null) {
throw new Error(
[
'MUI X: The Tree View component requires all items to have a unique `id` property.',
Expand All @@ -64,7 +64,7 @@ const updateItemsState = ({
);
}

nodeMap[id] = {
itemMetaMap[id] = {
id,
label,
index,
Expand All @@ -82,11 +82,11 @@ const updateItemsState = ({
};
};

const nodeTree = items.map((item, itemIndex) => processItem(item, itemIndex, null));
const itemTree = items.map((item, itemIndex) => processItem(item, itemIndex, null));

return {
nodeMap,
nodeTree,
itemMetaMap,
itemTree,
itemMap,
};
};
Expand All @@ -98,9 +98,9 @@ export const useTreeViewItems: TreeViewPlugin<UseTreeViewItemsSignature> = ({
state,
setState,
}) => {
const getNode = React.useCallback(
(itemId: string) => state.items.nodeMap[itemId],
[state.items.nodeMap],
const getItemMeta = React.useCallback(
(itemId: string) => state.items.itemMetaMap[itemId],
[state.items.itemMetaMap],
);

const getItem = React.useCallback(
Expand All @@ -114,20 +114,20 @@ export const useTreeViewItems: TreeViewPlugin<UseTreeViewItemsSignature> = ({
return false;
}

let node = instance.getNode(itemId);
let item = instance.getItemMeta(itemId);

// This can be called before the item has been added to the item map.
if (!node) {
if (!item) {
return false;
}

if (node.disabled) {
if (item.disabled) {
return true;
}

while (node.parentId != null) {
node = instance.getNode(node.parentId);
if (node.disabled) {
while (item.parentId != null) {
item = instance.getItemMeta(item.parentId);
if (item.disabled) {
return true;
}
}
Expand All @@ -139,11 +139,11 @@ export const useTreeViewItems: TreeViewPlugin<UseTreeViewItemsSignature> = ({

const getChildrenIds = React.useCallback(
(itemId: string | null) =>
Object.values(state.items.nodeMap)
Object.values(state.items.itemMetaMap)
.filter((item) => item.parentId === itemId)
.sort((a, b) => a.index - b.index)
.map((child) => child.id),
[state.items.nodeMap],
[state.items.itemMetaMap],
);

const getNavigableChildrenIds = (itemId: string | null) => {
Expand Down Expand Up @@ -175,8 +175,8 @@ export const useTreeViewItems: TreeViewPlugin<UseTreeViewItemsSignature> = ({
getItemLabel: params.getItemLabel,
});

Object.values(prevState.items.nodeMap).forEach((item) => {
if (!newState.nodeMap[item.id]) {
Object.values(prevState.items.itemMetaMap).forEach((item) => {
if (!newState.itemMetaMap[item.id]) {
publishTreeViewEvent(instance, 'removeItem', { id: item.id });
}
});
Expand All @@ -197,7 +197,7 @@ export const useTreeViewItems: TreeViewPlugin<UseTreeViewItemsSignature> = ({
id,
children,
}: TreeViewItemIdAndChildren): ReturnType<typeof instance.getItemsToRender>[number] => {
const item = state.items.nodeMap[id];
const item = state.items.itemMetaMap[id];
return {
label: item.label!,
itemId: item.id,
Expand All @@ -206,11 +206,11 @@ export const useTreeViewItems: TreeViewPlugin<UseTreeViewItemsSignature> = ({
};
};

return state.items.nodeTree.map(getPropsFromItemId);
return state.items.itemTree.map(getPropsFromItemId);
};

populateInstance<UseTreeViewItemsSignature>(instance, {
getNode,
getItemMeta,
getItem,
getItemsToRender,
getChildrenIds,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { TreeViewNode, DefaultizedProps, TreeViewPluginSignature } from '../../models';
import { TreeViewItemMeta, DefaultizedProps, TreeViewPluginSignature } from '../../models';
import { TreeViewItemId } from '../../../models';

interface TreeViewItemProps {
Expand All @@ -18,7 +18,7 @@ export interface UseTreeViewItemsPublicAPI<R extends {}> {
}

export interface UseTreeViewItemsInstance<R extends {}> extends UseTreeViewItemsPublicAPI<R> {
getNode: (itemId: string) => TreeViewNode;
getItemMeta: (itemId: string) => TreeViewItemMeta;
getItemsToRender: () => TreeViewItemProps[];
getChildrenIds: (itemId: string | null) => string[];
getNavigableChildrenIds: (itemId: string | null) => string[];
Expand Down Expand Up @@ -88,8 +88,8 @@ export interface TreeViewItemIdAndChildren {

export interface UseTreeViewItemsState<R extends {}> {
items: {
nodeTree: TreeViewItemIdAndChildren[];
nodeMap: TreeViewNodeMap;
itemTree: TreeViewItemIdAndChildren[];
itemMetaMap: TreeViewItemMetaMap;
itemMap: TreeViewItemMap<R>;
};
}
Expand All @@ -107,6 +107,6 @@ export type UseTreeViewItemsSignature = TreeViewPluginSignature<{
contextValue: UseTreeViewItemsContextValue;
}>;

export type TreeViewNodeMap = { [itemId: string]: TreeViewNode };
export type TreeViewItemMetaMap = { [itemId: string]: TreeViewItemMeta };

export type TreeViewItemMap<R extends {}> = { [itemId: string]: R };
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react';
import useEventCallback from '@mui/utils/useEventCallback';
import useForkRef from '@mui/utils/useForkRef';
import { TreeViewItemPlugin, TreeViewNode, TreeViewPlugin } from '../../models';
import { TreeViewItemPlugin, TreeViewItemMeta, TreeViewPlugin } from '../../models';
import { populateInstance } from '../../useTreeView/useTreeView.utils';
import { UseTreeViewJSXItemsSignature } from './useTreeViewJSXItems.types';
import { publishTreeViewEvent } from '../../utils/publishTreeViewEvent';
Expand All @@ -20,9 +20,9 @@ export const useTreeViewJSXItems: TreeViewPlugin<UseTreeViewJSXItemsSignature> =
}) => {
instance.preventItemUpdates();

const insertJSXItem = useEventCallback((item: TreeViewNode) => {
const insertJSXItem = useEventCallback((item: TreeViewItemMeta) => {
setState((prevState) => {
if (prevState.items.nodeMap[item.id] != null) {
if (prevState.items.itemMetaMap[item.id] != null) {
throw new Error(
[
'MUI X: The Tree View component requires all items to have a unique `id` property.',
Expand All @@ -36,7 +36,7 @@ export const useTreeViewJSXItems: TreeViewPlugin<UseTreeViewJSXItemsSignature> =
...prevState,
items: {
...prevState.items,
nodeMap: { ...prevState.items.nodeMap, [item.id]: item },
itemMetaMap: { ...prevState.items.itemMetaMap, [item.id]: item },
// For `SimpleTreeView`, we don't have a proper `item` object, so we create a very basic one.
itemMap: { ...prevState.items.itemMap, [item.id]: { id: item.id, label: item.label } },
},
Expand All @@ -46,15 +46,15 @@ export const useTreeViewJSXItems: TreeViewPlugin<UseTreeViewJSXItemsSignature> =

const removeJSXItem = useEventCallback((itemId: string) => {
setState((prevState) => {
const newNodeMap = { ...prevState.items.nodeMap };
const newItemMetaMap = { ...prevState.items.itemMetaMap };
const newItemMap = { ...prevState.items.itemMap };
delete newNodeMap[itemId];
delete newItemMetaMap[itemId];
delete newItemMap[itemId];
return {
...prevState,
items: {
...prevState.items,
nodeMap: newNodeMap,
itemMetaMap: newItemMetaMap,
itemMap: newItemMap,
},
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { TreeViewNode, TreeViewPluginSignature } from '../../models';
import { TreeViewItemMeta, TreeViewPluginSignature } from '../../models';
import { UseTreeViewItemsSignature } from '../useTreeViewItems';
import { UseTreeViewKeyboardNavigationSignature } from '../useTreeViewKeyboardNavigation';

export interface UseTreeViewItemsInstance {
insertJSXItem: (item: TreeViewNode) => void;
insertJSXItem: (item: TreeViewItemMeta) => void;
removeJSXItem: (itemId: string) => void;
mapFirstCharFromJSX: (itemId: string, firstChar: string) => () => void;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react';
import { useTheme } from '@mui/material/styles';
import useEventCallback from '@mui/utils/useEventCallback';
import { TreeViewNode, TreeViewPlugin } from '../../models';
import { TreeViewItemMeta, TreeViewPlugin } from '../../models';
import {
getFirstItem,
getLastItem,
Expand Down Expand Up @@ -48,13 +48,13 @@ export const useTreeViewKeyboardNavigation: TreeViewPlugin<

const newFirstCharMap: { [itemId: string]: string } = {};

const processItem = (node: TreeViewNode) => {
newFirstCharMap[node.id] = node.label!.substring(0, 1).toLowerCase();
const processItem = (item: TreeViewItemMeta) => {
newFirstCharMap[item.id] = item.label!.substring(0, 1).toLowerCase();
};

Object.values(state.items.nodeMap).forEach(processItem);
Object.values(state.items.itemMetaMap).forEach(processItem);
firstCharMap.current = newFirstCharMap;
}, [state.items.nodeMap, params.getItemId, instance]);
}, [state.items.itemMetaMap, params.getItemId, instance]);

const getFirstMatchingItem = (itemId: string, firstChar: string) => {
let start: number;
Expand All @@ -65,7 +65,7 @@ export const useTreeViewKeyboardNavigation: TreeViewPlugin<
const firstChars: string[] = [];
// This really only works since the ids are strings
Object.keys(firstCharMap.current).forEach((mapItemId) => {
const map = instance.getNode(mapItemId);
const map = instance.getItemMeta(mapItemId);
const visible = map.parentId ? instance.isItemExpanded(map.parentId) : true;
const shouldBeSkipped = params.disabledItemsFocusable
? false
Expand Down Expand Up @@ -228,7 +228,7 @@ export const useTreeViewKeyboardNavigation: TreeViewPlugin<
instance.toggleItemExpansion(event, itemId);
event.preventDefault();
} else {
const parent = instance.getNode(itemId).parentId;
const parent = instance.getItemMeta(itemId).parentId;
if (parent) {
instance.focusItem(event, parent);
event.preventDefault();
Expand Down