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 label editing code #14264

Merged
merged 7 commits into from
Sep 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 24 additions & 10 deletions packages/x-tree-view/src/TreeItem/TreeItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { TreeItem2Provider } from '../TreeItem2Provider';
import { TreeViewItemDepthContext } from '../internals/TreeViewItemDepthContext';
import { useTreeItemState } from './useTreeItemState';
import { isTargetInDescendants } from '../internals/utils/tree';
import { TreeViewItemPluginSlotPropsEnhancerParams } from '../internals/models';

const useThemeProps = createUseThemeProps('MuiTreeItem');

Expand Down Expand Up @@ -221,8 +222,16 @@ export const TreeItem = React.forwardRef(function TreeItem(
...other
} = props;

const { expanded, focused, selected, disabled, editing, handleExpansion } =
useTreeItemState(itemId);
const {
expanded,
focused,
selected,
disabled,
editing,
handleExpansion,
handleCancelItemLabelEditing,
handleSaveItemLabel,
} = useTreeItemState(itemId);

const { contentRef, rootRef, propsEnhancers } = runItemPlugins<TreeItemProps>(props);
const rootRefObject = React.useRef<HTMLLIElement>(null);
Expand Down Expand Up @@ -375,28 +384,33 @@ export const TreeItem = React.forwardRef(function TreeItem(
const idAttribute = instance.getTreeItemIdAttribute(itemId, id);
const tabIndex = instance.canItemBeTabbed(itemId) ? 0 : -1;

const sharedPropsEnhancerParams: Omit<
TreeViewItemPluginSlotPropsEnhancerParams,
'externalEventHandlers'
> = {
rootRefObject,
contentRefObject,
interactions: { handleSaveItemLabel, handleCancelItemLabelEditing },
};

const enhancedRootProps =
propsEnhancers.root?.({
rootRefObject,
contentRefObject,
...sharedPropsEnhancerParams,
externalEventHandlers: extractEventHandlers(other),
}) ?? {};
const enhancedContentProps =
propsEnhancers.content?.({
rootRefObject,
contentRefObject,
...sharedPropsEnhancerParams,
externalEventHandlers: extractEventHandlers(ContentProps),
}) ?? {};
const enhancedDragAndDropOverlayProps =
propsEnhancers.dragAndDropOverlay?.({
rootRefObject,
contentRefObject,
...sharedPropsEnhancerParams,
externalEventHandlers: {},
}) ?? {};
const enhancedLabelInputProps =
propsEnhancers.labelInput?.({
rootRefObject,
contentRefObject,
...sharedPropsEnhancerParams,
externalEventHandlers: {},
}) ?? {};

Expand Down
45 changes: 10 additions & 35 deletions packages/x-tree-view/src/TreeItem/TreeItemContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,6 @@ const TreeItemContent = React.forwardRef(function TreeItemContent(
preventSelection,
expansionTrigger,
toggleItemEditing,
handleSaveItemLabel,
handleCancelItemLabelEditing,
} = useTreeItemState(itemId);

const icon = iconProp || expansionIcon || displayIcon;
Expand Down Expand Up @@ -144,32 +142,6 @@ const TreeItemContent = React.forwardRef(function TreeItemContent(
}
toggleItemEditing();
};
const handleLabelInputBlur = (
event: React.FocusEvent<HTMLInputElement> & MuiCancellableEvent,
) => {
if (event.defaultMuiPrevented) {
return;
}

if (event.target.value) {
handleSaveItemLabel(event, event.target.value);
}
};

const handleLabelInputKeydown = (
event: React.KeyboardEvent<HTMLInputElement> & MuiCancellableEvent,
) => {
if (event.defaultMuiPrevented) {
return;
}

const target = event.target as HTMLInputElement;
if (event.key === 'Enter' && target.value) {
handleSaveItemLabel(event, target.value);
} else if (event.key === 'Escape') {
handleCancelItemLabelEditing(event);
}
};

return (
/* eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions -- Key event is handled by the TreeView */
Expand Down Expand Up @@ -200,12 +172,7 @@ const TreeItemContent = React.forwardRef(function TreeItemContent(
)}

{editing ? (
<TreeItem2LabelInput
{...labelInputProps}
className={classes.labelInput}
onBlur={handleLabelInputBlur}
onKeyDown={handleLabelInputKeydown}
/>
<TreeItem2LabelInput {...labelInputProps} className={classes.labelInput} />
) : (
<div className={classes.label} {...(editable && { onDoubleClick: handleLabelDoubleClick })}>
{label}
Expand Down Expand Up @@ -251,7 +218,15 @@ TreeItemContent.propTypes = {
* The tree item label.
*/
label: PropTypes.node,
labelInputProps: PropTypes.object,
labelInputProps: PropTypes.shape({
autoFocus: PropTypes.oneOf([true]),
'data-element': PropTypes.oneOf(['labelInput']),
onBlur: PropTypes.func,
onChange: PropTypes.func,
onKeyDown: PropTypes.func,
type: PropTypes.oneOf(['text']),
value: PropTypes.string,
}),
} as any;

export { TreeItemContent };
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
export interface TreeItem2LabelInputProps extends React.InputHTMLAttributes<HTMLInputElement> {
Copy link
Member Author

Choose a reason for hiding this comment

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

We should avoid to add props we don't actually use to the typing of our slots.
That way people implementing custom slots only need to support the props we actually need and not all the props we could theoretically use on our default slot component.

import * as React from 'react';
import { MuiCancellableEventHandler } from '../internals/models/MuiCancellableEvent';

export interface TreeItem2LabelInputProps {
value?: string;
onChange?: React.ChangeEventHandler<HTMLInputElement>;
/**
* Used to determine if the target of keydown or blur events is the input and prevent the event from propagating to the root.
*/
'data-element'?: 'labelInput';
onChange?: React.ChangeEventHandler<HTMLInputElement>;
onKeyDown?: MuiCancellableEventHandler<React.KeyboardEvent<HTMLInputElement>>;
onBlur?: MuiCancellableEventHandler<React.FocusEvent<HTMLInputElement>>;
autoFocus?: true;
type?: 'text';
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
import type { UseTreeItem2Status } from '../../useTreeItem2';
import { hasPlugin } from '../../internals/utils/plugins';

interface UseTreeItem2Interactions {
export interface UseTreeItem2Interactions {
handleExpansion: (event: React.MouseEvent) => void;
handleSelection: (event: React.MouseEvent) => void;
handleCheckboxSelection: (event: React.ChangeEvent<HTMLInputElement>) => void;
Expand Down
6 changes: 6 additions & 0 deletions packages/x-tree-view/src/internals/models/itemPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,17 @@ import type {
UseTreeItem2LabelInputSlotOwnProps,
UseTreeItem2RootSlotOwnProps,
} from '../../useTreeItem2';
import type { UseTreeItem2Interactions } from '../../hooks/useTreeItem2Utils/useTreeItem2Utils';

export interface TreeViewItemPluginSlotPropsEnhancerParams {
rootRefObject: React.MutableRefObject<HTMLLIElement | null>;
contentRefObject: React.MutableRefObject<HTMLDivElement | null>;
externalEventHandlers: EventHandlers;
// TODO v9: Remove "Pick" once the old TreeItem is removed.
interactions: Pick<
UseTreeItem2Interactions,
'handleSaveItemLabel' | 'handleCancelItemLabelEditing'
>;
}

type TreeViewItemPluginSlotPropsEnhancer<TSlotProps> = (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import * as React from 'react';
import { useTreeViewContext } from '../../TreeViewProvider';
import { TreeViewItemPlugin } from '../../models';
import { MuiCancellableEvent, TreeViewItemPlugin } from '../../models';
import { UseTreeViewItemsSignature } from '../useTreeViewItems';
import {
UseTreeItem2LabelInputSlotPropsFromItemsReordering,
UseTreeItem2LabelInputSlotPropsFromLabelEditing,
UseTreeViewLabelSignature,
} from './useTreeViewLabel.types';

export const isAndroid = () => navigator.userAgent.toLowerCase().includes('android');
Copy link
Member Author

Choose a reason for hiding this comment

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

Never used


export const useTreeViewLabelItemPlugin: TreeViewItemPlugin<any> = ({ props }) => {
const { instance } = useTreeViewContext<[UseTreeViewItemsSignature, UseTreeViewLabelSignature]>();
const { label, itemId } = props;
Expand All @@ -27,13 +25,41 @@ export const useTreeViewLabelItemPlugin: TreeViewItemPlugin<any> = ({ props }) =
propsEnhancers: {
labelInput: ({
externalEventHandlers,
}): UseTreeItem2LabelInputSlotPropsFromItemsReordering => {
interactions,
}): UseTreeItem2LabelInputSlotPropsFromLabelEditing => {
const editable = instance.isItemEditable(itemId);

if (!editable) {
return {};
}

const handleKeydown = (
Copy link
Member Author

Choose a reason for hiding this comment

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

Move those two callbacks here, that way useTreeItem2 is totally agnostic of anything regarding this prop
We have some infos about label editing in other slots but nothing too important

event: React.KeyboardEvent<HTMLInputElement> & MuiCancellableEvent,
) => {
externalEventHandlers.onKeyDown?.(event);
if (event.defaultMuiPrevented) {
return;
}
const target = event.target as HTMLInputElement;

if (event.key === 'Enter' && target.value) {
interactions.handleSaveItemLabel(event, target.value);
} else if (event.key === 'Escape') {
interactions.handleCancelItemLabelEditing(event);
}
};

const handleBlur = (event: React.FocusEvent<HTMLInputElement> & MuiCancellableEvent) => {
externalEventHandlers.onBlur?.(event);
if (event.defaultMuiPrevented) {
return;
}

if (event.target.value) {
interactions.handleSaveItemLabel(event, event.target.value);
}
};

const handleInputChange = (event: React.ChangeEvent<HTMLInputElement>) => {
externalEventHandlers.onChange?.(event);
setLabelInputValue(event.target.value);
Expand All @@ -43,6 +69,8 @@ export const useTreeViewLabelItemPlugin: TreeViewItemPlugin<any> = ({ props }) =
value: labelInputValue ?? '',
'data-element': 'labelInput',
onChange: handleInputChange,
onKeyDown: handleKeydown,
onBlur: handleBlur,
autoFocus: true,
type: 'text',
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,10 @@ export type UseTreeViewLabelSignature = TreeViewPluginSignature<{
experimentalFeatures: 'labelEditing';
dependencies: [UseTreeViewItemsSignature];
}>;
export interface UseTreeItem2LabelInputSlotPropsFromItemsReordering
Copy link
Member Author

Choose a reason for hiding this comment

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

Typo in the name and the props were never added to the typing of UseTreeItem2LabelInputSlotProps

extends TreeItem2LabelInputProps {}

export interface UseTreeItem2LabelInputSlotPropsFromLabelEditing extends TreeItem2LabelInputProps {}

declare module '@mui/x-tree-view/useTreeItem2' {
interface UseTreeItem2LabelInputSlotOwnProps
extends UseTreeItem2LabelInputSlotPropsFromLabelEditing {}
}
70 changes: 22 additions & 48 deletions packages/x-tree-view/src/useTreeItem2/useTreeItem2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ import {
UseTreeItem2ContentSlotPropsFromUseTreeItem,
} from './useTreeItem2.types';
import { useTreeViewContext } from '../internals/TreeViewProvider';
import { MuiCancellableEvent } from '../internals/models';
import {
MuiCancellableEvent,
TreeViewItemPluginSlotPropsEnhancerParams,
} from '../internals/models';
import { useTreeItem2Utils } from '../hooks/useTreeItem2Utils';
import { TreeViewItemDepthContext } from '../internals/TreeViewItemDepthContext';
import { isTargetInDescendants } from '../internals/utils/tree';
Expand Down Expand Up @@ -52,6 +55,11 @@ export const useTreeItem2 = <
const checkboxRef = React.useRef<HTMLButtonElement>(null);
const rootTabIndex = instance.canItemBeTabbed(itemId) ? 0 : -1;

const sharedPropsEnhancerParams: Omit<
TreeViewItemPluginSlotPropsEnhancerParams,
'externalEventHandlers'
> = { rootRefObject, contentRefObject, interactions };

const createRootHandleFocus =
(otherHandlers: EventHandlers) =>
(event: React.FocusEvent<HTMLElement> & MuiCancellableEvent) => {
Expand Down Expand Up @@ -164,35 +172,6 @@ export const useTreeItem2 = <
interactions.handleCheckboxSelection(event);
};

const createInputHandleKeydown =
(otherHandlers: EventHandlers) =>
(event: React.KeyboardEvent<HTMLInputElement> & MuiCancellableEvent) => {
otherHandlers.onKeyDown?.(event);
if (event.defaultMuiPrevented) {
return;
}
const target = event.target as HTMLInputElement;

if (event.key === 'Enter' && target.value) {
interactions.handleSaveItemLabel(event, target.value);
} else if (event.key === 'Escape') {
interactions.handleCancelItemLabelEditing(event);
}
};

const createInputHandleBlur =
(otherHandlers: EventHandlers) =>
(event: React.FocusEvent<HTMLInputElement> & MuiCancellableEvent) => {
otherHandlers.onBlur?.(event);
if (event.defaultMuiPrevented) {
return;
}

if (event.target.value) {
interactions.handleSaveItemLabel(event, event.target.value);
}
};

const createIconContainerHandleClick =
(otherHandlers: EventHandlers) => (event: React.MouseEvent & MuiCancellableEvent) => {
otherHandlers.onClick?.(event);
Expand Down Expand Up @@ -248,7 +227,7 @@ export const useTreeItem2 = <
}

const enhancedRootProps =
propsEnhancers.root?.({ rootRefObject, contentRefObject, externalEventHandlers }) ?? {};
propsEnhancers.root?.({ ...sharedPropsEnhancerParams, externalEventHandlers }) ?? {};

return {
...props,
Expand All @@ -275,7 +254,7 @@ export const useTreeItem2 = <
}

const enhancedContentProps =
propsEnhancers.content?.({ rootRefObject, contentRefObject, externalEventHandlers }) ?? {};
propsEnhancers.content?.({ ...sharedPropsEnhancerParams, externalEventHandlers }) ?? {};

return {
...props,
Expand Down Expand Up @@ -326,19 +305,17 @@ export const useTreeItem2 = <
): UseTreeItem2LabelInputSlotProps<ExternalProps> => {
const externalEventHandlers = extractEventHandlers(externalProps);

const props = {
...externalEventHandlers,
...externalProps,
onKeyDown: createInputHandleKeydown(externalEventHandlers),
onBlur: createInputHandleBlur(externalEventHandlers),
};

const enhancedlabelInputProps =
propsEnhancers.labelInput?.({ rootRefObject, contentRefObject, externalEventHandlers }) ?? {};
const enhancedLabelInputProps =
propsEnhancers.labelInput?.({
rootRefObject,
contentRefObject,
externalEventHandlers,
interactions,
}) ?? {};

return {
...props,
...enhancedlabelInputProps,
...externalProps,
...enhancedLabelInputProps,
} as UseTreeItem2LabelInputSlotProps<ExternalProps>;
};

Expand Down Expand Up @@ -379,14 +356,11 @@ export const useTreeItem2 = <
const getDragAndDropOverlayProps = <ExternalProps extends Record<string, any> = {}>(
externalProps: ExternalProps = {} as ExternalProps,
): UseTreeItem2DragAndDropOverlaySlotProps<ExternalProps> => {
const externalEventHandlers = {
...extractEventHandlers(externalProps),
};
const externalEventHandlers = extractEventHandlers(externalProps);

const enhancedDragAndDropOverlayProps =
propsEnhancers.dragAndDropOverlay?.({
rootRefObject,
contentRefObject,
...sharedPropsEnhancerParams,
externalEventHandlers,
}) ?? {};

Expand Down
Loading