From ab386c32bd054c311360c7556a1c25785cdfe0a2 Mon Sep 17 00:00:00 2001 From: Bernardo Sunderhus Date: Mon, 9 Jan 2023 09:41:05 -0300 Subject: [PATCH] feat(react-tree): Actions positioning and behaviour (#26113) * feat(react-tree): properly position actions * chore: updates API * chore: fix tests and groupper className * Update packages/react-components/react-tree/src/components/TreeItem/useTreeItem.tsx Co-authored-by: ling1726 * chore: ensures functionality with Menu * chore: adds role presentation to groupper Co-authored-by: ling1726 --- .../react-tree/etc/react-tree.api.md | 13 ++-- .../react-components/react-tree/package.json | 1 + .../BaseTreeItem/BaseTreeItem.types.ts | 11 +-- .../BaseTreeItem/useBaseTreeItem.ts | 57 +++++++------- .../src/components/Tree/Tree.types.ts | 17 +++-- .../src/components/TreeItem/TreeItem.test.tsx | 6 ++ .../src/components/TreeItem/TreeItem.types.ts | 22 +++--- .../__snapshots__/TreeItem.test.tsx.snap | 22 ++++-- .../components/TreeItem/renderTreeItem.tsx | 18 +++-- .../src/components/TreeItem/useTreeItem.tsx | 75 +++++++++++++++---- .../components/TreeItem/useTreeItemStyles.ts | 43 ++++++++--- .../stories/Tree/TreeActions.stories.tsx | 29 +++++-- 12 files changed, 202 insertions(+), 112 deletions(-) diff --git a/packages/react-components/react-tree/etc/react-tree.api.md b/packages/react-components/react-tree/etc/react-tree.api.md index 468b8d734a330a..0aeb610ba2e3cc 100644 --- a/packages/react-components/react-tree/etc/react-tree.api.md +++ b/packages/react-components/react-tree/etc/react-tree.api.md @@ -6,8 +6,6 @@ /// -import type { ARIAButtonElement } from '@fluentui/react-aria'; -import type { ARIAButtonSlotProps } from '@fluentui/react-aria'; import type { ComponentProps } from '@fluentui/react-utilities'; import type { ComponentState } from '@fluentui/react-utilities'; import { ContextSelector } from '@fluentui/react-context-selector'; @@ -30,7 +28,7 @@ export type BaseTreeItemProps = ComponentProps; // @public (undocumented) export type BaseTreeItemSlots = { - root: Slot; + root: Slot<'div'>; }; // @public @@ -81,10 +79,13 @@ export type TreeItemSlots = BaseTreeItemSlots & { iconAfter?: Slot<'span'>; badges?: Slot<'span'>; actions?: Slot<'span'>; + groupper: NonNullable>; }; // @public -export type TreeItemState = ComponentState & BaseTreeItemState; +export type TreeItemState = ComponentState & BaseTreeItemState & { + keepActionsOpen: boolean; +}; // @public (undocumented) export type TreeProps = ComponentProps & { @@ -109,7 +110,7 @@ export type TreeState = ComponentState & TreeContextValue & { }; // @public -export const useBaseTreeItem_unstable: (props: BaseTreeItemProps, ref: React_2.Ref) => BaseTreeItemState; +export const useBaseTreeItem_unstable: (props: BaseTreeItemProps, ref: React_2.Ref) => BaseTreeItemState; // @public export const useBaseTreeItemStyles_unstable: (state: BaseTreeItemState) => BaseTreeItemState; @@ -121,7 +122,7 @@ export const useTree_unstable: (props: TreeProps, ref: React_2.Ref) export const useTreeContext_unstable: (selector: ContextSelector) => T; // @public -export const useTreeItem_unstable: (props: TreeItemProps, ref: React_2.Ref) => TreeItemState; +export const useTreeItem_unstable: (props: TreeItemProps, ref: React_2.Ref) => TreeItemState; // @public export const useTreeItemStyles_unstable: (state: TreeItemState) => TreeItemState; diff --git a/packages/react-components/react-tree/package.json b/packages/react-components/react-tree/package.json index 5b21cee332a8d7..c3bda21dce8fd3 100644 --- a/packages/react-components/react-tree/package.json +++ b/packages/react-components/react-tree/package.json @@ -35,6 +35,7 @@ "@fluentui/react-shared-contexts": "^9.1.4", "@fluentui/react-aria": "^9.3.4", "@fluentui/react-tabster": "^9.3.5", + "@fluentui/react-portal": "^9.0.15", "@fluentui/keyboard-keys": "^9.0.1", "@fluentui/react-theme": "^9.1.5", "@fluentui/react-utilities": "^9.3.1", 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 15a60b4e5c5ec5..c6a016bf475b4e 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 1cd1af387ac388..86b3bf3f8be89f 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 fb1bdd91c67acd..4e26e1fea3a963 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.test.tsx b/packages/react-components/react-tree/src/components/TreeItem/TreeItem.test.tsx index 79512cdfa7064c..d61afb40fb3480 100644 --- a/packages/react-components/react-tree/src/components/TreeItem/TreeItem.test.tsx +++ b/packages/react-components/react-tree/src/components/TreeItem/TreeItem.test.tsx @@ -3,11 +3,17 @@ import { render } from '@testing-library/react'; import { TreeItem } from './TreeItem'; import { isConformant } from '../../testing/isConformant'; import { TreeItemProps } from './TreeItem.types'; +import { treeItemClassNames } from './useTreeItemStyles'; describe('TreeItem', () => { isConformant({ Component: TreeItem, displayName: 'TreeItem', + // primarySlot: 'groupper', + getTargetElement(renderResult, attr) { + return renderResult.container.querySelector(`.${treeItemClassNames.root}`) ?? renderResult.container; + }, + disabledTests: ['component-has-static-classnames-object'], testOptions: { 'has-static-classnames': [ { 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 4fe814c7abf871..60a8e04a24efd1 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>; }; /** @@ -45,4 +35,10 @@ export type TreeItemProps = ComponentProps> & BaseTreeIte /** * State used in rendering TreeItem */ -export type TreeItemState = ComponentState & BaseTreeItemState; +export type TreeItemState = ComponentState & + BaseTreeItemState & { + /** + * boolean indicating that actions should remain open due to focus on some portal + */ + keepActionsOpen: boolean; + }; diff --git a/packages/react-components/react-tree/src/components/TreeItem/__snapshots__/TreeItem.test.tsx.snap b/packages/react-components/react-tree/src/components/TreeItem/__snapshots__/TreeItem.test.tsx.snap index 0a31ffa7e10d45..d48c13a7ea3e09 100644 --- a/packages/react-components/react-tree/src/components/TreeItem/__snapshots__/TreeItem.test.tsx.snap +++ b/packages/react-components/react-tree/src/components/TreeItem/__snapshots__/TreeItem.test.tsx.snap @@ -2,14 +2,20 @@ exports[`TreeItem renders a default state 1`] = `
-
- Default TreeItem -
+
+ Default TreeItem +
+
`; 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 fc9a376575b507..6ebab4c66f9687 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 f30bcd068fc151..4bd12049fd7ac7 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,15 @@ import * as React from 'react'; import { isResolvedShorthand, resolveShorthand } from '@fluentui/react-utilities'; -import type { TreeItemElement, TreeItemProps, TreeItemState } from './TreeItem.types'; 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 { useBaseTreeItem_unstable } from '../BaseTreeItem/index'; +import { Enter } from '@fluentui/keyboard-keys'; +import { useMergedRefs } from '@fluentui/react-utilities'; +import { elementContains } from '@fluentui/react-portal'; +import type { TreeItemProps, TreeItemState } from './TreeItem.types'; /** * Create the state required to render TreeItem. @@ -15,22 +20,52 @@ 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 { dir } = useFluent_unstable(); + const { expandIcon, iconBefore, iconAfter, actions, badges, groupper } = props; + const { dir, targetDocument } = 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); + const actionsRef = React.useRef(null); + + const handleClick = useEventCallback((event: React.MouseEvent) => { + // if click event originates from actions, ignore it + if (actionsRef.current && elementContains(actionsRef.current, event.target as Node)) { + return; } - event.preventDefault(); + treeItemState.root.onClick?.(event); }); + const handleKeyDown = useEventCallback((event: React.KeyboardEvent) => { + if (event.key === Enter) { + // if Enter keydown event comes from actions, ignore it + if (actionsRef.current && elementContains(actionsRef.current, event.target as Node)) { + return; + } + } + treeItemState.root.onKeyDown?.(event); + }); + + const [keepActionsOpen, setKeepActionsOpen] = React.useState(false); + + // Listens to focusout event on the document to ensure treeitem actions visibility on portal scenarios + // TODO: find a better way to ensure this behavior + React.useEffect(() => { + if (actionsRef.current) { + const handleFocusOut = (event: FocusEvent) => { + setKeepActionsOpen(elementContains(actionsRef.current, event.relatedTarget as Node)); + }; + targetDocument?.addEventListener('focusout', handleFocusOut, { passive: true }); + return () => { + targetDocument?.removeEventListener('focusout', handleFocusOut); + }; + } + }, [targetDocument]); + return { ...treeItemState, + keepActionsOpen, components: { ...treeItemState.components, expandIcon: 'span', @@ -38,7 +73,20 @@ export const useTreeItem_unstable = (props: TreeItemProps, ref: React.Ref, + children: , 'aria-hidden': true, }, }), @@ -59,10 +107,7 @@ 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,26 @@ 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), }, + open: { + opacity: '1', + position: 'relative', + }, }); +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 */ @@ -223,6 +237,8 @@ export const useTreeItemStyles_unstable = (state: TreeItemState): TreeItemState state.root.className, ); + state.groupper.className = mergeClasses(treeItemClassNames.groupper, state.groupper.className); + state.root.style = { ...state.root.style, [treeItemTokens.level]: level, @@ -256,7 +272,12 @@ export const useTreeItemStyles_unstable = (state: TreeItemState): TreeItemState } if (actions) { - actions.className = mergeClasses(treeItemClassNames.actions, actionsStyles.base, actions.className); + actions.className = mergeClasses( + treeItemClassNames.actions, + actionsStyles.base, + state.keepActionsOpen && actionsStyles.open, + actions.className, + ); } if (badges) { badges.className = mergeClasses(treeItemClassNames.badges, badgesStyles.base, badges.className); 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 ec0814ae73fabe..cdd2cfbed1683b 100644 --- a/packages/react-components/react-tree/stories/Tree/TreeActions.stories.tsx +++ b/packages/react-components/react-tree/stories/Tree/TreeActions.stories.tsx @@ -1,14 +1,29 @@ 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 = () => ( - <> -