From e3eed45d6d445d574c2a1580a3b2cbaf2cef3929 Mon Sep 17 00:00:00 2001 From: Bernardo Sunderhus Date: Tue, 27 Dec 2022 18:05:11 +0000 Subject: [PATCH 1/6] feat(react-tree): properly position actions --- .../BaseTreeItem/BaseTreeItem.types.ts | 11 +--- .../BaseTreeItem/useBaseTreeItem.ts | 57 +++++++++--------- .../src/components/Tree/Tree.types.ts | 17 ++++-- .../src/components/TreeItem/TreeItem.types.ts | 14 +---- .../components/TreeItem/renderTreeItem.tsx | 18 +++--- .../src/components/TreeItem/useTreeItem.tsx | 59 +++++++++++++------ .../components/TreeItem/useTreeItemStyles.ts | 30 ++++++---- .../stories/Tree/TreeActions.stories.tsx | 17 +++++- 8 files changed, 130 insertions(+), 93 deletions(-) diff --git a/packages/react-components/react-tree/src/components/BaseTreeItem/BaseTreeItem.types.ts b/packages/react-components/react-tree/src/components/BaseTreeItem/BaseTreeItem.types.ts index 15a60b4e5c5ec..c6a016bf475b4 100644 --- a/packages/react-components/react-tree/src/components/BaseTreeItem/BaseTreeItem.types.ts +++ b/packages/react-components/react-tree/src/components/BaseTreeItem/BaseTreeItem.types.ts @@ -1,16 +1,7 @@ import type { ComponentProps, ComponentState, Slot } from '@fluentui/react-utilities'; -import type { ARIAButtonElement, ARIAButtonElementIntersection, ARIAButtonSlotProps } from '@fluentui/react-aria'; - -export type BaseTreeItemElement = ARIAButtonElement; - -/** @internal */ -export type BaseTreeItemElementIntersection = ARIAButtonElementIntersection; export type BaseTreeItemSlots = { - /** - * BaseTreeItem root wraps around `props.content` - */ - root: Slot; + root: Slot<'div'>; }; /** diff --git a/packages/react-components/react-tree/src/components/BaseTreeItem/useBaseTreeItem.ts b/packages/react-components/react-tree/src/components/BaseTreeItem/useBaseTreeItem.ts index 1cd1af387ac38..86b3bf3f8be89 100644 --- a/packages/react-components/react-tree/src/components/BaseTreeItem/useBaseTreeItem.ts +++ b/packages/react-components/react-tree/src/components/BaseTreeItem/useBaseTreeItem.ts @@ -1,13 +1,7 @@ import * as React from 'react'; import { getNativeElementProps, useEventCallback } from '@fluentui/react-utilities'; -import type { - BaseTreeItemElement, - BaseTreeItemElementIntersection, - BaseTreeItemProps, - BaseTreeItemState, -} from './BaseTreeItem.types'; -import { useARIAButtonProps } from '@fluentui/react-aria'; -import { ArrowRight, ArrowLeft } from '@fluentui/keyboard-keys'; +import type { BaseTreeItemProps, BaseTreeItemState } from './BaseTreeItem.types'; +import { ArrowRight, ArrowLeft, Enter } from '@fluentui/keyboard-keys'; import { useTreeContext_unstable } from '../../contexts/treeContext'; /** * Create the state required to render BaseTreeItem. @@ -20,7 +14,7 @@ import { useTreeContext_unstable } from '../../contexts/treeContext'; */ export const useBaseTreeItem_unstable = ( props: BaseTreeItemProps, - ref: React.Ref, + ref: React.Ref, ): BaseTreeItemState => { const { 'aria-owns': ariaOwns, as = 'div', onKeyDown, ...rest } = props; @@ -32,12 +26,12 @@ export const useBaseTreeItem_unstable = ( const isBranch = typeof ariaOwns === 'string'; const open = useTreeContext_unstable(ctx => isBranch && ctx.openSubtrees.includes(ariaOwns!)); - const handleClick = useEventCallback((event: React.MouseEvent) => { + const handleClick = useEventCallback((event: React.MouseEvent) => { if (isBranch) { requestOpenChange({ event, open: !open, type: 'click', id: ariaOwns! }); } }); - const handleArrowRight = (event: React.KeyboardEvent) => { + const handleArrowRight = (event: React.KeyboardEvent) => { if (open && isBranch) { focusFirstSubtreeItem(event.currentTarget); } @@ -45,7 +39,7 @@ export const useBaseTreeItem_unstable = ( requestOpenChange({ event, open: true, type: 'arrowRight', id: ariaOwns! }); } }; - const handleArrowLeft = (event: React.KeyboardEvent) => { + const handleArrowLeft = (event: React.KeyboardEvent) => { if (!isBranch || !open) { focusSubtreeOwnerItem(event.currentTarget); } @@ -53,12 +47,20 @@ export const useBaseTreeItem_unstable = ( requestOpenChange({ event, open: false, type: 'arrowLeft', id: ariaOwns! }); } }; - const handleKeyDown = useEventCallback((event: React.KeyboardEvent) => { + const handleEnter = (event: React.KeyboardEvent) => { + if (isBranch) { + requestOpenChange({ event, open: !open, type: 'enter', id: ariaOwns! }); + } + }; + const handleKeyDown = useEventCallback((event: React.KeyboardEvent) => { onKeyDown?.(event); if (event.isDefaultPrevented()) { return; } - switch (event.code) { + switch (event.key) { + case Enter: { + return handleEnter(event); + } case ArrowRight: { return handleArrowRight(event); } @@ -73,20 +75,17 @@ export const useBaseTreeItem_unstable = ( }, isLeaf: !isBranch, open, - root: getNativeElementProps( - as, - useARIAButtonProps(as, { - ...rest, - // casting here is required to convert union to intersection - ref: ref as React.Ref, - 'aria-owns': ariaOwns, - 'aria-level': level, - // FIXME: tabster fails to navigate when aria-expanded is true - // 'aria-expanded': isBranch ? isOpen : undefined, - role: 'treeitem', - onClick: handleClick, - onKeyDown: handleKeyDown, - }), - ), + root: getNativeElementProps(as, { + ...rest, + ref, + tabIndex: 0, + 'aria-owns': ariaOwns, + 'aria-level': level, + // FIXME: tabster fails to navigate when aria-expanded is true + // 'aria-expanded': isBranch ? isOpen : undefined, + role: 'treeitem', + onClick: handleClick, + onKeyDown: handleKeyDown, + }), }; }; diff --git a/packages/react-components/react-tree/src/components/Tree/Tree.types.ts b/packages/react-components/react-tree/src/components/Tree/Tree.types.ts index fb1bdd91c67ac..4e26e1fea3a96 100644 --- a/packages/react-components/react-tree/src/components/Tree/Tree.types.ts +++ b/packages/react-components/react-tree/src/components/Tree/Tree.types.ts @@ -1,6 +1,5 @@ import * as React from 'react'; import type { ComponentProps, ComponentState, Slot } from '@fluentui/react-utilities'; -import type { BaseTreeItemElement } from '../BaseTreeItem/BaseTreeItem.types'; import { TreeContextValue } from '../../contexts/treeContext'; export type TreeSlots = { @@ -9,16 +8,24 @@ export type TreeSlots = { export type TreeOpenChangeData = { open: boolean; id: string } & ( | { - event: React.MouseEvent; + event: React.MouseEvent; type: 'expandIconClick'; } | { - event: React.MouseEvent; + event: React.MouseEvent; type: 'click'; } | { - event: React.KeyboardEvent; - type: 'arrowRight' | 'arrowLeft'; + event: React.KeyboardEvent; + type: 'enter'; + } + | { + event: React.KeyboardEvent; + type: 'arrowRight'; + } + | { + event: React.KeyboardEvent; + type: 'arrowLeft'; } ); diff --git a/packages/react-components/react-tree/src/components/TreeItem/TreeItem.types.ts b/packages/react-components/react-tree/src/components/TreeItem/TreeItem.types.ts index 4fe814c7abf87..862a726f29e0d 100644 --- a/packages/react-components/react-tree/src/components/TreeItem/TreeItem.types.ts +++ b/packages/react-components/react-tree/src/components/TreeItem/TreeItem.types.ts @@ -1,16 +1,5 @@ import type { ComponentProps, ComponentState, Slot } from '@fluentui/react-utilities'; -import { - BaseTreeItemElement, - BaseTreeItemElementIntersection, - BaseTreeItemProps, - BaseTreeItemSlots, - BaseTreeItemState, -} from '../BaseTreeItem/index'; - -export type TreeItemElement = BaseTreeItemElement; - -/** @internal */ -export type TreeItemElementIntersection = BaseTreeItemElementIntersection; +import { BaseTreeItemProps, BaseTreeItemSlots, BaseTreeItemState } from '../BaseTreeItem/index'; export type TreeItemSlots = BaseTreeItemSlots & { /** @@ -35,6 +24,7 @@ export type TreeItemSlots = BaseTreeItemSlots & { * when the item is hovered/focused */ actions?: Slot<'span'>; + groupper: NonNullable>; }; /** diff --git a/packages/react-components/react-tree/src/components/TreeItem/renderTreeItem.tsx b/packages/react-components/react-tree/src/components/TreeItem/renderTreeItem.tsx index fc9a376575b50..6ebab4c66f968 100644 --- a/packages/react-components/react-tree/src/components/TreeItem/renderTreeItem.tsx +++ b/packages/react-components/react-tree/src/components/TreeItem/renderTreeItem.tsx @@ -9,13 +9,15 @@ export const renderTreeItem_unstable = (state: TreeItemState) => { const { slots, slotProps } = getSlots(state); return ( - - {slots.expandIcon && } - {slots.iconBefore && } - {slotProps.root.children} - {slots.iconAfter && } - {slots.badges && } - {slots.actions && } - + + + {slots.expandIcon && } + {slots.iconBefore && } + {slotProps.root.children} + {slots.iconAfter && } + {slots.badges && } + {slots.actions && } + + ); }; diff --git a/packages/react-components/react-tree/src/components/TreeItem/useTreeItem.tsx b/packages/react-components/react-tree/src/components/TreeItem/useTreeItem.tsx index f30bcd068fc15..2732354504016 100644 --- a/packages/react-components/react-tree/src/components/TreeItem/useTreeItem.tsx +++ b/packages/react-components/react-tree/src/components/TreeItem/useTreeItem.tsx @@ -1,10 +1,14 @@ import * as React from 'react'; -import { isResolvedShorthand, resolveShorthand } from '@fluentui/react-utilities'; -import type { TreeItemElement, TreeItemProps, TreeItemState } from './TreeItem.types'; +import { ExtractSlotProps, isResolvedShorthand, resolveShorthand, Slot } from '@fluentui/react-utilities'; import { ChevronRightRegular } from '@fluentui/react-icons'; import { useFluent_unstable } from '@fluentui/react-shared-contexts'; -import { useBaseTreeItem_unstable } from '../BaseTreeItem/index'; import { useEventCallback } from '@fluentui/react-utilities'; +import { useFocusableGroup } from '@fluentui/react-tabster'; +import { expandIconInlineStyles } from './useTreeItemStyles'; +import type { TreeItemProps, TreeItemState } from './TreeItem.types'; +import { useBaseTreeItem_unstable } from '../BaseTreeItem/index'; +import { Enter } from '@fluentui/keyboard-keys'; +import { useMergedRefs } from '@fluentui/react-utilities'; /** * Create the state required to render TreeItem. @@ -15,19 +19,14 @@ import { useEventCallback } from '@fluentui/react-utilities'; * @param props - props from this instance of TreeItem * @param ref - reference to root HTMLElement of TreeItem */ -export const useTreeItem_unstable = (props: TreeItemProps, ref: React.Ref): TreeItemState => { +export const useTreeItem_unstable = (props: TreeItemProps, ref: React.Ref): TreeItemState => { const treeItemState = useBaseTreeItem_unstable(props, ref); - const { expandIcon, iconBefore, iconAfter, actions, badges } = props; + const { expandIcon, iconBefore, iconAfter, actions, badges, groupper } = props; const { dir } = useFluent_unstable(); const expandIconRotation = treeItemState.open ? 90 : dir !== 'rtl' ? 0 : 180; + const groupperProps = useFocusableGroup(); - // prevent default of a click from actions to ensure it doesn't open the treeitem - const handleActionsClick = useEventCallback((event: React.MouseEvent) => { - if (isResolvedShorthand(actions)) { - actions.onClick?.(event); - } - event.preventDefault(); - }); + const actionsRef = React.useRef(null); return { ...treeItemState, @@ -38,7 +37,35 @@ export const useTreeItem_unstable = (props: TreeItemProps, ref: React.Ref { + // if click event comer from actions, ignore it + if (actionsRef.current && actionsRef.current?.contains(event.target as Node)) { + return; + } + // if click event comes from a portal, e.g: MenuItem click, ignore it + if (!event.currentTarget.contains(event.target as Node)) { + return; + } + treeItemState.root.onClick?.(event); + }), + onKeyDown: useEventCallback(event => { + if (event.key === Enter) { + // if Enter keydown event comes from actions, ignore it + if (actionsRef.current && actionsRef.current.contains(event.target as Node)) { + return; + } + } + treeItemState.root.onKeyDown?.(event); + }), }, + groupper: resolveShorthand(groupper, { + required: true, + defaultProps: groupperProps as ExtractSlotProps>, + }), iconBefore: resolveShorthand(iconBefore, { defaultProps: { 'aria-hidden': true }, }), @@ -48,7 +75,7 @@ export const useTreeItem_unstable = (props: TreeItemProps, ref: React.Ref, + children: , 'aria-hidden': true, }, }), @@ -57,12 +84,10 @@ export const useTreeItem_unstable = (props: TreeItemProps, ref: React.Ref = { iconAfter: 'fui-TreeItem__iconAfter', actions: 'fui-TreeItem__actions', badges: 'fui-TreeItem__badges', + groupper: 'fui-TreeItem__groupper', }; const treeItemTokens = { @@ -45,24 +46,22 @@ const useRootStyles = makeStyles({ }, ':focus': { [`& .${treeItemClassNames.actions}`]: { - display: 'flex', opacity: '1', position: 'relative', - width: 'unset', - height: 'unset', - right: 'unset', + }, + }, + ':focus-within': { + [`& .${treeItemClassNames.actions}`]: { + opacity: '1', + position: 'relative', }, }, ':hover': { color: tokens.colorNeutralForeground2Hover, backgroundColor: tokens.colorSubtleBackgroundHover, [`& .${treeItemClassNames.actions}`]: { - display: 'flex', opacity: '1', position: 'relative', - width: 'unset', - height: 'unset', - right: 'unset', }, [`& .${treeItemClassNames.expandIcon}`]: { color: tokens.colorNeutralForeground3Hover, @@ -75,6 +74,11 @@ const useRootStyles = makeStyles({ display: 'none', }, }, + ':focus-within': { + [`& .${treeItemClassNames.badges}`]: { + display: 'none', + }, + }, ':hover': { [`& .${treeItemClassNames.badges}`]: { display: 'none', @@ -184,16 +188,22 @@ const useBadgesStyles = makeStyles({ */ const useActionsStyles = makeStyles({ base: { + display: 'flex', opacity: '0', position: 'absolute', - width: 0, - height: 0, right: 0, + top: 0, marginLeft: 'auto', ...shorthands.padding(0, tokens.spacingHorizontalXS), }, }); +export const expandIconInlineStyles = { + 90: { transform: `rotate(90deg)` }, + 0: { transform: `rotate(0deg)` }, + 180: { transform: `rotate(180deg)` }, +} as const; + /** * Apply styling to the TreeItem slots based on the state */ diff --git a/packages/react-components/react-tree/stories/Tree/TreeActions.stories.tsx b/packages/react-components/react-tree/stories/Tree/TreeActions.stories.tsx index ec0814ae73fab..a5403d8fe265b 100644 --- a/packages/react-components/react-tree/stories/Tree/TreeActions.stories.tsx +++ b/packages/react-components/react-tree/stories/Tree/TreeActions.stories.tsx @@ -1,12 +1,25 @@ import * as React from 'react'; import { Tree, TreeItem } from '@fluentui/react-tree'; import { Edit20Regular, MoreHorizontal20Regular } from '@fluentui/react-icons'; -import { Button } from '@fluentui/react-components'; +import { Button, Menu, MenuItem, MenuList, MenuPopover, MenuTrigger } from '@fluentui/react-components'; const RenderActions = () => ( <>