[v12] feat(ui-popover,ui-options,ui-menu): rework Menu and Options

docs/guides/upgrade-guide.md

@@ -391,6 +391,97 @@ type: embed
 
 ```
 
+### Menu
+
+Icons for checkbox and radio menu items are now positioned on the right side instead of the left.
+
+```js
+---
+type: embed
+---
+<V12ChangelogTable
+  removed={[
+    {name:"focusBorderStyle",note:"style uses sharedTokens.focusOutline"},
+    {name:"focusBorderWidth",note:"style uses sharedTokens.focusOutline"},
+    {name:"focusBorderColor",note:"style uses sharedTokens.focusOutline"},
+    {name:"focusBorderRadius",note:"style uses sharedTokens.focusOutline"}
+  ]}
+/>
+
+```
+
+#### Menu.Item
+
+```js
+---
+type: embed
+---
+<V12ChangelogTable
+  removed={[
+    {name:"iconColor",note:""},
+    {name:"activeIconColor",note:""},
+    {name:"padding",note:"split into paddingVertical and paddingHorizontal"}
+  ]}
+/>
+
+```
+
+#### Menu.Group
+
+```js
+---
+type: embed
+---
+<V12ChangelogTable
+  removed={[
+    {name:"padding",note:"split into paddingVertical and paddingHorizontal"}
+  ]}
+/>
+
+```
+
+### Options
+
+```js
+---
+type: embed
+---
+<V12ChangelogTable
+  removed={[
+    {name:"nestedLabelPadding",note:"split into nestedLabelPaddingVertical and nestedLabelPaddingHorizontal"}
+  ]}
+/>
+
+```
+
+#### Options.Item
+
+```js
+---
+type: embed
+---
+<V12ChangelogTable
+  removed={[
+    {name:"padding",note:"split into paddingVertical and paddingHorizontal"},
+  ]}
+/>
+
+```
+
+#### Options.Separator
+
+```js
+---
+type: embed
+---
+<V12ChangelogTable
+  removed={[
+    {name:"margin",note:"split into marginVertical and marginHorizontal"}
+  ]}
+/>
+
+```
+
 ### NumberInput
 
 `error` or `success` messages are no longer displayed when the component is `readOnly` or `disabled`.

packages/ui-menu/src/Menu/MenuItem/index.tsx

@@ -25,7 +25,7 @@
 import { Component } from 'react'
 import keycode from 'keycode'
 
-import { IconCheckSolid, IconArrowOpenEndSolid } from '@instructure/ui-icons'
+import { CheckInstUIIcon, ChevronRightInstUIIcon } from '@instructure/ui-icons'
 import {
   omitProps,
   getElementType,
@@ -34,12 +34,11 @@ import {
 } from '@instructure/ui-react-utils'
 import { createChainedFunction } from '@instructure/ui-utils'
 import { isActiveElement, findDOMNode } from '@instructure/ui-dom-utils'
-import { withStyleRework as withStyle } from '@instructure/emotion'
+import { withStyle } from '@instructure/emotion'
 
 import { MenuContext } from '../../MenuContext'
 
 import generateStyle from './styles'
-import generateComponentTheme from './theme'
 
 import { allowedProps } from './props'
 import type { MenuItemProps, MenuItemState } from './props'
@@ -51,7 +50,7 @@ id: Menu.Item
 ---
 **/
 @withDeterministicId()
-@withStyle(generateStyle, generateComponentTheme)
+@withStyle(generateStyle)
 class MenuItem extends Component<MenuItemProps, MenuItemState> {
   static readonly componentId = 'Menu.Item'
 
@@ -67,12 +66,16 @@ class MenuItem extends Component<MenuItemProps, MenuItemState> {
   constructor(props: MenuItemProps) {
     super(props)
 
+    const state: MenuItemState = {
+      isHovered: false,
+      isFocused: false
+    }
+
     if (typeof props.selected === 'undefined') {
-      this.state = {
-        selected: !!props.defaultSelected
-      }
+      state.selected = !!props.defaultSelected
     }
 
+    this.state = state
     this.labelId = props.deterministicId!('MenuItem__label')
   }
 
@@ -176,6 +179,31 @@ class MenuItem extends Component<MenuItemProps, MenuItemState> {
     }
   }
 
+  handleMouseEnter = () => {
+    this.setState({ isHovered: true })
+  }
+
+  handleMouseLeave = () => {
+    this.setState({ isHovered: false })
+  }
+
+  handleFocusEvent = () => {
+    this.setState({ isFocused: true })
+  }
+
+  handleBlurEvent = () => {
+    this.setState({ isFocused: false })
+  }
+
+  getIconColor = () => {
+    const { type } = this.props
+
+    if (type === 'flyout') {
+      return 'baseColor'
+    }
+    return this.selected ? 'inverseColor' : 'baseColor'
+  }
+
   get elementType() {
     return getElementType(MenuItem, this.props)
   }
@@ -211,17 +239,19 @@ class MenuItem extends Component<MenuItemProps, MenuItemState> {
 
     return (
       <span>
+        <span css={this.props.styles?.label} id={this.labelId}>
+          {children}
+        </span>
         {(type === 'checkbox' || type === 'radio') && (
           <span css={this.props.styles?.icon}>
-            {this.selected && <IconCheckSolid />}
+            {this.selected && (
+              <CheckInstUIIcon size="md" color={this.getIconColor()} />
+            )}
           </span>
         )}
-        <span css={this.props.styles?.label} id={this.labelId}>
-          {children}
-        </span>
         {type === 'flyout' && (
           <span css={this.props.styles?.icon}>
-            <IconArrowOpenEndSolid />
+            <ChevronRightInstUIIcon size="md" color={this.getIconColor()} />
           </span>
         )}
         {renderLabelInfo && (
@@ -234,8 +264,17 @@ class MenuItem extends Component<MenuItemProps, MenuItemState> {
   }
 
   render() {
-    const { disabled, controls, onKeyDown, onKeyUp, type, href, target } =
-      this.props
+    const {
+      disabled,
+      controls,
+      onKeyDown,
+      onKeyUp,
+      onFocus,
+      onBlur,
+      type,
+      href,
+      target
+    } = this.props
 
     const props = omitProps(this.props, MenuItem.allowedProps)
     const ElementType = this.elementType
@@ -263,6 +302,10 @@ class MenuItem extends Component<MenuItemProps, MenuItemState> {
         ref={this.handleRef}
         css={this.props.styles?.menuItem}
         onMouseOver={this.handleMouseOver}
+        onMouseEnter={this.handleMouseEnter}
+        onMouseLeave={this.handleMouseLeave}
+        onFocus={createChainedFunction(onFocus, this.handleFocusEvent)}
+        onBlur={createChainedFunction(onBlur, this.handleBlurEvent)}
         data-cid="MenuItem"
       >
         {this.renderContent()}

packages/ui-menu/src/Menu/MenuItem/props.ts

@@ -124,7 +124,9 @@ const allowedProps: AllowedPropKeys = [
   'renderLabelInfo'
 ]
 type MenuItemState = {
-  selected: boolean
+  selected?: boolean
+  isHovered: boolean
+  isFocused: boolean
 }
 export type { MenuItemProps, MenuItemStyle, MenuItemState, OnMenuItemSelect }
 export { allowedProps }

packages/ui-menu/src/Menu/MenuItem/styles.ts

@@ -22,7 +22,7 @@
  * SOFTWARE.
  */
 
-import type { MenuItemTheme } from '@instructure/shared-types'
+import type { NewComponentTypes } from '@instructure/ui-themes'
 import type { MenuItemProps, MenuItemStyle } from './props'
 
 /**
@@ -36,10 +36,10 @@ import type { MenuItemProps, MenuItemStyle } from './props'
  * @return {Object} The final style object, which will be used in the component
  */
 const generateStyle = (
-  componentTheme: MenuItemTheme,
+  componentTheme: NewComponentTypes['MenuItem'],
   props: MenuItemProps
 ): MenuItemStyle => {
-  const { type, disabled } = props
+  const { type, disabled, selected } = props
 
   const isRadioOrCheckbox = type === 'checkbox' || type === 'radio'
 
@@ -53,14 +53,14 @@ const generateStyle = (
 
   const roleStyles = isRadioOrCheckbox
     ? {
-        paddingInlineStart: componentTheme.labelPadding
+        paddingInlineEnd: componentTheme.labelPadding
       }
     : {}
 
   const roleIconStyles = isRadioOrCheckbox
     ? {
-        insetInlineStart: componentTheme.iconPadding,
-        insetInlineEnd: 'auto'
+        insetInlineStart: 'auto',
+        insetInlineEnd: componentTheme.iconPadding
       }
     : {}
 
@@ -72,6 +72,23 @@ const generateStyle = (
       }
     : {}
 
+  const selectedStyles = selected
+    ? {
+        background: componentTheme.activeBackground,
+        '[class*="menuItem__label"]': {
+          color: componentTheme.activeLabelColor
+        }
+      }
+    : {}
+
+  const selectedHighlightedStyles = selected
+    ? {
+        background: componentTheme.selectedHighlightedBackground
+      }
+    : {
+        background: componentTheme.highlightedBackground
+      }
+
   const linkStyles = { textDecoration: 'none' }
 
   return {
@@ -80,7 +97,7 @@ const generateStyle = (
       position: 'relative',
       border: 'none',
       outline: 'none',
-      padding: componentTheme.padding,
+      padding: `${componentTheme.paddingVertical} ${componentTheme.paddingHorizontal}`,
       margin: '0',
       width: '100%',
       borderRadius: 'initial',
@@ -101,13 +118,7 @@ const generateStyle = (
       textDecoration: 'none',
       ...roleStyles,
       '&:focus,  &:active,  &:hover': {
-        background: componentTheme.activeBackground,
-        '[class*="menuItem__label"]': {
-          color: componentTheme.activeLabelColor
-        },
-        '[class*="menuItem__icon"]': {
-          color: componentTheme.activeIconColor
-        }
+        ...selectedHighlightedStyles
       },
       //removes extra ff button spacing
       '&::-moz-focus-inner': {
@@ -116,6 +127,7 @@ const generateStyle = (
         border: '0'
       },
       ...disabledStyles,
+      ...selectedStyles,
 
       // NOTE: needs separate groups for `:is()` and `:-webkit-any()` because of css selector group validation (see https://www.w3.org/TR/selectors-3/#grouping)
       '&:is(a)': {
@@ -133,7 +145,6 @@ const generateStyle = (
       top: '0',
       width: '1em',
       height: '100%',
-      color: componentTheme.iconColor,
       ...roleIconStyles,
       ...flyoutIconStyles
     },