From 3a334861615db618115d96dec7e124ba32b42136 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Sun, 17 Jan 2021 14:38:25 +0000 Subject: [PATCH 01/44] Add new template sections --- specs/Menu.md | 150 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 137 insertions(+), 13 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index f89225f0b32438..d92ddedc20a70c 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -1,26 +1,150 @@ # Menu -## TODOS +## Background -- Get reviews -- Write summary at end -- Checkin to github -- Write prototype -- Finalize structure -- Finalize API -- Final review +### Definition -## Menu Basics +This spec defines the default function of a `Menu` as an interactive component that displays a list of options that can be represented by a range possible states. Possible variants are defined in [the relevant section](##variants) -A menu is a way of displaying items that a user may be able to interact with. Menus vary quite a bit based on implementation and usage. Some are horizontal, some have collapsable sections, and others are just a list. Likewise menu items are equally varied. Some may just convey information but cannot be interacted with like headers or dividers while others are either buttons, links, or expand into a submenu. +The `Menu` should be displayed on a temporary surface that interrupts the normal flow of content. The temporary surface should be triggered by an external user action such as (but not limited to) a click on a button or other UI control. -The w3 specifications for a menu suggest that it is only for user actions, not user input. Most varieties of menus interpret this as being for navigation. +The interactions that result in the dismiss/removal of the `Menu` component should be configurable. -Menus are often combined with a popover or similar floating component so the menu appears over other items and is subsequently dismissed. +## Prior art + +As a part of the spec definitions in Fluent UI, a research effort has been made through [Open UI](https://open-ui.org/). The current research proposal is available as an open source contribution undergoing review ([LINK to research proposal](https://github.com/WICG/open-ui/pull/249)) + +### Comparison of `@fluentui/react` and `@fluentui/react-northstar` + +## Variants + +### Nested nenus + +A `Menu` should be able to trigger an additional instance of itself as a part of one or more of its options. The nested `Menu` component should have the same functional capabilities as the root `Menu` component. + +The actions that trigger the the nested `Menu` should be be consistent with the actions that can trigger any root `Menu` from a similar UI control. + +We advise that no more than two nested `Menu` components be used, but this spec does not functionally apply that constrain to the implementation of the `Menu` component. + +### Selection state + +A `Menu` should be able to track and represent the selection state of all or some of its options if required. + +When an options is associated with a selection state. The `Menu`, either root or nested, should control its dismiss behaviour accordingly based on configuration. + +### Sections + +A `Menu` can be partitioned into sections using visible dividers in its list of options. Each section can contain a heading title that announces or briefly describes the options in the particular section + +### Secondary text + +An option of a `Menu` component should be able to declare additional secondary text that can provide additional context describing the option or its usage. + +For example a secondary text can be a label that shows a keyboard shortcut that will perform an equivalent action of the option of the `Menu` component. + +### Split option with nesting + +An option of a `Menu` component can trigger a nested `Menu` component and also perform its default action by splitting the option into two interactable areas that handle each action separately. + +### Disabled option(s) + +All options in a `Menu` component can be disabled and should provide a visible indication for this purpose. User interaction should be defined for disabled options + +### Scrollable + +A `Menu` should display a vertical scrollbar when the number of options exceeds the height of the component + +### Standalone/No surface + +A `Menu` can be used without the temporary popup surface and its trigger. This will allow `Menu` components to be permanent page content or used in custom surfaces with a wider range of UI components. + +### Custom content + +``` +This variant is still a work in progress and needs additional thought +``` + +## API + +The `Menu` should implement a `children` based API as is the standard across all the surveyed alternatives as a part of Open UI research in [Prior Art](#prior-art). The component will leverage the use of `context` in the interaction and data flows of child components. + +Sample usages will be give in the following section of this document [Sample code](#sample-code) + +### Menu + +The root level component serves as a simplified interface (sugar) for popup positioning and triggering. + +### MenuList + +This component is used internally by `Menu` and manages the context and layout its items. + +`MenuList` can also be used separately as the standalone variant of the `Menu`, since it should not control popup positioning or triggers. It is the only component in the API that can be used standalone. Envisioned to be used with more complex popup or trigger scenarios where the `Menu` component does not provide enough control for these situations. + +### MenuSection + +Creates a section inside a `MenuList`, setting up header layout and dividers between `MenuItems` ### MenuItem -Most menus have a concept of a specific menuitem. It is an integral part of the menu and will have some of its own sections throughout. +As the name infers + +### SubMenu + +Creates a `Menu` component with `MenuItem` trigger and handles the positioning of the nested menu. + +## Sample code + +### Default Menu + +```typescript +const trigger = + +const menu = ( + + + + + +) +``` + +### Sections and submenus + +```typescript +const trigger = + +const menu = ( + + + + + + + + + + + + + +) +``` + +### Standlone + +```typescript +const trigger = + +const menu = ( + + + + + +) +``` + +## Behaviours ## References From b97ba688e3c6acaa4c2370d121dedd0628d7f9aa Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Sun, 17 Jan 2021 14:51:58 +0000 Subject: [PATCH 02/44] Add behaviour sections --- specs/Menu.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/specs/Menu.md b/specs/Menu.md index d92ddedc20a70c..e716c7a0559b57 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -146,6 +146,18 @@ const menu = ( ## Behaviours +### Menu trigger + +### Submenu trigger/navigation + +### Menu navigation + +### Split button MenuItem + +### Checkbox MenuItem + +### Radio MenuItem + ## References [CodeSandbox of implemented versions](https://codesandbox.io/s/menuexamples-uybsr) From 57309d4e925bd8c8fe54950d8c45eb47716c0372 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Mon, 18 Jan 2021 20:46:35 +0000 Subject: [PATCH 03/44] Add comparison --- specs/Menu.md | 264 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 263 insertions(+), 1 deletion(-) diff --git a/specs/Menu.md b/specs/Menu.md index e716c7a0559b57..385656441faab3 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -14,7 +14,267 @@ The interactions that result in the dismiss/removal of the `Menu` component shou As a part of the spec definitions in Fluent UI, a research effort has been made through [Open UI](https://open-ui.org/). The current research proposal is available as an open source contribution undergoing review ([LINK to research proposal](https://github.com/WICG/open-ui/pull/249)) -### Comparison of `@fluentui/react` and `@fluentui/react-northstar` +## Comparison of `@fluentui/react` and `@fluentui/react-northstar` + +All mentions of v7 or v8 == `@fluentui/react` +All mentions of v0 == `@fluentui/react-northstar` + +The most relevant comparison that can be achieved between the two libraries is between `ContextualMenu` in v7 and a combination of `Menu`, `Popup` and `ToolbarItem` in v0. + +v0 suffers from a consistency issue that the control used in `Menu` and the menu variant of `ToolbarItem` are not actually the same component and have different behaviour. However, semantically for the purposes of this spec, they represet the same control that will be implemented. + +Note that the below code samples are not meant to be complete, but to highlight differences between the two libraries. Please refer to official docsites for actual API references. + +### Positioning + +`ContextualMenu` in v7 is a component that also exposes the API to control the positioning of the temporary popup surface that the Menu is rendered on. This aspect of the v7 component should be compared with the `Popup` component in v0 since the v0 `Menu` is created as a standalone component with no positioning properties. + +v0 uses the [OSS Popper.js library](https://popper.js.org/) while v7 uses a component based implementation `CalloutContent`. Below we provide the results of testing common positioning boundary/edge cases between the two. + +// TODO compare boudary/edge cases in positioning + +- flip +- prevent overflow +- offset from reference +- tethering + +### Trigger vs target + +The v7 `ContextualMenu` has a prop `target` which is intended to be a ref to the DOM element that the positioning logic anchors to. The usage of this prop requires the visibility state of the component to be controlled using React state by the consumer. The same prop exists on the v0 `Popup` component that is intended to perform the same function. + +```typescript +const buttonRef = React.useRef( +const [open] = React.useState(false); const menu = ( - - - - - + + + + + + + +) +``` + +### Selection + +```typescript +const trigger = +const [selectedItems, setSelectedItems] = React.useState([]); + +// basic checkbox example +const menuCheckbox = ( + + + + + +) + +// leverage MenuSection for different selection groups +const menuSelectableSections = ( + + + + + + + + + + + + ) ``` From 94a58d544c41cfee671cea7b51bacb2091166031 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Mon, 18 Jan 2021 23:24:57 +0000 Subject: [PATCH 05/44] Add behaviour spec --- specs/Menu.md | 758 ++++---------------------------------------------- 1 file changed, 59 insertions(+), 699 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index 809e0684e80040..d41fb114b0408d 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -452,722 +452,82 @@ const menuSelectableSections = ( ## Behaviours -### Menu trigger +### Menu open/dismiss + +A menu can be triggered by the following user interactions on the triggering/anchor element. Not all interactions should be supported at the same time, but the component must be able to support combinations of the below interactions. + +As a general rule, once the menu is closed the focus should return to the triggering element once the menu is closed unless the interaction would involve another focusable element. + +| Type | Action | Result | Details | +| -------- | ---------- | ------- | ---------------------------------------------------------------------- | +| Mouse | Click | Open | Click on the trigger element | +| Mouse | Hover | Open | Hover over the trigger element with delay | +| Mouse | LongPress | Open | MouseDown with delay | +| Keyboard | Enter | Open | Focus on trigger element and press Enter | +| Keyboard | Space | Open | Focus on trigger element and press Space | +| Keyboard | Shift+F10 | Open | Focus on trigger element to open context menu (i.e. right click) | +| Keyboard | ArrowDown | Open | Focus on trigger element. Used in menu buttons | +| Mouse | Click | Dismiss | Click anywhere outside the component | +| Mouse | Click | Dismiss | Click on the trigger while the menu is open | +| Mouse | MouseLeave | Dismiss | Mouse leaves the component after a delay | +| Keyboard | Esc | Dismiss | Closes the menu and focuses on the triggering element | +| Keyboard | Tab | Dismiss | Closes the menu and all submenus, focus moves to next element in order | ### Submenu trigger/navigation -### Menu navigation +A submenu can be triggered by the following user interactions on the triggering menu item. Not all interactions should be supported at the same time, but the component must be able to support combinations of the below interactions. -### Split button MenuItem +As a general rule, once a submenu is dismissed without dismissing the menu, the focus should revert to the triggering menu item unless the interaction involves another focusable UI component. -### Checkbox MenuItem +| Type | Action | Result | Details | +| -------- | ---------- | ------- | ---------------------------------------------------------------------- | +| Mouse | Click | Open | Click the menu item that contains a submenu | +| Mouse | Hover | Open | Hover over the menu item that contains a submenu with delay | +| Keyboard | Enter | Open | Focus on triggering menu item | +| Keyboard | Space | Open | Focus on triggering menu item | +| Keyboard | ArrowRight | Open | Focus on triggering menu item | +| Mouse | Click | Dismiss | Click on an item in the submenu | +| Mouse | Click | Dismiss | Click on a UI element that is not the submenu | +| Mouse | MouseLeave | Dismiss | Mouse leaves the submenu or its triggering menu item after delay | +| Keyboard | ArrowLeft | Dismiss | Closes the submenu and focuses on the triggering menu item | +| Keyboard | Esc | Dismiss | Closes the submenu and focuses on the triggering menu item | +| Keyboard | Tab | Dismiss | Closes the menu and all submenus, focus moves to next element in order | -### Radio MenuItem +### Split button MenuItem submenu -## References +All of the above Mouse events in the [previous section](#submenu-trigger/navigation) should apply to the part of the split button that is intended to open a submenu. -[CodeSandbox of implemented versions](https://codesandbox.io/s/menuexamples-uybsr) - -- [Aria practices](https://www.w3.org/TR/wai-aria-1.1/#menu) -- [Fabric](https://developer.microsoft.com/en-us/fabric#/controls/web/contextualmenu) -- [Stardust](https://microsoft.github.io/fluent-ui-react/components/menu/definition) -- [Material-ui](https://material-ui.com/components/menus/) -- [Chakra-ui](https://chakra-ui.com/menu) -- [AntDesign](https://ant.design/components/menu/) -- [Jquery-ui](https://jqueryui.com/menu/) -- [Carbon-ui](https://www.carbondesignsystem.com/components/overflow-menu/code) -- [Fast-DNA](https://github.com/microsoft/fast-dna/blob/master/packages/fast-components-react-base/src/context-menu/context-menu.tsx) -- [BluePrintJs](https://blueprintjs.com/docs/#core/components/menu) - -### Notes from references: - -- Most render children, rather than passing a list of items in as props -- Menu is usually a `
    ` with `
  • ` as children -- Some are popout menus rather than just being a list, but most do not have an explicit popout. -- Most have a concept of submenus, either explicitly or as a result of nested lists. -- The line between list and menu is fuzzy. - -## API - -### Prop comparison \(Stardust vs Fabric\) - -_Note:_ I've skipped some of the boilerplate props like className. - -#### Fabric - -##### Menu - -| Prop Name | Type | Description | -| ------------------------ | ----------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------- | -| target | Target | Target for positioning | -| directionalHint | DirectionalHint | How the menu should try to align itself | -| gapSpace | number | Distance between menu and target | -| beakWidth | number | Width of the beak | -| useTargetWidth | 'boolean' | If the menu should match the targets width | -| useTargetAsMinWidth | boolean | see above | -| bounds | IRectangle \| (target?: targetWindow?: Window)=>< IRectangle \| undefined | The space that the menu can appear in | -| directionalHintForRTL | DirectionalHint | -| gapSpace | number | The space between the menu and its target | -| beakWidth? | number | The size of the beak that points | -| isBeakVisible? | boolean | | -| coverTarget? | boolean | If the menu should cover its target | -| alignTargetEdge? | boolean | If the menu should be aligned. | -| items | IContextualMenuItem[] | The list of items to be rendered | -| labelElementId? | string | The id of a label element for the menu | -| shouldFocusOnMount? | boolean | | -| shouldFocusOnContainer? | boolean | | -| onDismiss? | Event | callback for what should happen when menu is dismissed | Not needed | -| onItemClick? | (ev?: React.MouseEvent \| React.KeyboardEvent, item?: IContextualMenuItem) => boolean \| void | | -| isSubMenu? | boolean | whether or not the menu is owned by another menu | -| id? | string | | -| ariaLabel? | string | | -| doNotLayer? | boolean | | -| directionalHintFixed? | boolean | | -| onMenuOpened? | (contextualMenu?: IContextualMenuProps) => void | callback for when the menu is opened | -| onMenuDismissed? | (contextualMenu?: IContextualMenuProps) => void | callback for when the menu is completly dismissed | -| calloutProps? | ICalloutProps | passthrough props to the callout | -| title? | string | | -| getMenuClassNames? | (theme: ITheme, className?: string) => IContextualMenuClassNames | styling function | -| onRenderSubMenu? | IRenderFunction | a different way of rendering the submenu | -| onRenderMenuList? | IRenderFunction | a different way of rendering the list of items | -| subMenuHoverDelay? | number | how long it should take for the submenu to open | -| contextualMenuItemAs? | React.ComponentClass | React.StatelessComponent | a different way of rendering each item | -| focusZoneProps? | IFocusZoneProps | passthrough props to control how the focus zone works | -| hidden? | boolean | if the menu should be hidden and not destroyed | -| shouldUpdateWhenHidden? | boolean | | -| delayUpdateFocusOnHover? | boolean | if focus should go to the menu items when they are hovered | - -##### Menuitem - -| Prop Name | Type | Description | -| ------------------------ | ------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| ariaLabel | string | | -| canCheck | boolean | Whether or not this menu item can be checked | -| checked | boolean | Whether or not this menu item is currently checked. | -| className | string | Additional CSS class to apply to the menu item. | -| componentRef | IRefObject | Optional callback to access the IContextualMenuRenderItem interface. This will get passed down to ContextualMenuItem. | -| customOnRenderListLength | number | When rendering a custom menu component that is passed in, the component might also be a list of elements. We want to keep track of the correct index our menu is using based off of the length of the custom list. It is up to the user to increment the count for their list. | -| data | any | Any custom data the developer wishes to associate with the menu item. | -| disabled | boolean | false | Whether the menu item is disabled | -| href | string | Navigate to this URL when the menu item is clicked. | -| iconProps | IIconProps | Props for the Icon. | -| itemProps | Partial | Optional IContextualMenuItemProps overrides to customize behaviors such as item styling via styles. | -| itemType | ContextualMenuItemType | -| key | string | Unique id to identify the item | -| keytipProps | IKeytipProps | Keytip for this contextual menu item | -| onClick | (ev?: React.MouseEvent | React.KeyboardEvent, item?: IContextualMenuItem) => boolean | void | Callback for when the menu item is invoked. If ev.preventDefault() is called in onClick, the click will not close the menu. Returning true will dismiss the menu even if ev.preventDefault() was called. | -| onMouseDown | (item: IContextualMenuItem, event: React.MouseEvent) => void | A function to be executed on mouse down. This is executed before an onClick event and can be used to interrupt native on click events as well. The click event should still handle the commands. This should only be used in special cases when react and non-react are mixed. | -| onRender | (item: any, dismissMenu: (ev?: any, dismissAll?: boolean) => void) => React.ReactNode | Method to custom render this menu item. For keyboard accessibility, the top-level rendered item should be a focusable element (like an anchor or a button) or have the data-is-focusable property set to true. The function receives a function that can be called to dismiss the menu as a second argument. This can be used to make sure that a custom menu item click dismisses the menu. | -| onRenderIcon | IRenderFunction | Custom render function for the menu item icon | -| primaryDisabled | boolean | false | If the menu item is a split button, this prop disables purely the primary action of the button. | -| rel | string | Link relation setting when using href. If target is \_blank, rel is defaulted to a value to prevent clickjacking. | -| role | string | Optional override for the menu button's role. Defaults to menuitem or menuitemcheckbox. | -| secondaryText | string | Seconday description for the menu item to display | -| sectionProps | IContextualMenuSection | Properties to apply to render this item as a section. This prop is mutually exclusive with subMenuProps. | -| split | boolean | false | Whether or not this menu item is a splitButton. | -| subMenuProps | IContextualMenuProps | Properties to apply to a submenu to this item. The ContextualMenu will provide default values for target, onDismiss, isSubMenu, id, shouldFocusOnMount, directionalHint, className, and gapSpace, all of which can be overridden. | -| submenuIconProps | IIconProps | Props for the Icon used for the chevron. | -| target | string | Target window when using href. | -| text | string | Text description for the menu item to display | -| title | string | Optional title for displaying text when hovering over an item. | - -#### Stardust - -##### Menu - -| Prop Name | Type | Description | -| ------------------ | -------------------------------------------------------- | ------------------------------------------------------- | -| accessibility | Accessibility | Determines how the keyboard interacts with the menu | -| activeIndex | number \| string | Index of currently active item | -| defaultActiveIndex | number \| string | Default index of currently active item | -| fluid | boolean | whether or not the menu should fill its container | -| iconOnly | boolean | whether or not the menu only has icons | Not needed | -| items | ShorthandCollection\ | The items that are to be rendered in the menu | -| onItemClick | ComponentEventHandler\ | Callback for what should happen when an item is clicked | -| pills | boolean | If the items should have rounded edges | not needed | -| pointing | boolean \| 'start' \| 'end' | The direction in which an item should point towards | -| primary | boolean | Determines if the menu is primary or not, changes color | -| secondary | boolean | see above but secondary | -| underlined | boolean | If the items should be underlined | -| vertical | boolean | How the menu should render, vertically or horizontally | -| submenu | boolean | If the menu is a submenu or not | -| indicator | ShorthandValue\ | How the submenu icon should look | - -##### Menuitem - -| Prop Name | Type | Description | -| ---------------- | ------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------ | -| accessibility | "menuItemBehavior" any | Accessibility behavior if overridden by the user. | -| active | false \| boolean | A menu item can be active. | -| animation | AnimationProp | Generic animation property that can be used for applying different theme animations. | -| as | "a" React.ElementType | An element type to render as (string or component). | -| className | string | Additional CSS class name(s) to apply. | -| content | ReactNode | Shorthand for primary content. | -| defaultMenuOpen | false \| boolean | Default menu open | -| design | ComponentDesign | | -| disabled | false \| boolean | A menu item can show it is currently unable to be interacted with. | -| icon S | ShorthandValue | Name or shorthand for Menu Item Icon | -| iconOnly | false \| boolean | A menu may have just icons. | -| inSubmenu | false \| boolean | Indicates whether the menu item is part of submenu. | -| index | number | MenuItem index inside Menu. | -| indicator S | ShorthandValue | Shorthand for the submenu indicator. | -| itemPosition | number | MenuItem position inside Menu (skipping separators). | -| itemsCount | number | MenuItem count inside Menu. | -| menu S | ShorthandValue | ShorthandCollection | Shorthand for the submenu. | -| menuOpen | false \| boolean | Indicates if the menu inside the item is open. | -| onActiveChanged | ComponentEventHandler | Callback for setting the current menu item as active element in the menu. | -| onBlur | ComponentEventHandler | Called after item blur. | -| onClick | ComponentEventHandler | Called on click. | -| onFocus | ComponentEventHandler | Called after user's focus. | -| onMenuOpenChange | ComponentEventHandler | Event for request to change 'open' value. | -| pills | false \| boolean | A menu can adjust its appearance to de-emphasize its contents. | -| pointing | boolean \| enum | A menu can point to show its relationship to nearby content. For vertical menu, it can point to the start of the item or to the end. | -| primary | false \| boolean | The menu item can have primary type. | -| secondary | false \| boolean | The menu item can have secondary type. | -| styles | ComponentSlotStyle | Additional CSS styles to apply to the component instance. | -| underlined | false \| boolean | Menu items can by highlighted using underline. | -| variables | any | Override for theme site variables to allow modifications of component styling via themes. | -| vertical | false \| boolean | A vertical menu displays elements vertically. | -| wrapper S | {"as":"li"} ShorthandValue | Shorthand for the wrapper component. | - -### Recommended Props - -Most props should be removed in favor of a composition model where the parent component passes in menuItems which contain content styled however they want. - -#### Menu component - -Should extend `ul` props - -| Prop Name | Prop Type | Notes | -| ----------- | ----------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| children | `li | MenuItem | Divider` | Text | -| orientation | enum: `vertical | horizontal` | Specifies how the menu is oriented. | -| onClick | `onClickHandler` | This is from the root UL props but it's worth noting that there shouldn't be a specific onMenuClick/onMenuItemClick handler. See [Behaviors](#On-Menu-Click) for more | - -#### MenuItem component - -Should extend `li` props -Most of the other props exist as HTMLAttributes like `selected`, `checked`, `onClick`. - -| Prop Name | Prop Type | Notes | -| --------- | ------------------------------------------------------------------------------------------------------- | -------------------------------- | -| children | [Flow content](https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Content_categories#Flow_content) | No content should have on clicks | - -#### SubmenuItem component - -Should extend MenuItems props but omit onClick - -| Prop Name | Prop Type | Notes | -| --------- | ------------------------------------------------------------------------------------------------------- | -------------------------------- | -| children | [Flow content](https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Content_categories#Flow_content) | No content should have on clicks | - -#### Submenu component - -| Prop Name | Prop Type | Notes | -| --------- | ------------------------------------------------------------------------------------------------------- | -------------------------------------------------- | -| children | [Flow content](https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Content_categories#Flow_content) | No content should have on clicks | -| open | boolean | whether or not this particular menu group is open. | - -#### Submenu component - -Should take in MenuProps - -#### Communication between Menu and MenuItems - -To help provide additional information to menu items and their context, the menu should implement react context. This allows for synchronization between menu items. This also allows menu groups to pass their open status down to their children to determine how the sub items are rendered. - -Similarly Submenus should control their open state through context. - -#### Discussion: - -This is a large departure from the way that both Stardust and Fabric implement menus but it is more in line with the way a lot of other frameworks menus work. Additionally I believe it gives a lot more flexibility through composition which removes some of the pressure to add many props. - -There should be a lot more discussion to see if this relaxed approach to props is appropriate. Additionally it could make SplitButton menu items difficult to implement. - -### Conversion Plan: - -#### Fabric to Fluent: - -##### Menu - -| Action to take/taken | Property transitioned? | Breaking change? | Codemod/Shim created? | -| ------------------------ | ---------------------- | ---------------- | --------------------- | -| target | ❌ | ❌ | ❌ | -| directionalHint | ❌ | ❌ | ❌ | -| gapSpace | ❌ | ❌ | ❌ | -| beakWidth | ❌ | ❌ | ❌ | -| useTargetWidth | ❌ | ❌ | ❌ | -| useTargetAsMinWidth | ❌ | ❌ | ❌ | -| bounds | ❌ | ❌ | ❌ | -| directionalHintForRTL | ❌ | ❌ | ❌ | -| gapSpace | ❌ | ❌ | ❌ | -| beakWidth? | ❌ | ❌ | ❌ | -| isBeakVisible? | ❌ | ❌ | ❌ | -| coverTarget? | ❌ | ❌ | ❌ | -| alignTargetEdge? | ❌ | ❌ | ❌ | -| items | ❌ | ❌ | ❌ | -| labelElementId? | ❌ | ❌ | ❌ | -| shouldFocusOnMount? | ❌ | ❌ | ❌ | -| shouldFocusOnContainer? | ❌ | ❌ | ❌ | -| onDismiss? | ❌ | ❌ | ❌ | -| onItemClick? | ❌ | ❌ | ❌ | -| isSubMenu? | ❌ | ❌ | ❌ | -| id? | ❌ | ❌ | ❌ | -| ariaLabel? | ❌ | ❌ | ❌ | -| doNotLayer? | ❌ | ❌ | ❌ | -| directionalHintFixed? | ❌ | ❌ | ❌ | -| onMenuOpened? | ❌ | ❌ | ❌ | -| onMenuDismissed? | ❌ | ❌ | ❌ | -| calloutProps? | ❌ | ❌ | ❌ | -| title? | ❌ | ❌ | ❌ | -| getMenuClassNames? | ❌ | ❌ | ❌ | -| onRenderSubMenu? | ❌ | ❌ | ❌ | -| onRenderMenuList? | ❌ | ❌ | ❌ | -| subMenuHoverDelay? | ❌ | ❌ | ❌ | -| contextualMenuItemAs? | ❌ | ❌ | ❌ | -| focusZoneProps? | ❌ | ❌ | ❌ | -| hidden? | ❌ | ❌ | ❌ | -| shouldUpdateWhenHidden? | ❌ | ❌ | ❌ | -| delayUpdateFocusOnHover? | ❌ | ❌ | ❌ | - -##### Menuitem - -| Action to take/taken | Property transitioned? | Breaking change? | Codemod/Shim created? | -| ------------------------ | ---------------------- | ---------------- | --------------------- | -| ariaLabel | ❌ | ❌ | ❌ | -| canCheck | ❌ | ❌ | ❌ | -| checked | ❌ | ❌ | ❌ | -| className | ❌ | ❌ | ❌ | -| componentRef | ❌ | ❌ | ❌ | -| customOnRenderListLength | ❌ | ❌ | ❌ | -| data | ❌ | ❌ | ❌ | -| disabled | ❌ | ❌ | ❌ | -| href | ❌ | ❌ | ❌ | -| iconProps | ❌ | ❌ | ❌ | -| itemProps | ❌ | ❌ | ❌ | -| itemType | ❌ | ❌ | ❌ | -| key | ❌ | ❌ | ❌ | -| keytipProps | ❌ | ❌ | ❌ | -| onClick | ❌ | ❌ | ❌ | -| onMouseDown | ❌ | ❌ | ❌ | -| onRender | ❌ | ❌ | ❌ | -| onRenderIcon | ❌ | ❌ | ❌ | -| primaryDisabled | ❌ | ❌ | ❌ | -| rel | ❌ | ❌ | ❌ | -| role | ❌ | ❌ | ❌ | -| secondaryText | ❌ | ❌ | ❌ | -| sectionProps | ❌ | ❌ | ❌ | -| split | ❌ | ❌ | ❌ | -| subMenuProps | ❌ | ❌ | ❌ | -| submenuIconProps | ❌ | ❌ | ❌ | -| target | ❌ | ❌ | ❌ | -| text | ❌ | ❌ | ❌ | -| title | ❌ | ❌ | ❌ | - -#### Stardust to Fluent - -##### Menu - -| Action to take/taken | Property transitioned? | Breaking change? | Codemod/Shim created? | -| -------------------- | ---------------------- | ---------------- | --------------------- | -| accessibility | ❌ | ❌ | ❌ | -| activeIndex | ❌ | ❌ | ❌ | -| defaultActiveIndex | ❌ | ❌ | ❌ | -| fluid | ❌ | ❌ | ❌ | -| iconOnly | ❌ | ❌ | ❌ | -| items | ❌ | ❌ | ❌ | -| onItemClick | ❌ | ❌ | ❌ | -| pills | ❌ | ❌ | ❌ | -| pointing | ❌ | ❌ | ❌ | -| primary | ❌ | ❌ | ❌ | -| secondary | ❌ | ❌ | ❌ | -| underlined | ❌ | ❌ | ❌ | -| vertical | ❌ | ❌ | ❌ | -| submenu | ❌ | ❌ | ❌ | -| indicator | ❌ | ❌ | ❌ | - -##### Menuitem - -| Action to take/taken | Property transitioned? | Breaking change? | Codemod/Shim created? | -| -------------------- | ---------------------- | ---------------- | --------------------- | -| accessibility | ❌ | ❌ | ❌ | -| active | ❌ | ❌ | ❌ | -| animation | ❌ | ❌ | ❌ | -| as | ❌ | ❌ | ❌ | -| className | ❌ | ❌ | ❌ | -| content | ❌ | ❌ | ❌ | -| defaultMenuOpen | ❌ | ❌ | ❌ | -| design | ❌ | ❌ | ❌ | -| disabled | ❌ | ❌ | ❌ | -| icon S | ❌ | ❌ | ❌ | -| iconOnly | ❌ | ❌ | ❌ | -| inSubmenu | ❌ | ❌ | ❌ | -| index | ❌ | ❌ | ❌ | -| indicator S | ❌ | ❌ | ❌ | -| itemPosition | ❌ | ❌ | ❌ | -| itemsCount | ❌ | ❌ | ❌ | -| menu S | ❌ | ❌ | ❌ | -| menuOpen | ❌ | ❌ | ❌ | -| onActiveChanged | ❌ | ❌ | ❌ | -| onBlur | ❌ | ❌ | ❌ | -| onClick | ❌ | ❌ | ❌ | -| onFocus | ❌ | ❌ | ❌ | -| onMenuOpenChange | ❌ | ❌ | ❌ | -| pills | ❌ | ❌ | ❌ | -| pointing | ❌ | ❌ | ❌ | -| primary | ❌ | ❌ | ❌ | -| secondary | ❌ | ❌ | ❌ | -| styles | ❌ | ❌ | ❌ | -| underlined | ❌ | ❌ | ❌ | -| variables | ❌ | ❌ | ❌ | -| vertical | ❌ | ❌ | ❌ | -| wrapper S | ❌ | ❌ | ❌ | - -### Notable things - -Based on my recommendations, the MenuItem ends up doing a lot of work compared to the Menu itself. The MenuItem would be responsible for managing whether or not its expanded, has a submenu, and any other state it might have. - -## DOM Structure - -### HTML DOM structure - -```HTML -
      -
    • {"Item one"}
    • -
      -
    • {"Item two"}
    • -
    • - {"Item Three"} -
        -
      • {"SubItem one"}
      • -
      • {"SubItem two"}
      • -
      -
    • -
    -``` - -### Stardust Dom Structure - -Note: Class names removed - -```HTML - -``` - -### office-ui-fabric-react DOM Structure - -_Note:_ Some long class names removed - -```HTML -
    -
      - - - - -
    -
    -``` - -### Fast-DNA DOM Structure - -```HTML - -``` - -#### Comments - -Fast-DNA is one of the only menus that doesn't use `li` elements. - -### MaterialUI - -_Note:_ Some long class names removed - -```HTML - -``` - -### AntDesign DOM Structure - -_Note:_ Some long class names removed - -```HTML -
      - -
    • - - -
    ``` - -## Recommendations - -### Recommended DOM Structure - -```HTML -
    -
    {"Item one"}
    -
    -
    {"Item two"}
    - link1 -
    - {"Item Three"} -
    - - I'm not sure if this is the right role or if it is actually a menu item. It should - probably be related to its parent's item in some way. - <--> -
    - - Note: Does not necessarily need to be part of the same dom - tree, could be floating. - <--> -
    -
    {"SubItem one"}
    -
    {"SubItem two"}
    -
    -
    -
-``` - -### Recommended React Structure - -#### Shape when used - -```TSX -return - {"Item one"} - - {"Item two"} - - {"Item two"} - - {"SubItem one"} - {"SubItem two"} - - -; -``` - -#### Menu - -```TSX -function() { - return ( - - - {props.children} - - ); -} -``` - -#### MenuItem - -```TSX -function() { - return ( - - {props.children} - ); -} -``` - -#### Submenu - -Maybe should be named SubmenuContext? -Should submenu have its own context? - -```TSX -function() { -return ( - - {props.children} - ); -} -``` - -#### SubmenuList - -```TSX -function() { - const openableContext = React.useContext(OpenableContext); - return (openableContext.open && - - {props.children} - ); -} +Keyboard interaction for the split button menu item WIP and requires input from a11y champs ``` -#### SubmenuItem - -```TSX -function() { - const openableContext = React.useContext(OpenableContext); - return ( - - {props.children} - ); -} -``` - -### Slots - -#### Menu - -Since the Menu is only rendering its children, the only slot necessary is the one for the Root `div`. The rest can be passed in as children variants. - -#### MenuItem - -Since the MenuItem is only rendering its children, the only slot necessary is the one for the Root `div`. The rest can be passed in as children. - -#### SubmenuList - -Since the SubmenuList is effectively the same as the Menu, it should have only one slot for the Root `div` and for all intents and purposes should behave exactly the same as the Menu slot does. - -#### SubmenuItem - -Since the SubmenuItem is effectively the same as the MenuItem, it should have only one slot for the Root `div` and for all intents and purposes should behave exactly the same as the MenuItem slot does. - -### Behaviors - -#### Menu - -The menu itself should be a list that renders menu items, it will provide context about its overall state but should not pass that directly into its items as props. - -##### Orientation - -The menu should provide some help for orienting its contents either vertically, like a left nav, or horizontally, like a nav bar. Ideally this will just be a prop that gets put into context so each item can decide how it should appear. It's possible that the menu should enforce direction itself. - -##### Focus - -By default Focus should go to the tab stops and it's up to each menuItem to decide if it has tabindex=0. An example of a menuItem that shouldn't be focusable would be a Divider. - -However for the menu we should provide a focus wrapper that is able to track focus using role="menuitem" to determine what item should get focused. - -The menu does not need to provide a native way to be controlled without getting focus directly. Like above, there should be a simple utility wrapper that could be written to implement this. - -##### On Menu Click - -Some implementations of menu have a general `onClick` function that is applied to all items as a way of getting more information. - -Recommendation: There is not a need to have an `onClick` that is called whenever an item is clicked. Instead the root `menu` should take an `onClick` that will fire if a child's on click does not prevent default. - -#### Menu Items - -##### On Item Click - -The on item click should be supplied individually by the consuming component - -##### Focus - -The menu item should change border and background color on focus. - -##### Hover - -The menu item should change border and background color on hover. - -An extra consideration needs to be made for forcing focus into the menu item on hover. Menus on most Microsoft desktop applications work this way. We should ensure there is a way to achieve this behavior through composition or props. - -#### Submenu Items - -A submenu item should have all of the same states and behaviors as menu items with the only difference being that submenu items lack an `onClick` callback. - -A submenu item needs to provide a way for menus to open on hover as many submenus have that behavior. - -##### Open - -A menu item should have a different state depending on if it is open or not. - -### Theming && customization - -The menu should have as few opinions on theming as possible, allowing the items to determine customizations as much as possible. There should also be a way to easily remove the theme entirely from the menu so the items can determine the look and feel. +Once the the submenu is open, the same behaviour as in the [previous section](#submenu-trigger/navigation) apply -#### Menu Tokens +### Menu keyboard navigation -None needed +Keyboard interactions required to navigate the menu. The alphanumeric match interaction does not need to be supported in all cases, but should be supported as much as possible. -#### Menu Item Tokens +| Type | Action | Result | Details | +| -------- | --------- | ----------------- | --------------------------------------------------------------------- | +| Keyboard | ArrowDown | Next Item | Roving, if on the last item got to the first | +| Keyboard | ArrowUp | Previous Item | Roving, if on the first item go to the last | +| Keyboard | Home | First item | | +| Keyboard | End | Last item | | +| Keyboard | A-Z, 0-9 | Matched item | Matches the first item that corresponds alphabetically or numerically | +| Mouse | Hover | Reveals scrollbar | If required, reveals scrollbar after delay | -| Token Name | -| -------------------------- | -| rootBackgroundColor | -| rootBorderColor | -| rootHoverBackgroundColor | -| rootFocusedBackgroundColor | -| rootHoverBorderColor | -| rootFocusedBorderColor | -| rootOutlineColor | -| rootHoverOutlineColor | -| rootFocusedOutlineColor | +### MenuItem selection -#### Submenu Item Tokens +Below are the interactions that should be supported for all menu items that are required to handle a selection state. -Extends menu item tokens +In the event that the selection method is a radio, the previous selected item must be unselected. -| Token Name | -| ----------------------- | -| rootOpenBackgroundColor | -| rootOpenBorderColor | +| Type | Action | Result | Details | +| -------- | ------ | ------ | -------------------------------------------- | +| Keyboard | Space | Toggle | Toggle the selection status of the menu item | +| Keyboard | Enter | Toggle | Toggle the selection status of the menu item | +| Mouse | Click | Toggle | Toggle the selection status of the menu item | -### Composition +## Accessibiltiy -The menu should consist of a list element, a `div`, which renders individual menu items. Each menu item consists of an element which renders with `role="menuitem"` +WIP 🚧 From 11092a07fa45d51df29efdebab4450fda27c2b8e Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Mon, 18 Jan 2021 23:26:57 +0000 Subject: [PATCH 06/44] update behaviour spec --- specs/Menu.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/specs/Menu.md b/specs/Menu.md index d41fb114b0408d..e304e5ced795c5 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -469,7 +469,10 @@ As a general rule, once the menu is closed the focus should return to the trigge | Keyboard | ArrowDown | Open | Focus on trigger element. Used in menu buttons | | Mouse | Click | Dismiss | Click anywhere outside the component | | Mouse | Click | Dismiss | Click on the trigger while the menu is open | +| Mouse | Click | Dismiss | Click on a menu item | | Mouse | MouseLeave | Dismiss | Mouse leaves the component after a delay | +| Keyboard | Enter | Dismiss | Invoked on a menu item | +| Keyboard | Space | Dismiss | Invoked on a menu item | | Keyboard | Esc | Dismiss | Closes the menu and focuses on the triggering element | | Keyboard | Tab | Dismiss | Closes the menu and all submenus, focus moves to next element in order | From 1eb477445b0f09653419e853797c230b941001bf Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Mon, 18 Jan 2021 23:28:56 +0000 Subject: [PATCH 07/44] update spec --- specs/Menu.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/specs/Menu.md b/specs/Menu.md index e304e5ced795c5..04baf7bbe4f555 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -344,7 +344,9 @@ This component is used internally by `Menu` and manages the context and layout i ### MenuSection -Creates a section inside a `MenuList`, setting up header layout and dividers between `MenuItems` +Creates a section inside a `MenuList`, setting up header layout and dividers between `MenuItems`. + +The MenuSection is also a useful component to declare different selection groups (checkbox/radio) in a `MenuList`. ### MenuItem @@ -450,6 +452,8 @@ const menuSelectableSections = ( ) ``` +// TODO positioning examples ? + ## Behaviours ### Menu open/dismiss From 2ad09fd29d915d4f6b5da9b17c455fbaeb653197 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 19 Jan 2021 10:17:52 +0000 Subject: [PATCH 08/44] minor fixes --- specs/Menu.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index 04baf7bbe4f555..294716d3c7f32a 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -4,7 +4,7 @@ ### Definition -This spec defines the default function of a `Menu` as an interactive component that displays a list of options that can be represented by a range possible states. Possible variants are defined in [the relevant section](##variants) +This spec defines the default function of a `Menu` as an interactive component that displays a list of options that can be represented by a range possible states. Possible variants are defined in [the relevant section](#variants) The `Menu` should be displayed on a temporary surface that interrupts the normal flow of content. The temporary surface should be triggered by an external user action such as (but not limited to) a click on a button or other UI control. @@ -12,12 +12,12 @@ The interactions that result in the dismiss/removal of the `Menu` component shou ## Prior art -As a part of the spec definitions in Fluent UI, a research effort has been made through [Open UI](https://open-ui.org/). The current research proposal is available as an open source contribution undergoing review ([LINK to research proposal](https://github.com/WICG/open-ui/pull/249)) +As a part of the spec definitions in Fluent UI, a research effort has been made through [Open UI](https://open-ui.org/). The current research proposal is available as an open source contribution undergoing review ([research proposal](https://github.com/WICG/open-ui/pull/249)) ## Comparison of `@fluentui/react` and `@fluentui/react-northstar` -All mentions of v7 or v8 == `@fluentui/react` -All mentions of v0 == `@fluentui/react-northstar` +- All mentions of v7 or v8 == `@fluentui/react` +- All mentions of v0 == `@fluentui/react-northstar` The most relevant comparison that can be achieved between the two libraries is between `ContextualMenu` in v7 and a combination of `Menu`, `Popup` and `ToolbarItem` in v0. From 8ac83da023b754d582013f34d971f3b866d094af Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 19 Jan 2021 10:42:02 +0000 Subject: [PATCH 09/44] issues --- specs/Menu.md | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index 294716d3c7f32a..11c3d2fb56d680 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -171,7 +171,6 @@ const toolbarItems = [ content: 'Bold', kind: 'toggle', // kind: 'radio', // for radio - index: 0, onClick: handleToggleClick, }, { @@ -179,7 +178,6 @@ const toolbarItems = [ content: 'Italic', kind: 'toggle', // kind: 'radio', // for radio - index: 1, }, ], menuOpen, @@ -278,7 +276,7 @@ const items = [ ## Variants -### Nested nenus +### Nested menus A `Menu` should be able to trigger an additional instance of itself as a part of one or more of its options. The nested `Menu` component should have the same functional capabilities as the root `Menu` component. @@ -425,9 +423,9 @@ const menuCheckbox = ( onSelectionChange={setSeelctedItems} trigger={trigger} > - - - + + + ) @@ -439,14 +437,14 @@ const menuSelectableSections = ( trigger={trigger} > - - - + + + - - - + + + ) From 8dad4de8b029f75b59874260def0d3fe0d3e3848 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 19 Jan 2021 10:47:31 +0000 Subject: [PATCH 10/44] fix typos --- specs/Menu.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index 11c3d2fb56d680..f1a73175117513 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -21,7 +21,7 @@ As a part of the spec definitions in Fluent UI, a research effort has been made The most relevant comparison that can be achieved between the two libraries is between `ContextualMenu` in v7 and a combination of `Menu`, `Popup` and `ToolbarItem` in v0. -v0 suffers from a consistency issue that the control used in `Menu` and the menu variant of `ToolbarItem` are not actually the same component and have different behaviour. However, semantically for the purposes of this spec, they represet the same control that will be implemented. +v0 suffers from a consistency issue that the control used in `Menu` and the menu variant of `ToolbarItem` are not actually the same component and have different behavior. However, semantically for the purposes of this spec, they representthe same control that will be implemented. Note that the below code samples are not meant to be complete, but to highlight differences between the two libraries. Please refer to official docsites for actual API references. @@ -188,7 +188,7 @@ const toolbarItems = [ ``` -The v7 `ContextualMenu` on the other hand, only supports the checkbox selection state implicitly. This behaviour must be controlled by consumers and uses `canCheck` and `isChecked` props: +The v7 `ContextualMenu` on the other hand, only supports the checkbox selection state implicitly. This behavior must be controlled by consumers and uses `canCheck` and `isChecked` props: ```typescript const menuProps = { @@ -288,7 +288,7 @@ We advise that no more than two nested `Menu` components be used, but this spec A `Menu` should be able to track and represent the selection state of all or some of its options if required. -When an options is associated with a selection state. The `Menu`, either root or nested, should control its dismiss behaviour accordingly based on configuration. +When an options is associated with a selection state. The `Menu`, either root or nested, should control its dismiss behavior accordingly based on configuration. ### Sections @@ -452,7 +452,7 @@ const menuSelectableSections = ( // TODO positioning examples ? -## Behaviours +## Behaviors ### Menu open/dismiss @@ -506,7 +506,7 @@ All of the above Mouse events in the [previous section](#submenu-trigger/navigat Keyboard interaction for the split button menu item WIP and requires input from a11y champs ``` -Once the the submenu is open, the same behaviour as in the [previous section](#submenu-trigger/navigation) apply +Once the the submenu is open, the same behavior as in the [previous section](#submenu-trigger/navigation) apply ### Menu keyboard navigation From 29a4ee943ee5aad2f054914de0f068c7e949f6b5 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 19 Jan 2021 11:05:58 +0000 Subject: [PATCH 11/44] add a11y sections --- specs/Menu.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/specs/Menu.md b/specs/Menu.md index f1a73175117513..e750f34846e32d 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -536,3 +536,11 @@ In the event that the selection method is a radio, the previous selected item mu ## Accessibiltiy WIP 🚧 + +### Focus management + +### Specific keyboard behaviors + +Address any specific a11y/narration keyboard behaviours that should be considered which might not fit under official specced behaviour + +### Narration From 940853627ca59d92450165ab719c5cd2c8558a3d Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 19 Jan 2021 11:29:02 +0000 Subject: [PATCH 12/44] add links to docsite --- specs/Menu.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index e750f34846e32d..4da29e4231fb96 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -16,8 +16,8 @@ As a part of the spec definitions in Fluent UI, a research effort has been made ## Comparison of `@fluentui/react` and `@fluentui/react-northstar` -- All mentions of v7 or v8 == `@fluentui/react` -- All mentions of v0 == `@fluentui/react-northstar` +- All mentions of v7 or v8 == `@fluentui/react` ([docsite](https://developer.microsoft.com/en-us/fluentui#/)) +- All mentions of v0 == `@fluentui/react-northstar` ([docsite](https://fluentsite.z22.web.core.windows.net/)) The most relevant comparison that can be achieved between the two libraries is between `ContextualMenu` in v7 and a combination of `Menu`, `Popup` and `ToolbarItem` in v0. From 84b26db75a0fce1782b50e1b03d8ac0312316693 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 19 Jan 2021 11:34:30 +0000 Subject: [PATCH 13/44] update open/dismiss --- specs/Menu.md | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index 4da29e4231fb96..3819e4e9ad0bb1 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -87,10 +87,10 @@ The v7 `ContextualMenu` is intended to be used as a controlled component. The vi The v0 `Popup` should be compared here, since the v0 `Menu` does not handle open/dismiss events. `Popup` visibility can be controlled using the `open` prop. `Popup` provides a callback prop `onOpenChange` which can be used both to open and dismiss. -However as mentioned above, the general usage of `Popup` is with an autocontrolled trigger. If a trigger is used with the `open` prop, this disables autocontrolling +As mentioned above, `Popup` implements an autocontrolled pattern which allows both controlled an uncontrolled variants to be used in its API. ```typescript -// v0 controlled ContextualMenu +// v7 controlled ContextualMenu const [showContextualMenu, setShowContextualMenu] = React.useState(false); const onShowContextualMenu = () => setShowContextualMenu(true); const onHideContextualMenu = () => setShowContextualMenu(false); @@ -101,21 +101,20 @@ const onHideContextualMenu = () => setShowContextualMenu(false); onDismiss={onHideContextualMenu />; -// v7 controlled Popup +// v0 uncontrolled Popup const [open, setOpen] = React.useState(false); setOpen(open)} + onOpenChange={(e, props: PopupProps) => {/*react on changes*/}} trigger={ const menu = ( - - - + Option 1 + Option 2 + Option 3 ) ``` @@ -378,16 +378,16 @@ const trigger = const menu = ( - + Option 1 - - - + Section Option 1 + Section Option 2 + Section Option 3 - - - + Option 1 + Option 2 + Option 3 ) @@ -401,9 +401,9 @@ const [open] = React.useState(false); const menu = ( - - - + Option 1 + Option 2 + Option 3 ) @@ -422,9 +422,9 @@ const menuCheckbox = ( onSelectionChange={setSeelctedItems} trigger={trigger} > - - - + Option 1 + Option 2 + Option 3 ) @@ -436,14 +436,14 @@ const menuSelectableSections = ( trigger={trigger} > - - - + Option 1 + Option 2 + Option 3 - - - + Option 1 + Option 2 + Option 3 ) From b998d23858448e3ee95b6c8eb119c19768abb3f8 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 19 Jan 2021 13:54:24 +0000 Subject: [PATCH 17/44] add right click --- specs/Menu.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/specs/Menu.md b/specs/Menu.md index c0c10c578006b6..f1eaeb772a8b38 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -471,7 +471,8 @@ As a general rule, once the menu is closed the focus should return to the trigge | -------- | ---------- | ------- | ---------------------------------------------------------------------- | | Mouse | Click | Open | Click on the trigger element | | Mouse | Hover | Open | Hover over the trigger element with delay | -| Mouse | LongPress | Open | MouseDown with delay | +| Mouse | LongPress | Open | MouseDown with delay, equivalent to right click for touch devices | +| Mouse | Click | Open | Right click for contextual menus | | Keyboard | Enter | Open | Focus on trigger element and press Enter | | Keyboard | Space | Open | Focus on trigger element and press Space | | Keyboard | Shift+F10 | Open | Focus on trigger element to open context menu (i.e. right click) | From 761c0a8d4dd2fe7dd0fc9d033873a1f0edeb6518 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 19 Jan 2021 14:12:48 +0000 Subject: [PATCH 18/44] Add basic dom output for comparison --- specs/Menu.md | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/specs/Menu.md b/specs/Menu.md index f1eaeb772a8b38..fc444a536aeb59 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -212,6 +212,46 @@ const menuProps = { ; ``` +### DOM output + +Below are some sample DOM outputs to compare for certain scenarios. Not all DOM attributes are reflected here, a subset have been chosen to provide easier reading and comparison. + +### Basic menu + +```html + +
    +
  • + +
  • +
  • + +
  • +
+ + + +``` + ### Custom rendering and data v7 provides render callbacks that can be used to render either the entire menu list or specific slots of menut items. Each call back provides the props avaialble to that slot and a `defaultRender` which allows to easily extend the original render, if required. From c5e4049bbb6fc94846c9dc84921fa70eaefee9da Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 19 Jan 2021 14:14:57 +0000 Subject: [PATCH 19/44] Add menu divider dom output --- specs/Menu.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/specs/Menu.md b/specs/Menu.md index fc444a536aeb59..43c450ad8e45ba 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -252,6 +252,16 @@ Below are some sample DOM outputs to compare for certain scenarios. Not all DOM ``` +### Menu divider + +```html + + + + + +``` + ### Custom rendering and data v7 provides render callbacks that can be used to render either the entire menu list or specific slots of menut items. Each call back provides the props avaialble to that slot and a `defaultRender` which allows to easily extend the original render, if required. From 38ce93cd3934e6785c57d0ad4cb26edcfd80f297 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 19 Jan 2021 14:19:24 +0000 Subject: [PATCH 20/44] add divider API --- specs/Menu.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/specs/Menu.md b/specs/Menu.md index 43c450ad8e45ba..aaaea9e517cf7e 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -395,6 +395,10 @@ Creates a section inside a `MenuList`, setting up header layout and dividers bet The MenuSection is also a useful component to declare different selection groups (checkbox/radio) in a `MenuList`. +### MenuDivider + +Creates a divider element in the `MenuList` with correct HTML and aria semantics for divider. Intention is to use this internally within `MenuList` and `MenuSection` and avoid explicit uses of dividers to ensure correct HTML and aria semantics. + ### MenuItem As the name infers From dce4cfe5342c2bec585fba723bd9bf045f4baeab Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 19 Jan 2021 15:41:38 +0000 Subject: [PATCH 21/44] Add expected DOM output --- specs/Menu.md | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/specs/Menu.md b/specs/Menu.md index aaaea9e517cf7e..eb847c6552ff92 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -425,6 +425,16 @@ const menu = ( ) ``` +```html + + +
+
Option 1
+
Option 2
+
Option 3
+
+``` + ### Sections and submenus ```typescript @@ -447,6 +457,20 @@ const menu = ( ) ``` +```html + + +
+
Option 1
+
+
Section Option 1
+
Section Option 2
+
Section Option 3
+
+ +
+``` + ### Standlone ```typescript @@ -463,6 +487,15 @@ const menu = ( ) ``` +```html + +
+
Option 1
+
Option 2
+
Option 3
+
+``` + ### Selection ```typescript @@ -503,6 +536,28 @@ const menuSelectableSections = ( ) ``` +```html + +
+
Option 1
+
Option 2
+
Option 3
+
+ + +
+
Option 1
+
Option 2
+
Option 3
+
+
+
Option 1
+
Option 2
+
Option 3
+
+
+``` + // TODO positioning examples ? ## Behaviors From f04ca9433b74ae2669543785ea20fc366f04ebda Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 19 Jan 2021 15:46:37 +0000 Subject: [PATCH 22/44] Add learnings --- specs/Menu.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/specs/Menu.md b/specs/Menu.md index eb847c6552ff92..39c03e04eb7875 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -218,6 +218,12 @@ Below are some sample DOM outputs to compare for certain scenarios. Not all DOM ### Basic menu +Both the current v7 and v0 versions of this control use the `ul` and `li` combination along with content wrapper elements. This makes style overrides kind of complicated to target and also makes custom rendering difficult since there is the added complexity of targeting stricter DOM structures. + +`ul`/`li` combinations are also very strict in markdown and might not play well with newer concepts like virtualization and custom scrollbars where arbitrary `div` elements can be inserted into the DOM. + +In terms off A11y and narration there is effectively no difference in having a wrapping element or not. Would useful in the proposed new API to use a simpler DOM structure that provides more flexibility. + ```html
    From e347a792ae0561f3b95a6515853474d93a509734 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 20 Jan 2021 12:41:08 +0000 Subject: [PATCH 23/44] add focus specs --- specs/Menu.md | 85 ++++++++++++++++++++++++++------------------------- 1 file changed, 44 insertions(+), 41 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index 39c03e04eb7875..6143cf2fa7231a 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -582,24 +582,26 @@ A menu can be triggered by the following user interactions on the triggering/anc As a general rule, once the menu is closed the focus should return to the triggering element once the menu is closed unless the interaction would involve another focusable element. -| Type | Action | Result | Details | -| -------- | ---------- | ------- | ---------------------------------------------------------------------- | -| Mouse | Click | Open | Click on the trigger element | -| Mouse | Hover | Open | Hover over the trigger element with delay | -| Mouse | LongPress | Open | MouseDown with delay, equivalent to right click for touch devices | -| Mouse | Click | Open | Right click for contextual menus | -| Keyboard | Enter | Open | Focus on trigger element and press Enter | -| Keyboard | Space | Open | Focus on trigger element and press Space | -| Keyboard | Shift+F10 | Open | Focus on trigger element to open context menu (i.e. right click) | -| Keyboard | ArrowDown | Open | Focus on trigger element. Used in menu buttons | -| Mouse | Click | Dismiss | Click anywhere outside the component | -| Mouse | Click | Dismiss | Click on the trigger while the menu is open | -| Mouse | Click | Dismiss | Click on a menu item | -| Mouse | MouseLeave | Dismiss | Mouse leaves the component after a delay | -| Keyboard | Enter | Dismiss | Invoked on a menu item | -| Keyboard | Space | Dismiss | Invoked on a menu item | -| Keyboard | Esc | Dismiss | Closes the menu and focuses on the triggering element | -| Keyboard | Tab | Dismiss | Closes the menu and all submenus, focus moves to next element in order | +| Type | Action | Result | Details | Focus after | +| -------- | ---------- | ------- | ----------------------------------------------------------------- | --------------------------------------------- | +| Mouse | Click | Open | Click on the trigger element | First menuitem | +| Mouse | Hover | Open | Hover over the trigger element with delay | First menuitem | +| Mouse | LongPress | Open | MouseDown with delay, equivalent to right click for touch devices | First menuitem | +| Mouse | Click | Open | Right click for contextual menus | First menuitem | +| Keyboard | Enter | Open | Focus on trigger element and press Enter | First menuitem | +| Keyboard | Space | Open | Focus on trigger element and press Space | First menuitem | +| Keyboard | Shift+F10 | Open | Focus on trigger element to open context menu (i.e. right click) | First menuitem | +| Keyboard | ArrowDown | Open | Focus on trigger element. Used in menu buttons | First menuitem | +| Keyboard | ArrowUp | Open | Focus on trigger element. Used in menu buttons | Last menuitem | +| Mouse | Click | Dismiss | Click anywhere outside the component | menu trigger | +| Mouse | Click | Dismiss | Click on the trigger while the menu is open | menu trigger | +| Mouse | Click | Dismiss | Click on a menu item | User defined - default menu trigger | +| Mouse | MouseLeave | Dismiss | Mouse leaves the component after a delay | menu trigger | +| Keyboard | Enter | Dismiss | Invoked on a menu item | User defined - default menu trigger | +| Keyboard | Space | Dismiss | Invoked on a menu item | User defined - default menu trigger | +| Keyboard | Esc | Dismiss | Closes the menu | menu trigger | +| Keyboard | Tab | Dismiss | Closes the menu and all submenus | next tabbable element after menu trigger | +| Keyboard | Shift+Tab | Dismiss | Closes the menu and all submenus | previous tabbable element before menu trigger | ### Submenu trigger/navigation @@ -607,19 +609,22 @@ A submenu can be triggered by the following user interactions on the triggering As a general rule, once a submenu is dismissed without dismissing the menu, the focus should revert to the triggering menu item unless the interaction involves another focusable UI component. -| Type | Action | Result | Details | -| -------- | ---------- | ------- | ---------------------------------------------------------------------- | -| Mouse | Click | Open | Click the menu item that contains a submenu | -| Mouse | Hover | Open | Hover over the menu item that contains a submenu with delay | -| Keyboard | Enter | Open | Focus on triggering menu item | -| Keyboard | Space | Open | Focus on triggering menu item | -| Keyboard | ArrowRight | Open | Focus on triggering menu item | -| Mouse | Click | Dismiss | Click on an item in the submenu | -| Mouse | Click | Dismiss | Click on a UI element that is not the submenu | -| Mouse | MouseLeave | Dismiss | Mouse leaves the submenu or its triggering menu item after delay | -| Keyboard | ArrowLeft | Dismiss | Closes the submenu and focuses on the triggering menu item | -| Keyboard | Esc | Dismiss | Closes the submenu and focuses on the triggering menu item | -| Keyboard | Tab | Dismiss | Closes the menu and all submenus, focus moves to next element in order | +| Type | Action | Result | Details | Focus after | +| -------- | ---------- | ------- | ---------------------------------------------------------------- | -------------------------------------------------- | +| Mouse | Click | Open | Click the menu item that contains a submenu | First menuitem in submenu | +| Mouse | Hover | Open | Hover over the menu item that contains a submenu with delay | First menuitem in submenu | +| Keyboard | Enter | Open | Focus on triggering menu item | First menuitem in submenu | +| Keyboard | Space | Open | Focus on triggering menu item | Frist menuitem in submenu | +| Keyboard | ArrowRight | Open | Focus on triggering menu item | First menuitem in submenu | +| Mouse | Click | Dismiss | Click on an item in the submenu | | +| Keyboard | Space | Dismiss | Invoked on a submenu item | | +| Keyboard | Space | Dismiss | Invoked on a submenu item | | +| Mouse | Click | Dismiss | Click on a UI element that is not the submenu | Root menu trigger | +| Mouse | MouseLeave | Dismiss | Mouse leaves the submenu or its triggering menu item after delay | Root menu trigger | +| Keyboard | ArrowLeft | Dismiss | Closes the submenu | menu item that contained submenu | +| Keyboard | Esc | Dismiss | Closes the submenu | menu item that contained submenu | +| Keyboard | Tab | Dismiss | Closes the menu and all submenus | Next tabbable element after root menu trigger | +| Keyboard | Shift+Tab | Dismiss | Closes the menu and all submenus | Previous tabbable element before root menu trigger | ### Split button MenuItem submenu @@ -635,16 +640,14 @@ Once the the submenu is open, the same behavior as in the [previous section](#su Keyboard interactions required to navigate the menu. The alphanumeric match interaction does not need to be supported in all cases, but should be supported as much as possible. -| Type | Action | Result | Details | -| -------- | --------- | ----------------- | --------------------------------------------------------------------- | -| Keyboard | ArrowDown | Next Item | Roving, if on the last item got to the first | -| Keyboard | ArrowUp | Previous Item | Roving, if on the first item go to the last | -| Keyboard | Home | First item | | -| Keyboard | End | Last item | | -| Keyboard | A-Z, 0-9 | Matched item | Matches the first item that corresponds alphabetically or numerically | -| Mouse | Hover | Reveals scrollbar | If required, reveals scrollbar after delay | - -### MenuItem selection +| Type | Action | Result | Details | Focus after | +| -------- | --------- | ----------------- | --------------------------------------------------------------------- | ------------------------------------------ | +| Keyboard | ArrowDown | Next Item | Roving | Next item, if on last item then first | +| Keyboard | ArrowUp | Previous Item | Roving | Previous item, if on first item go to last | +| Keyboard | Home | First item | | First item | +| Keyboard | End | Last item | | Last item | +| Keyboard | A-Z, 0-9 | Matched item | Matches the first item that corresponds alphabetically or numerically | Matched item | +| Mouse | Hover | Reveals scrollbar | If required, reveals scrollbar after delay | Keeps focus on existing item | ### MenuItem selection | Below are the interactions that should be supported for all menu items that are required to handle a selection state. From 1b0664cc51e177cddafe387e1cc2acd9b18ee43d Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 20 Jan 2021 13:04:59 +0000 Subject: [PATCH 24/44] add aria --- specs/Menu.md | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index 6143cf2fa7231a..6eafb55e13cb88 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -216,7 +216,7 @@ const menuProps = { Below are some sample DOM outputs to compare for certain scenarios. Not all DOM attributes are reflected here, a subset have been chosen to provide easier reading and comparison. -### Basic menu +#### Basic menu Both the current v7 and v0 versions of this control use the `ul` and `li` combination along with content wrapper elements. This makes style overrides kind of complicated to target and also makes custom rendering difficult since there is the added complexity of targeting stricter DOM structures. @@ -434,7 +434,8 @@ const menu = ( ```html -
    + +
    Option 1
    Option 2
    Option 3
    @@ -454,7 +455,7 @@ const menu = ( Section Option 2 Section Option 3 - + Option 1 Option 2 Option 3 @@ -466,14 +467,22 @@ const menu = ( ```html -
    + +
    Option 1
    Section Option 1
    Section Option 2
    Section Option 3
    - + +
    + + +
    +
    Option 1
    +
    Option 2
    +
    Option 3
    ``` @@ -543,15 +552,17 @@ const menuSelectableSections = ( ``` ```html + + -
    +
    Option 1
    Option 2
    Option 3
    -
    +
    Option 1
    Option 2
    Option 3
    @@ -564,8 +575,6 @@ const menuSelectableSections = (
    ``` -// TODO positioning examples ? - ## Behaviors ### Useful references @@ -661,12 +670,10 @@ In the event that the selection method is a radio, the previous selected item mu ## Accessibiltiy -WIP 🚧 +Accessibility behaviour is built into the spec as much as possible. This section addresses specific issues that don't fit well with the standard definition of the component. ### Focus management -### Specific keyboard behaviors - -Address any specific a11y/narration keyboard behaviours that should be considered which might not fit under official specced behaviour +### Disabled menu items -### Narration +Disabled menu items should be focusable From 4d977ab3a4ee3ffc0945cf6160d4222887bb6ea4 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 20 Jan 2021 13:29:44 +0000 Subject: [PATCH 25/44] add split button proposal --- specs/Menu.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/specs/Menu.md b/specs/Menu.md index 6eafb55e13cb88..7ecf40ff9d6d88 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -409,6 +409,10 @@ Creates a divider element in the `MenuList` with correct HTML and aria semantics As the name infers +### MenuItemSplit + +A layout component that renders two `MeuItem`s in the same design as a split button. We consider both parts of the split button to be separate menu items to get the most straightforward keyboard and narration experience. + ### SubMenu Creates a `Menu` component with `MenuItem` trigger and handles the positioning of the nested menu. @@ -520,6 +524,7 @@ const [selectedItems, setSelectedItems] = React.useState([]); // basic checkbox example const menuCheckbox = ( ``` +### Split button + +```typescript +const trigger = + +// basic checkbox example +const menuSplitbutton= ( + + Option 1 + + Main content + + +) +``` + +```html + +
    +
    Option 1
    +
    +
    children content
    +
    splitContent slot
    +
    +
    +``` + ## Behaviors ### Useful references From fd216582158a936682c56bba6eb3990fce597746 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 20 Jan 2021 15:55:58 +0000 Subject: [PATCH 26/44] add missing group --- specs/Menu.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index 7ecf40ff9d6d88..86ea5ef1440e8f 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -568,9 +568,11 @@ const menuSelectableSections = (
    -
    Option 1
    -
    Option 2
    -
    Option 3
    +
    +
    Option 1
    +
    Option 2
    +
    Option 3
    +
    Option 1
    From cc5f402789be92b6c875acfe10e1696d5e19a793 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 20 Jan 2021 16:10:09 +0000 Subject: [PATCH 27/44] rework section --- specs/Menu.md | 48 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index 86ea5ef1440e8f..4bbbea1b2c809b 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -446,7 +446,7 @@ const menu = (
    ``` -### Sections and submenus +### Sections ```typescript const trigger = @@ -454,11 +454,40 @@ const trigger = const menu = ( Option 1 - + Section Option 1 Section Option 2 Section Option 3 + +) +``` + +```html + + + +
    +
    Option 1
    +
    + +
    +
    Section Option 1
    +
    Section Option 2
    +
    Section Option 3
    +
    +
    +
    +``` + +### Submenus + +```typescript +const trigger = + +const menu = ( + + Option 1 Option 1 Option 2 @@ -474,11 +503,6 @@ const menu = (
    Option 1
    -
    -
    Section Option 1
    -
    Section Option 2
    -
    Section Option 3
    -
    @@ -542,12 +566,12 @@ const menuSelectableSections = ( onSelectionChange={setSeelctedItems} trigger={trigger} > - + Option 1 Option 2 Option 3 - + Option 1 Option 2 Option 3 @@ -568,13 +592,15 @@ const menuSelectableSections = (
    -
    + +
    Option 1
    Option 2
    Option 3
    -
    + +
    Option 1
    Option 2
    Option 3
    From f4637d09fc459292c0410635890ccdcf19a4d48d Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 20 Jan 2021 16:40:25 +0000 Subject: [PATCH 28/44] rework split submenu --- specs/Menu.md | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index 4bbbea1b2c809b..50a405256fce9e 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -409,10 +409,12 @@ Creates a divider element in the `MenuList` with correct HTML and aria semantics As the name infers -### MenuItemSplit +### SubMenuSplit A layout component that renders two `MeuItem`s in the same design as a split button. We consider both parts of the split button to be separate menu items to get the most straightforward keyboard and narration experience. +Only the indicator part of the split button will control the submenu. + ### SubMenu Creates a `Menu` component with `MenuItem` trigger and handles the positioning of the nested menu. @@ -617,22 +619,38 @@ const trigger = const menuSplitbutton= ( Option 1 - - Main content - + + Option 1 + Option 2 + Option 3 + ) ``` ```html +
    +
    Option 1
    + +
    +
    Option 1
    -
    children content
    -
    splitContent slot
    +
    content slot
    +
    + + +
    +
    Option 1
    +
    Option 2
    +
    Option 3
    +
    ``` ## Behaviors From 68d2e08e6c413210a7dc3e3591f913d1a0abf2ba Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 20 Jan 2021 16:48:56 +0000 Subject: [PATCH 29/44] add icon examples --- specs/Menu.md | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/specs/Menu.md b/specs/Menu.md index 50a405256fce9e..78320eacbd91ae 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -423,7 +423,7 @@ Creates a `Menu` component with `MenuItem` trigger and handles the positioning o The below samples do not represent the definitive props of the final implemented component, but represent the ideal final implementations. Can be subject to change during the implementation phase. -### Default Menu +### Basic Menu ```typescript const trigger = @@ -448,6 +448,40 @@ const menu = (
    ``` +### Menu items with icons + +```typescript +const trigger = + +const menu = ( + + }>Option 1 + }>Option 2 + }>Option 3 + +) +``` + +```html + + + +
    +
    + FileIcon + Option 1 +
    +
    + BellIcon + Option 2 +
    +
    + LinkIcon + Option 3 +
    +
    +``` + ### Sections ```typescript From e72c28d32679456b53dde627f0c3883dd8b87325 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 20 Jan 2021 16:55:45 +0000 Subject: [PATCH 30/44] add props --- specs/Menu.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/specs/Menu.md b/specs/Menu.md index 78320eacbd91ae..8fd1df4f177784 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -401,6 +401,11 @@ Creates a section inside a `MenuList`, setting up header layout and dividers bet The MenuSection is also a useful component to declare different selection groups (checkbox/radio) in a `MenuList`. +| Prop name | Type | Details | +| ------------ | ------- | --------------------------------------------------- | +| title | text | The title of of the section | +| displayTitle | boolean | Whether to visually render the title in the section | + ### MenuDivider Creates a divider element in the `MenuList` with correct HTML and aria semantics for divider. Intention is to use this internally within `MenuList` and `MenuSection` and avoid explicit uses of dividers to ensure correct HTML and aria semantics. @@ -409,16 +414,31 @@ Creates a divider element in the `MenuList` with correct HTML and aria semantics As the name infers +| Prop name | Type | Details | +| --------- | --------- | ---------------------------------------- | +| icon | ReactNode | Icon that is rendered with the menu item | + ### SubMenuSplit A layout component that renders two `MeuItem`s in the same design as a split button. We consider both parts of the split button to be separate menu items to get the most straightforward keyboard and narration experience. Only the indicator part of the split button will control the submenu. +| Prop name | Type | Details | +| ------------- | --------- | -------------------------------------------------------- | +| content | ReactNode | The children that would normally be rendered in MenuItem | +| icon | ReactNode | Icon that is rendered with the menu item | +| indicatorIcon | ReactNode | Icon that is rendered for the split button indicator | + ### SubMenu Creates a `Menu` component with `MenuItem` trigger and handles the positioning of the nested menu. +| Prop name | Type | Details | +| --------- | --------- | -------------------------------------------------------- | +| content | ReactNode | The children that would normally be rendered in MenuItem | +| icon | ReactNode | Icon that is rendered with the menu item | + ## Sample code The below samples do not represent the definitive props of the final implemented component, but represent the ideal final implementations. Can be subject to change during the implementation phase. From fb1f3dc1b52a1ae29ca6a7d4552c6fa1abdf020d Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 20 Jan 2021 17:00:02 +0000 Subject: [PATCH 31/44] add secondary label --- specs/Menu.md | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index 8fd1df4f177784..eb96eaee863490 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -349,11 +349,11 @@ When an options is associated with a selection state. The `Menu`, either root or A `Menu` can be partitioned into sections using visible dividers in its list of options. Each section can contain a heading title that announces or briefly describes the options in the particular section -### Secondary text +### Secondary label -An option of a `Menu` component should be able to declare additional secondary text that can provide additional context describing the option or its usage. +An option of a `Menu` component should be able to declare additional secondary label that can provide additional context describing the option or its usage. -For example a secondary text can be a label that shows a keyboard shortcut that will perform an equivalent action of the option of the `Menu` component. +For example a secondary label can be a label that shows a keyboard shortcut that will perform an equivalent action of the option of the `Menu` component. ### Split option with nesting @@ -414,9 +414,10 @@ Creates a divider element in the `MenuList` with correct HTML and aria semantics As the name infers -| Prop name | Type | Details | -| --------- | --------- | ---------------------------------------- | -| icon | ReactNode | Icon that is rendered with the menu item | +| Prop name | Type | Details | +| -------------- | --------- | ----------------------------------------- | +| icon | ReactNode | Icon that is rendered with the menu item | +| secondaryLabel | text | A secondary label i.e. keyboard shortcuts | ### SubMenuSplit @@ -434,10 +435,11 @@ Only the indicator part of the split button will control the submenu. Creates a `Menu` component with `MenuItem` trigger and handles the positioning of the nested menu. -| Prop name | Type | Details | -| --------- | --------- | -------------------------------------------------------- | -| content | ReactNode | The children that would normally be rendered in MenuItem | -| icon | ReactNode | Icon that is rendered with the menu item | +| Prop name | Type | Details | +| -------------- | --------- | -------------------------------------------------------- | +| content | ReactNode | The children that would normally be rendered in MenuItem | +| icon | ReactNode | Icon that is rendered with the menu item | +| secondaryLabel | text | A secondary label i.e. keyboard shortcuts | ## Sample code From bb794709c6108144c2eb18b4bbf1e241564a1575 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Fri, 22 Jan 2021 12:51:17 +0000 Subject: [PATCH 32/44] convert menu section to group --- specs/Menu.md | 78 +++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 60 insertions(+), 18 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index eb96eaee863490..07a498b0cae91f 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -395,20 +395,25 @@ This component is used internally by `Menu` and manages the context and layout i `MenuList` can also be used separately as the standalone variant of the `Menu`, since it should not control popup positioning or triggers. It is the only component in the API that can be used standalone. Envisioned to be used with more complex popup or trigger scenarios where the `Menu` component does not provide enough control for these situations. -### MenuSection +### MenuGroup -Creates a section inside a `MenuList`, setting up header layout and dividers between `MenuItems`. +Creates a group inside a `MenuList`, setting up header layout and dividers between `MenuItems`. -The MenuSection is also a useful component to declare different selection groups (checkbox/radio) in a `MenuList`. +The MenuGroup is also a useful component to declare different selection groups (checkbox/radio) in a `MenuList`. -| Prop name | Type | Details | -| ------------ | ------- | --------------------------------------------------- | -| title | text | The title of of the section | -| displayTitle | boolean | Whether to visually render the title in the section | +| Prop name | Type | Details | +| --------- | ---- | --------------------------------------------------------------------------- | +| title | text | The title of of the section renders a [MenuGroupHeader](#menusectionheader) | + +### MenuGroupHeader + +Creates a section header element with appropriate styling. Will set correct `aria-labelledby` relationship if it is instantiated within a [MenuGroup](#menugroup) ### MenuDivider -Creates a divider element in the `MenuList` with correct HTML and aria semantics for divider. Intention is to use this internally within `MenuList` and `MenuSection` and avoid explicit uses of dividers to ensure correct HTML and aria semantics. +Creates a divider element in the `MenuList` with correct HTML and aria semantics for divider. + +This divider is purely a visual cue. To ensure consistent narration experience across all screenreaders [MenuGroup](#menugroup) should be used ### MenuItem @@ -512,11 +517,12 @@ const trigger = const menu = ( Option 1 - + + Section Option 1 Section Option 2 Section Option 3 - + ) ``` @@ -527,9 +533,45 @@ const menu = (
    Option 1
    + +
    + +
    Section Option 1
    +
    Section Option 2
    +
    Section Option 3
    +
    - +
    +``` + +Custom section headings can also be used, but must be used within a [MenuGroup](#menugroup) to ensure correct narration experience + +```typescript +const trigger = + +const menu = ( + + Option 1 + + + {children} + Section Option 1 + Section Option 2 + Section Option 3 + + +) +``` + +```html + + + +
    +
    Option 1
    +
    +
    Section Option 1
    Section Option 2
    Section Option 3
    @@ -617,23 +659,23 @@ const menuCheckbox = ( ) -// leverage MenuSection for different selection groups +// leverage MenuGroup for different selection groups const menuSelectableSections = ( - + Option 1 Option 2 Option 3 - - + + Option 1 Option 2 Option 3 - + ) ``` @@ -650,15 +692,15 @@ const menuSelectableSections = (
    -
    +
    Option 1
    Option 2
    Option 3
    -
    +
    Option 1
    Option 2
    Option 3
    From 3b54cecc2c016e2045e877521b23f3bd2b1da9ec Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Fri, 22 Jan 2021 13:02:45 +0000 Subject: [PATCH 33/44] update selection sopec --- specs/Menu.md | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index 07a498b0cae91f..939768cc35b519 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -424,6 +424,24 @@ As the name infers | icon | ReactNode | Icon that is rendered with the menu item | | secondaryLabel | text | A secondary label i.e. keyboard shortcuts | +### MenuItemCheckbox + +A variant of `MenuItem` that allows a multiple selection state based on the value that it represents + +| Prop name | Type | Details | +| --------- | ---- | -------------------------------------------------- | +| name | text | The name of the value that the checkbox represents | +| value | text | The value of the checkbox | + +### MenuItemRadio + +A variant of `MenuItem` that allows a single selection state based on the value that it represents + +| Prop name | Type | Details | +| --------- | ---- | -------------------------------------------------- | +| name | text | The name of the value that the checkbox represents | +| value | text | The value of the checkbox | + ### SubMenuSplit A layout component that renders two `MeuItem`s in the same design as a split button. We consider both parts of the split button to be separate menu items to get the most straightforward keyboard and narration experience. @@ -653,9 +671,9 @@ const menuCheckbox = ( onSelectionChange={setSeelctedItems} trigger={trigger} > - Option 1 - Option 2 - Option 3 + Option 1 + Option 2 + Option 3 ) @@ -667,14 +685,14 @@ const menuSelectableSections = ( trigger={trigger} > - Option 1 - Option 2 - Option 3 + Option 1 + Option 2 + Option 3 - Option 1 - Option 2 - Option 3 + Option 1 + Option 2 + Option 3 ) From 18690bf7d7705341d7f342130d26866c5e29df2c Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Fri, 22 Jan 2021 13:05:38 +0000 Subject: [PATCH 34/44] add a11y warning --- specs/Menu.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/specs/Menu.md b/specs/Menu.md index 939768cc35b519..49eb59ae640168 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -866,6 +866,12 @@ In the event that the selection method is a radio, the previous selected item mu Accessibility behaviour is built into the spec as much as possible. This section addresses specific issues that don't fit well with the standard definition of the component. +### Creating sections or groups within a menu + +⚠️ When using [MenuDivider](#menudivider) without [MenuGroup](#menugroup) + +The [MenuDivider](#menudivider) is a purely visual component. The component is only intended to be used as visual 'sugar'. When meaningful partitions [MenuItems](#menuitem) exists, [MenuGroup](#menugroup) should be used to provide the correct experience for narration. + ### Focus management ### Disabled menu items From 021d1027d2d93a3102f9581db9ac698bf39cabe8 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Fri, 22 Jan 2021 13:08:40 +0000 Subject: [PATCH 35/44] a11y warning --- specs/Menu.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/specs/Menu.md b/specs/Menu.md index 49eb59ae640168..bf1df426a8624e 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -872,6 +872,10 @@ Accessibility behaviour is built into the spec as much as possible. This section The [MenuDivider](#menudivider) is a purely visual component. The component is only intended to be used as visual 'sugar'. When meaningful partitions [MenuItems](#menuitem) exists, [MenuGroup](#menugroup) should be used to provide the correct experience for narration. +⚠️ When using [MenuSectionHeader](#menudivider) + +[MenuGroup](#menugroup) as a parent component ensures that correct `aria-labelledby` relationship is defined between the header and the group. + ### Focus management ### Disabled menu items From a9ff81eb7f26d84a2e2c2c89742a425bfe855289 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Sun, 24 Jan 2021 17:26:48 +0000 Subject: [PATCH 36/44] start prop mapping --- specs/Menu.md | 42 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index bf1df426a8624e..ffd56166e5c979 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -29,14 +29,44 @@ Note that the below code samples are not meant to be complete, but to highlight `ContextualMenu` in v7 is a component that also exposes the API to control the positioning of the temporary popup surface that the Menu is rendered on. This aspect of the v7 component should be compared with the `Popup` component in v0 since the v0 `Menu` is created as a standalone component with no positioning properties. -v0 uses the [OSS Popper.js library](https://popper.js.org/) while v7 uses a component based implementation `CalloutContent`. Below we provide the results of testing common positioning boundary/edge cases between the two. +v0 uses the [OSS Popper.js library](https://popper.js.org/) while v7 uses a component based implementation `CalloutContent`. As a result, the API is very similar in intent and vocabulary as Popper. -// TODO compare boudary/edge cases in positioning +Below we provide the results of testing common positioning boundary/edge cases between the two. -- flip -- prevent overflow -- offset from reference -- tethering +#### Positioning vs styling + +The biggest difference bewteen the two libraries is that v0 provides a purely positioning based API out of the `Popup` react component. v0 Provides no direct prop values that will style the popup container and any adjustments to stlying properties such as (but not limited to) dimensions/margin/padding/layoud are expected to be implmented through the styling system used throughout the library + +The `Callout` component has some styling helpers that affect the styling of the contents: + +- calloutMaxHeight +- calloutMaxWidth +- calloutWidth + +The `ContextualMenu` component uses two styling properties not offered by `Callout` (useTargetWidth, useTargetMinWidth) and also duplicates some of `Callout's` own positioning properties while also allowing a shorthand slot for the `Callout` + +```typescript + +``` + +The result being that `ContextualMenu's` positiong and styling risks being abused by developers inexperienced in the library. There is also no documentation on the v7 docsite that states `calloutProps` is actually overriden by props declared directly on `ContextualMenu` + +#### Prop mapping + +While this doc generally avoids large tables and tries to use positive reinforcement with readable code examples, the only way to provide a somewhat consistent comparison to positioning is with a mapping table to intended functionality. ### Trigger vs target From fc8d01231025c4b122f4e2f047e7f718b5b3fe10 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Sun, 24 Jan 2021 21:23:06 +0000 Subject: [PATCH 37/44] add position alignment hint mapping --- specs/Menu.md | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index ffd56166e5c979..098bbf12028f13 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -64,9 +64,32 @@ The `ContextualMenu` component uses two styling properties not offered by `Callo The result being that `ContextualMenu's` positiong and styling risks being abused by developers inexperienced in the library. There is also no documentation on the v7 docsite that states `calloutProps` is actually overriden by props declared directly on `ContextualMenu` -#### Prop mapping - -While this doc generally avoids large tables and tries to use positive reinforcement with readable code examples, the only way to provide a somewhat consistent comparison to positioning is with a mapping table to intended functionality. +### Position/Alignment hints + +Both libraries provide an API that achieves the same end result for positioning and alignment. Below is a table that maps the v7 `DirectionalHint` with the v0 props of `position` and `alignment` + +| DirectionalHint (v7) | Position (v0) | Alignment (v0) | +| -------------------- | ------------- | -------------- | +| topLeftEdge | above | start | +| topCenter | above | center | +| topRightEdge | above | bottom | +| topAutoEdge | above | | +| bottomLeftEdge | above | start | +| bottomCenter | below | center | +| bottomRightEdge | below | bottom | +| bottomAutoEdge | below | | +| leftTopEdge | before | top | +| leftCenter | before | center | +| leftBottomEdge | before | bottom | +| rightTopEdge | after | before | +| rightCenter | after | center | +| rightBottomEdge | after | bottom | + +First it's necessary to note the difference between the vocabulary used between the two. v7 will use `left` and `right` while v0 uses `before` and `after`. v0 vocabulary here is chosen to convey the appropriate meaning regardless of RTL by using the semantics of the conntent. + +In general the separation of both the position and alignment in v0 results in an API that is easier to use if a consumer only needs to modify one of the two props. However both try to achieve the same result in the end. + +It's important to note that if an incorrect pair of `position` and `align` are provided in v0, then `position` takes priority and `align` is set immediately to `center` ### Trigger vs target From c607343bcec30dd3d3608b7ce772b54f8972235b Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Sun, 24 Jan 2021 21:36:54 +0000 Subject: [PATCH 38/44] add offset --- specs/Menu.md | 60 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 42 insertions(+), 18 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index 098bbf12028f13..a137aec211992f 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -33,7 +33,7 @@ v0 uses the [OSS Popper.js library](https://popper.js.org/) while v7 uses a comp Below we provide the results of testing common positioning boundary/edge cases between the two. -#### Positioning vs styling +#### Configuration consistency The biggest difference bewteen the two libraries is that v0 provides a purely positioning based API out of the `Popup` react component. v0 Provides no direct prop values that will style the popup container and any adjustments to stlying properties such as (but not limited to) dimensions/margin/padding/layoud are expected to be implmented through the styling system used throughout the library @@ -68,28 +68,52 @@ The result being that `ContextualMenu's` positiong and styling risks being abuse Both libraries provide an API that achieves the same end result for positioning and alignment. Below is a table that maps the v7 `DirectionalHint` with the v0 props of `position` and `alignment` -| DirectionalHint (v7) | Position (v0) | Alignment (v0) | -| -------------------- | ------------- | -------------- | -| topLeftEdge | above | start | -| topCenter | above | center | -| topRightEdge | above | bottom | -| topAutoEdge | above | | -| bottomLeftEdge | above | start | -| bottomCenter | below | center | -| bottomRightEdge | below | bottom | -| bottomAutoEdge | below | | -| leftTopEdge | before | top | -| leftCenter | before | center | -| leftBottomEdge | before | bottom | -| rightTopEdge | after | before | -| rightCenter | after | center | -| rightBottomEdge | after | bottom | +`DirectionalHint` can be passed to both `Callout` (which powers positioning) or directly to `ContextualMenu` (in two different ways). Whereas `position` and `alignment` are props of `Popup` in v0 and not used directly in `Menu` even for the positioning of its submenu. + +| DirectionalHint (v7) | position (v0) | align (v0) | +| -------------------- | ------------- | ---------- | +| topLeftEdge | above | start | +| topCenter | above | center | +| topRightEdge | above | bottom | +| topAutoEdge | above | | +| bottomLeftEdge | above | start | +| bottomCenter | below | center | +| bottomRightEdge | below | bottom | +| bottomAutoEdge | below | | +| leftTopEdge | before | top | +| leftCenter | before | center | +| leftBottomEdge | before | bottom | +| rightTopEdge | after | before | +| rightCenter | after | center | +| rightBottomEdge | after | bottom | First it's necessary to note the difference between the vocabulary used between the two. v7 will use `left` and `right` while v0 uses `before` and `after`. v0 vocabulary here is chosen to convey the appropriate meaning regardless of RTL by using the semantics of the conntent. In general the separation of both the position and alignment in v0 results in an API that is easier to use if a consumer only needs to modify one of the two props. However both try to achieve the same result in the end. -It's important to note that if an incorrect pair of `position` and `align` are provided in v0, then `position` takes priority and `align` is set immediately to `center` +It's important to note that if an incorrect pair of `position` and `align` are provided in v0, then `position` takes priority and `align` is set to `center` + +#### Offset + +```typescript + + + + +// offset can also be a function of raw Popper properties +const offsetFunction = ({ + popper: PopperJs.Rect; + reference: PopperJs.Rect; + placement: PopperJs.Placement; +}) => ([popper.width, -popper.height]) +``` + +v7 positioning can only apply a numerical value to the first part position attribute of `DirectionalHint`. v0 uses a much more flexible API that not only supports a function to defer calculation at runtime, but also supports the offset of the `Popup` in both axes. ### Trigger vs target From 99b622c8356ef6fa8bc8e96a9882154e92e20a21 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Sun, 24 Jan 2021 22:09:12 +0000 Subject: [PATCH 39/44] add submenu positioning --- specs/Menu.md | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/specs/Menu.md b/specs/Menu.md index a137aec211992f..b74f0947e30842 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -42,6 +42,7 @@ The `Callout` component has some styling helpers that affect the styling of the - calloutMaxHeight - calloutMaxWidth - calloutWidth +- backgroundColor The `ContextualMenu` component uses two styling properties not offered by `Callout` (useTargetWidth, useTargetMinWidth) and also duplicates some of `Callout's` own positioning properties while also allowing a shorthand slot for the `Callout` @@ -87,7 +88,7 @@ Both libraries provide an API that achieves the same end result for positioning | rightCenter | after | center | | rightBottomEdge | after | bottom | -First it's necessary to note the difference between the vocabulary used between the two. v7 will use `left` and `right` while v0 uses `before` and `after`. v0 vocabulary here is chosen to convey the appropriate meaning regardless of RTL by using the semantics of the conntent. +First it's necessary to note the difference between the vocabulary used between the two. v7 will use `left` and `right` while v0 uses `before` and `after`. v0 vocabulary here is chosen to convey the appropriate meaning regardless of RTL by using the semantics of the conntent. It's also interesting to note that it's possible to supply an explicit RTL hint to v7 which is a flip by default. v0 will flip by default but requires the consumer to detect RTL scenarios and modify props in these situations In general the separation of both the position and alignment in v0 results in an API that is easier to use if a consumer only needs to modify one of the two props. However both try to achieve the same result in the end. @@ -115,6 +116,33 @@ const offsetFunction = ({ v7 positioning can only apply a numerical value to the first part position attribute of `DirectionalHint`. v0 uses a much more flexible API that not only supports a function to defer calculation at runtime, but also supports the offset of the `Popup` in both axes. +#### Bounds and overflow + +#### Submenu positioning + +The default positioning for both v0 and v7 submenus is: + +- rightTopEdge (v7) +- top-after (v0) + +Both will also flip appropriately when the overflow boundary is too small. + +The main difference between the two is that v0 submenu's position does not expose any way to customize or override the positioning of the submenu. However v7 allows every single customization as the root menu. It is very possible to do the below: + +```typescript +const menuItems: IContextualMenuItem[] = [ + { + key: 'newItem', + subMenuProps: { + // Any positioning props of `ContextualMenu` are usable + directionalHint: DirectionalHint.rightTopEdge, + // All `Callout` props as also usable + calloutProps: {...}, + items: [...], + }, + }, +``` + ### Trigger vs target The v7 `ContextualMenu` has a prop `target` which is intended to be a ref to the DOM element that the positioning logic anchors to. The usage of this prop requires the visibility state of the component to be controlled using React state by the consumer. The same prop exists on the v0 `Popup` component that is intended to perform the same function. From 5767d0d98e46406646f0874275dfc604cbea1d2d Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Mon, 25 Jan 2021 12:54:49 +0000 Subject: [PATCH 40/44] add bounds --- specs/Menu.md | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/specs/Menu.md b/specs/Menu.md index b74f0947e30842..122bcc4d36eda2 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -118,6 +118,42 @@ v7 positioning can only apply a numerical value to the first part position attri #### Bounds and overflow +v0 `Popup` API is more consistent in this aspect and provides more control than the v7 `Callout`. + +```typescript + +``` + +`Popup` provider 3 different properties to handle bounds and overflow: + +- flipBoundary - the bounds to calculate when to flip positioning of the popup +- overflowBoundary - the bounds to shift the popup without overflowing +- mountNode - where the popup is actually rendered in the DOM, by default this is a portal to a div in body + +```typescript + ({/*Same object as above*/})} + target={htmlElement} + + // renders to a portal node on body + layerProps={/*ILayerProps*/} + + // every single one of the above can all be declared here too + calloutProps={{bounds, target}} +/> +``` + +`ContextualMenu` or `Callout` has no notion of separate boundaries for flip or overflow, and auto behaviour is used for flip and overflow 'pushing' + #### Submenu positioning The default positioning for both v0 and v7 submenus is: @@ -143,6 +179,14 @@ const menuItems: IContextualMenuItem[] = [ }, ``` +#### Events + +v7 provides the following positioning event callbacks that might be used and should probably be supported for backwards compatibility: + +- onLayerMounted +- onPositioned +- onScroll + ### Trigger vs target The v7 `ContextualMenu` has a prop `target` which is intended to be a ref to the DOM element that the positioning logic anchors to. The usage of this prop requires the visibility state of the component to be controlled using React state by the consumer. The same prop exists on the v0 `Popup` component that is intended to perform the same function. From dc474f6eb01ff52b5704c785c5070aedf02b1310 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Mon, 25 Jan 2021 13:49:30 +0000 Subject: [PATCH 41/44] Add positioning behaviour --- specs/Menu.md | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/specs/Menu.md b/specs/Menu.md index 122bcc4d36eda2..fe9d402507397d 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -153,6 +153,7 @@ v0 `Popup` API is more consistent in this aspect and provides more control than ``` `ContextualMenu` or `Callout` has no notion of separate boundaries for flip or overflow, and auto behaviour is used for flip and overflow 'pushing' +It should also be noted that `ContextualMenu` allows all of the same props to be passed to its submenus to custom tweak position for each submenu if necessary #### Submenu positioning @@ -1011,6 +1012,41 @@ In the event that the selection method is a radio, the previous selected item mu | Keyboard | Enter | Toggle | Toggle the selection status of the menu item | | Mouse | Click | Toggle | Toggle the selection status of the menu item | +### Positioning + +### Placement + alignment + +A menu can be placed and aligned in any of the configurations allowed by current v0 and v7: + +- Before or after anchor element +- Above or below anchor element +- Aligned at top/bottom/left/right edge of anchor element +- Aligned centered to the anchor element + +The above should result in 12 possible position hints in total + +#### Flip + +A menu should be positioned so that it will flip its positioning on a given axis if the boundary (e.g. viewport) gets to small that it might overflow + +#### Nudging + +A menu should be positioned so that if its boundary (e.g. viewport) might overflow, the placement of the popup should be 'nudged' closer into the boundary + +#### Anchor placement offset + +A menu should be positioned so that the distance with respect to the anchor element should be configurable on both axes. + +#### Inline vs portal rendering + +A menu should be positioned so that it can be rendered either out of order on the DOM (e.g. portal to body) or inline in DOM order. + +#### Submenu positioning + +The default positioning for a submenu should be the standard seen in both v7 and v0. Submenu should be placed after the menu item trigger and aligned with the top edge. + +Although this should not be recommended, for the purposes of compatibility with v7, all positioning aspects should be configurable for submenus. + ## Accessibiltiy Accessibility behaviour is built into the spec as much as possible. This section addresses specific issues that don't fit well with the standard definition of the component. From b4cc4d488009a722d508a187d28a2abc87dcc95f Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 26 Jan 2021 13:46:53 +0000 Subject: [PATCH 42/44] use menu trigger --- specs/Menu.md | 156 ++++++++++++++++++++++++++------------------------ 1 file changed, 82 insertions(+), 74 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index fe9d402507397d..0a521f5a5b5010 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -604,16 +604,6 @@ Only the indicator part of the split button will control the submenu. | icon | ReactNode | Icon that is rendered with the menu item | | indicatorIcon | ReactNode | Icon that is rendered for the split button indicator | -### SubMenu - -Creates a `Menu` component with `MenuItem` trigger and handles the positioning of the nested menu. - -| Prop name | Type | Details | -| -------------- | --------- | -------------------------------------------------------- | -| content | ReactNode | The children that would normally be rendered in MenuItem | -| icon | ReactNode | Icon that is rendered with the menu item | -| secondaryLabel | text | A secondary label i.e. keyboard shortcuts | - ## Sample code The below samples do not represent the definitive props of the final implemented component, but represent the ideal final implementations. Can be subject to change during the implementation phase. @@ -621,20 +611,20 @@ The below samples do not represent the definitive props of the final implemented ### Basic Menu ```typescript -const trigger = - const menu = ( - - Option 1 - Option 2 - Option 3 + + + + Option 1 + Option 2 + Option 3 + ) ``` ```html -
    Option 1
    @@ -646,20 +636,20 @@ const menu = ( ### Menu items with icons ```typescript -const trigger = - const menu = ( - - }>Option 1 - }>Option 2 - }>Option 3 + + + + }>Option 1 + }>Option 2 + }>Option 3 + ) ``` ```html -
    @@ -680,17 +670,18 @@ const menu = ( ### Sections ```typescript -const trigger = - const menu = ( - - Option 1 - - - Section Option 1 - Section Option 2 - Section Option 3 - + + + + Option 1 + + + Section Option 1 + Section Option 2 + Section Option 3 + + ) ``` @@ -715,18 +706,20 @@ const menu = ( Custom section headings can also be used, but must be used within a [MenuGroup](#menugroup) to ensure correct narration experience ```typescript -const trigger = const menu = ( - - Option 1 - - - {children} - Section Option 1 - Section Option 2 - Section Option 3 - + + + + Option 1 + + + {children} + Section Option 1 + Section Option 2 + Section Option 3 + + ) ``` @@ -751,30 +744,35 @@ const menu = ( ### Submenus ```typescript -const trigger = - const menu = ( - - Option 1 - + + + Option 1 - Option 2 - Option 3 - + + + Open submenu + + + Option 1 + Option 2 + Option 3 + + + ) ``` ```html -
    Option 1
    - +
    Option 1
    Option 2
    @@ -819,11 +817,13 @@ const menuCheckbox = ( kind="checkbox" selectedItems={selectedItems} onSelectionChange={setSeelctedItems} - trigger={trigger} > - Option 1 - Option 2 - Option 3 + + + Option 1 + Option 2 + Option 3 + ) @@ -832,18 +832,20 @@ const menuSelectableSections = ( - - Option 1 - Option 2 - Option 3 - - - Option 1 - Option 2 - Option 3 - + + + + Option 1 + Option 2 + Option 3 + + + Option 1 + Option 2 + Option 3 + + ) ``` @@ -884,12 +886,18 @@ const trigger = // basic checkbox example const menuSplitbutton= ( - Option 1 - + + Option 1 - Option 2 - Option 3 - + + + + + Option 1 + Option 2 + Option 3 + + ) ``` From 4884a0683620a03bdf2f199ed2bdca62614a3a35 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 26 Jan 2021 13:48:06 +0000 Subject: [PATCH 43/44] add description for menu triogger --- specs/Menu.md | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/specs/Menu.md b/specs/Menu.md index 0a521f5a5b5010..5e99128f925ba6 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -539,6 +539,10 @@ Sample usages will be give in the following section of this document [Sample cod The root level component serves as a simplified interface (sugar) for popup positioning and triggering. +### MenuTrigger + +A non-visual component that wraps its child and configures them to be the trigger that will open a menu. This component should only accept one child + ### MenuList This component is used internally by `Menu` and manages the context and layout its items. @@ -592,18 +596,6 @@ A variant of `MenuItem` that allows a single selection state based on the value | name | text | The name of the value that the checkbox represents | | value | text | The value of the checkbox | -### SubMenuSplit - -A layout component that renders two `MeuItem`s in the same design as a split button. We consider both parts of the split button to be separate menu items to get the most straightforward keyboard and narration experience. - -Only the indicator part of the split button will control the submenu. - -| Prop name | Type | Details | -| ------------- | --------- | -------------------------------------------------------- | -| content | ReactNode | The children that would normally be rendered in MenuItem | -| icon | ReactNode | Icon that is rendered with the menu item | -| indicatorIcon | ReactNode | Icon that is rendered for the split button indicator | - ## Sample code The below samples do not represent the definitive props of the final implemented component, but represent the ideal final implementations. Can be subject to change during the implementation phase. From 85c1a4d27aad203d5b18582b9a9b5e5343701c7a Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 27 Jan 2021 16:31:27 +0000 Subject: [PATCH 44/44] add migration component variants --- specs/Menu.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/specs/Menu.md b/specs/Menu.md index 5e99128f925ba6..58c40d4d0a894a 100644 --- a/specs/Menu.md +++ b/specs/Menu.md @@ -1051,6 +1051,23 @@ Although this should not be recommended, for the purposes of compatibility with Accessibility behaviour is built into the spec as much as possible. This section addresses specific issues that don't fit well with the standard definition of the component. +## Migration + +The immediate candidates for adoption for a converged `Menu` component which are hinted at the beginning of the spec are: + +- [ContextualMenu](https://developer.microsoft.com/en-us/fluentui#/controls/web/contextualmenu) for v7 +- [Menu](https://fluentsite.z22.web.core.windows.net/0.52.0/components/menu/definition) for v0 + +This component has characteristics that should probably be considered for the following components in terms of future migrations: + +- [Toolbar](https://fluentsite.z22.web.core.windows.net/0.52.0/components/toolbar/definition) in v0 which shares a menu component. The component itself also should use similar behaviour and interactions to `Menu` +- [Dropdown](https://fluentsite.z22.web.core.windows.net/0.52.0/components/dropdown/definition) in v0 contains a menu. Dropdown semantics are different to that of standard menus in accessibility, but certain behaviours such as keyboard navigation and selection can be reused for such a component +- [CommandBar](https://developer.microsoft.com/en-us/fluentui#/controls/web/commandbar) in v7 also contains a menu subcomponent as well as behaviour similar to `Menu` semantics +- [Nav](https://developer.microsoft.com/en-us/fluentui#/controls/web/nav) in v7 could reuse certain behaviours in `Menu` such as keyboard navigation, but will use different DOM and aria semantics, this could be achieved through state hook variants or composition +- [OverflowSet](https://developer.microsoft.com/en-us/fluentui#/controls/web/overflowset) in v7 contains a submenu component +- [PivotSet](https://developer.microsoft.com/en-us/fluentui#/controls/web/pivot) could be considered as a component variant of `Menu` +- [Breadcrumb](https://developer.microsoft.com/en-us/fluentui#/controls/web/breadcrumb) in v7 contains a submenu component and could also be considered as a component variant of `Menu + ### Creating sections or groups within a menu ⚠️ When using [MenuDivider](#menudivider) without [MenuGroup](#menugroup)