From 72ea550709b4274c4a7d44c0b6592cf33373f233 Mon Sep 17 00:00:00 2001 From: Gopal Goel Date: Fri, 12 Oct 2018 19:01:35 +0530 Subject: [PATCH 01/55] feat(menuItem: add menu prop) --- ...nuExampleVerticalWithSubmenu.shorthand.tsx | 25 ++++++++ .../MenuExampleWithSubmenu.shorthand.tsx | 25 ++++++++ .../examples/components/Menu/Types/index.tsx | 10 ++++ src/components/Menu/MenuItem.tsx | 58 ++++++++++++++----- 4 files changed, 102 insertions(+), 16 deletions(-) create mode 100644 docs/src/examples/components/Menu/Types/MenuExampleVerticalWithSubmenu.shorthand.tsx create mode 100644 docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx diff --git a/docs/src/examples/components/Menu/Types/MenuExampleVerticalWithSubmenu.shorthand.tsx b/docs/src/examples/components/Menu/Types/MenuExampleVerticalWithSubmenu.shorthand.tsx new file mode 100644 index 0000000000..8853a08123 --- /dev/null +++ b/docs/src/examples/components/Menu/Types/MenuExampleVerticalWithSubmenu.shorthand.tsx @@ -0,0 +1,25 @@ +import React from 'react' +import { Menu } from '@stardust-ui/react' + +const items = [ + { + key: 'editorials', + content: 'Editorials', + menu: { + items: [ + { key: '1', content: 'item1' }, + { + key: '2', + content: 'item2', + menu: { items: [{ key: '1', content: 'item1' }, { key: '2', content: 'item2' }] }, + }, + ], + }, + }, + { key: 'review', content: 'Reviews' }, + { key: 'events', content: 'Upcoming Events' }, +] + +const MenuExampleVerticalWithSubmenu = () => + +export default MenuExampleVerticalWithSubmenu diff --git a/docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx b/docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx new file mode 100644 index 0000000000..9005c3ce05 --- /dev/null +++ b/docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx @@ -0,0 +1,25 @@ +import React from 'react' +import { Menu } from '@stardust-ui/react' + +const items = [ + { + key: 'editorials', + content: 'Editorials', + menu: { + items: [ + { key: '1', content: 'item1' }, + { + key: '2', + content: 'item2', + menu: { items: [{ key: '1', content: 'item1' }, { key: '2', content: 'item2' }] }, + }, + ], + }, + }, + { key: 'review', content: 'Reviews' }, + { key: 'events', content: 'Upcoming Events' }, +] + +const MenuExampleWithSubMenu = () => + +export default MenuExampleWithSubMenu diff --git a/docs/src/examples/components/Menu/Types/index.tsx b/docs/src/examples/components/Menu/Types/index.tsx index 8ce62a8f8d..a589b627a9 100644 --- a/docs/src/examples/components/Menu/Types/index.tsx +++ b/docs/src/examples/components/Menu/Types/index.tsx @@ -19,6 +19,16 @@ const Types = () => ( description="A vertical menu displays elements vertically." examplePath="components/Menu/Types/MenuExampleVertical" /> + + ) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 5691f45a8e..da8e91fd8a 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -5,6 +5,8 @@ import * as React from 'react' import { childrenExist, createShorthandFactory, customPropTypes, UIComponent } from '../../lib' import Icon from '../Icon' +import Menu from '../Menu' +import Popup from '../Popup' import { menuItemBehavior } from '../../lib/accessibility' import { Accessibility, AccessibilityActionHandlers } from '../../lib/accessibility/interfaces' import IsFromKeyboard from '../../lib/isFromKeyboard' @@ -29,6 +31,7 @@ export interface IMenuItemProps { icon?: ShorthandValue iconOnly?: boolean index?: number + menu?: any onClick?: ComponentEventHandler pills?: boolean pointing?: boolean | 'start' | 'end' @@ -82,6 +85,9 @@ class MenuItem extends UIComponent, IMenuItemState> { /** MenuItem index inside Menu. */ index: PropTypes.number, + /** MenuItem's submenu */ + menu: PropTypes.any, + /** * Called on click. When passed, the component will render as an `a` * tag by default instead of a `div`. @@ -136,7 +142,7 @@ class MenuItem extends UIComponent, IMenuItemState> { state = IsFromKeyboard.initial renderComponent({ ElementType, classes, accessibility, rest }) { - const { children, content, icon, renderIcon } = this.props + const { children, menu, vertical, type } = this.props return ( , IMenuItemState> { > {childrenExist(children) ? ( children + ) : !menu ? ( + this.renderMenuItem(accessibility, classes) ) : ( - , + styles: { + padding: '0px', + border: '', + }, + }} + // content={} > - {icon && - Icon.create(this.props.icon, { - defaultProps: { xSpacing: !!content ? 'after' : 'none' }, - render: renderIcon, - })} - {content} - + {this.renderMenuItem(accessibility, classes)} + )} ) } - + private renderMenuItem = (accessibility, classes) => { + const { content, icon, renderIcon } = this.props + return ( + + {icon && + Icon.create(this.props.icon, { + defaultProps: { xSpacing: !!content ? 'after' : 'none' }, + render: renderIcon, + })} + {content} + + ) + } protected actionHandlers: AccessibilityActionHandlers = { performClick: event => this.handleClick(event), } From d4a3a0c3b22dcdf6ca80f838f06e213664634729 Mon Sep 17 00:00:00 2001 From: Gopal Goel Date: Mon, 15 Oct 2018 13:30:05 +0530 Subject: [PATCH 02/55] Only one submenu open at a time --- .../MenuExampleWithSubmenu.shorthand.tsx | 20 ++++++++++++++++++- src/components/Menu/Menu.tsx | 20 ++++++++++++++++++- src/components/Menu/MenuItem.tsx | 5 +++-- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx b/docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx index 9005c3ce05..3829e7c908 100644 --- a/docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx +++ b/docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx @@ -5,6 +5,25 @@ const items = [ { key: 'editorials', content: 'Editorials', + menu: { + items: [ + { key: '1', content: 'item1' }, + { + key: '2', + content: 'item2', + menu: { items: [{ key: '1', content: 'item1' }, { key: '2', content: 'item2' }] }, + }, + { + key: '3', + content: 'item3', + menu: { items: [{ key: '1', content: 'item1' }, { key: '2', content: 'item2' }] }, + }, + ], + }, + }, + { + key: 'review', + content: 'Reviews', menu: { items: [ { key: '1', content: 'item1' }, @@ -16,7 +35,6 @@ const items = [ ], }, }, - { key: 'review', content: 'Reviews' }, { key: 'events', content: 'Upcoming Events' }, ] diff --git a/src/components/Menu/Menu.tsx b/src/components/Menu/Menu.tsx index b92919d8fa..c3e4e4eab8 100644 --- a/src/components/Menu/Menu.tsx +++ b/src/components/Menu/Menu.tsx @@ -121,12 +121,27 @@ class Menu extends AutoControlledComponent, any> { static Item = MenuItem + state = { + menuItemOpenKey: 0, + popupOpen: false, + activeIndex: '0', + } + handleItemOverrides = predefinedProps => ({ onClick: (e, itemProps) => { - const { index } = itemProps + const { index, menu } = itemProps this.trySetState({ activeIndex: index }) + if (menu) { + this.setState(prev => { + if (prev.menuItemOpenKey === index) { + return { popupOpen: !prev.popupOpen } + } + return { menuItemOpenKey: index, popupOpen: true } + }) + } + _.invoke(predefinedProps, 'onClick', e, itemProps) }, }) @@ -147,6 +162,9 @@ class Menu extends AutoControlledComponent, any> { vertical, index, active: parseInt(activeIndex, 10) === index, + popupOpen: this.state.popupOpen, + menuItemOpenKey: this.state.menuItemOpenKey, + // togglePopup: this.togglePopup, }, overrideProps: this.handleItemOverrides, render: renderItem, diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index da8e91fd8a..08758896b8 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -142,7 +142,7 @@ class MenuItem extends UIComponent, IMenuItemState> { state = IsFromKeyboard.initial renderComponent({ ElementType, classes, accessibility, rest }) { - const { children, menu, vertical, type } = this.props + const { children, index, menu, menuItemOpenKey, popupOpen, vertical, type } = this.props return ( , IMenuItemState> { this.renderMenuItem(accessibility, classes) ) : ( , IMenuItemState> { border: '', }, }} - // content={} + // content={} > {this.renderMenuItem(accessibility, classes)} From 85981f8a4f78898e1e80f08a84901baf4705e2ad Mon Sep 17 00:00:00 2001 From: Gopal Goel Date: Mon, 15 Oct 2018 13:35:35 +0530 Subject: [PATCH 03/55] Fix bug: submenu noe closes on clicking a menuItem with no submenu --- src/components/Menu/Menu.tsx | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/components/Menu/Menu.tsx b/src/components/Menu/Menu.tsx index c3e4e4eab8..aaef995d0f 100644 --- a/src/components/Menu/Menu.tsx +++ b/src/components/Menu/Menu.tsx @@ -133,14 +133,12 @@ class Menu extends AutoControlledComponent, any> { this.trySetState({ activeIndex: index }) - if (menu) { - this.setState(prev => { - if (prev.menuItemOpenKey === index) { - return { popupOpen: !prev.popupOpen } - } - return { menuItemOpenKey: index, popupOpen: true } - }) - } + this.setState(prev => { + if (prev.menuItemOpenKey === index) { + return { popupOpen: !prev.popupOpen } + } + return { menuItemOpenKey: index, popupOpen: true } + }) _.invoke(predefinedProps, 'onClick', e, itemProps) }, From fa99935df12ab41d6e6c4e080f89bc4ede967dc8 Mon Sep 17 00:00:00 2001 From: Gopal Goel Date: Mon, 15 Oct 2018 14:12:15 +0530 Subject: [PATCH 04/55] Code cleanup --- src/components/Menu/Menu.tsx | 16 +++++++--------- src/components/Menu/MenuItem.tsx | 4 ++-- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/components/Menu/Menu.tsx b/src/components/Menu/Menu.tsx index aaef995d0f..f596aeeb9d 100644 --- a/src/components/Menu/Menu.tsx +++ b/src/components/Menu/Menu.tsx @@ -122,22 +122,23 @@ class Menu extends AutoControlledComponent, any> { static Item = MenuItem state = { - menuItemOpenKey: 0, popupOpen: false, - activeIndex: '0', + activeIndex: '', } handleItemOverrides = predefinedProps => ({ + popupOpen: this.state.popupOpen, + activeIndex: this.state.activeIndex, onClick: (e, itemProps) => { - const { index, menu } = itemProps + const { index } = itemProps - this.trySetState({ activeIndex: index }) + // this.trySetState({ activeIndex: index }) this.setState(prev => { - if (prev.menuItemOpenKey === index) { + if (prev.activeIndex === index) { return { popupOpen: !prev.popupOpen } } - return { menuItemOpenKey: index, popupOpen: true } + return { activeIndex: index, popupOpen: true } }) _.invoke(predefinedProps, 'onClick', e, itemProps) @@ -160,9 +161,6 @@ class Menu extends AutoControlledComponent, any> { vertical, index, active: parseInt(activeIndex, 10) === index, - popupOpen: this.state.popupOpen, - menuItemOpenKey: this.state.menuItemOpenKey, - // togglePopup: this.togglePopup, }, overrideProps: this.handleItemOverrides, render: renderItem, diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 08758896b8..33157571bf 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -142,7 +142,7 @@ class MenuItem extends UIComponent, IMenuItemState> { state = IsFromKeyboard.initial renderComponent({ ElementType, classes, accessibility, rest }) { - const { children, index, menu, menuItemOpenKey, popupOpen, vertical, type } = this.props + const { activeIndex, children, index, menu, popupOpen, vertical, type } = this.props return ( , IMenuItemState> { this.renderMenuItem(accessibility, classes) ) : ( Date: Mon, 15 Oct 2018 14:46:42 +0530 Subject: [PATCH 05/55] Remove the activeIndex prop passed to the menuItem --- src/components/Menu/Menu.tsx | 13 ++++++------- src/components/Menu/MenuItem.tsx | 8 ++++++-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/components/Menu/Menu.tsx b/src/components/Menu/Menu.tsx index f596aeeb9d..186ddd1ba2 100644 --- a/src/components/Menu/Menu.tsx +++ b/src/components/Menu/Menu.tsx @@ -122,25 +122,23 @@ class Menu extends AutoControlledComponent, any> { static Item = MenuItem state = { - popupOpen: false, + submenuOpen: false, activeIndex: '', } handleItemOverrides = predefinedProps => ({ - popupOpen: this.state.popupOpen, - activeIndex: this.state.activeIndex, onClick: (e, itemProps) => { const { index } = itemProps - // this.trySetState({ activeIndex: index }) - this.setState(prev => { if (prev.activeIndex === index) { - return { popupOpen: !prev.popupOpen } + return { submenuOpen: !prev.submenuOpen } } - return { activeIndex: index, popupOpen: true } + return { submenuOpen: true } }) + this.trySetState({ activeIndex: index }) + _.invoke(predefinedProps, 'onClick', e, itemProps) }, }) @@ -161,6 +159,7 @@ class Menu extends AutoControlledComponent, any> { vertical, index, active: parseInt(activeIndex, 10) === index, + submenuOpen: this.state.activeIndex === index ? this.state.submenuOpen : false, }, overrideProps: this.handleItemOverrides, render: renderItem, diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 33157571bf..a65ddcd652 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -36,6 +36,7 @@ export interface IMenuItemProps { pills?: boolean pointing?: boolean | 'start' | 'end' renderIcon?: ShorthandRenderFunction + submenuOpen?: boolean type?: 'primary' | 'secondary' underlined?: boolean vertical?: boolean @@ -106,6 +107,9 @@ class MenuItem extends UIComponent, IMenuItemState> { */ pointing: PropTypes.oneOfType([PropTypes.bool, PropTypes.oneOf(['start', 'end'])]), + /** */ + submenuOpen: PropTypes.bool, + /** The menu can have primary or secondary type */ type: PropTypes.oneOf(['primary', 'secondary']), @@ -142,7 +146,7 @@ class MenuItem extends UIComponent, IMenuItemState> { state = IsFromKeyboard.initial renderComponent({ ElementType, classes, accessibility, rest }) { - const { activeIndex, children, index, menu, popupOpen, vertical, type } = this.props + const { children, menu, submenuOpen, vertical, type } = this.props return ( , IMenuItemState> { this.renderMenuItem(accessibility, classes) ) : ( Date: Tue, 13 Nov 2018 11:45:41 +0530 Subject: [PATCH 06/55] Remove Popup and implement submenu without it --- ...nuExampleVerticalWithSubmenu.shorthand.tsx | 18 ++++- .../MenuExampleWithSubmenu.shorthand.tsx | 18 ++++- src/components/Menu/Menu.tsx | 7 +- src/components/Menu/MenuItem.tsx | 75 +++++++++---------- .../teams/components/Menu/menuVariables.ts | 2 +- 5 files changed, 74 insertions(+), 46 deletions(-) diff --git a/docs/src/examples/components/Menu/Types/MenuExampleVerticalWithSubmenu.shorthand.tsx b/docs/src/examples/components/Menu/Types/MenuExampleVerticalWithSubmenu.shorthand.tsx index 8853a08123..1e8dae4410 100644 --- a/docs/src/examples/components/Menu/Types/MenuExampleVerticalWithSubmenu.shorthand.tsx +++ b/docs/src/examples/components/Menu/Types/MenuExampleVerticalWithSubmenu.shorthand.tsx @@ -1,5 +1,5 @@ import React from 'react' -import { Menu } from '@stardust-ui/react' +import { Menu, Provider } from '@stardust-ui/react' const items = [ { @@ -20,6 +20,20 @@ const items = [ { key: 'events', content: 'Upcoming Events' }, ] -const MenuExampleVerticalWithSubmenu = () => +const MenuExampleVerticalWithSubmenu = () => ( + + + +) export default MenuExampleVerticalWithSubmenu diff --git a/docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx b/docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx index 3829e7c908..187296aa78 100644 --- a/docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx +++ b/docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx @@ -1,5 +1,5 @@ import React from 'react' -import { Menu } from '@stardust-ui/react' +import { Menu, Provider } from '@stardust-ui/react' const items = [ { @@ -38,6 +38,20 @@ const items = [ { key: 'events', content: 'Upcoming Events' }, ] -const MenuExampleWithSubMenu = () => +const MenuExampleWithSubMenu = () => ( + + + +) export default MenuExampleWithSubMenu diff --git a/src/components/Menu/Menu.tsx b/src/components/Menu/Menu.tsx index 186ddd1ba2..0aea78c915 100644 --- a/src/components/Menu/Menu.tsx +++ b/src/components/Menu/Menu.tsx @@ -145,7 +145,7 @@ class Menu extends AutoControlledComponent, any> { renderItems = (variables: ComponentVariablesObject) => { const { iconOnly, items, pills, pointing, renderItem, type, underlined, vertical } = this.props - const { activeIndex } = this.state + const { activeIndex, submenuOpen } = this.state return _.map(items, (item, index) => MenuItem.create(item, { @@ -159,7 +159,10 @@ class Menu extends AutoControlledComponent, any> { vertical, index, active: parseInt(activeIndex, 10) === index, - submenuOpen: this.state.activeIndex === index ? this.state.submenuOpen : false, + ...(activeIndex === index && { submenuOpen }), + ...(item.menu && { + styles: { position: 'relative' }, + }), }, overrideProps: this.handleItemOverrides, render: renderItem, diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index a65ddcd652..bdce6028e2 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -6,7 +6,7 @@ import * as React from 'react' import { childrenExist, createShorthandFactory, customPropTypes, UIComponent } from '../../lib' import Icon from '../Icon' import Menu from '../Menu' -import Popup from '../Popup' +// import Provider from '../Provider' import { menuItemBehavior } from '../../lib/accessibility' import { Accessibility, AccessibilityActionHandlers } from '../../lib/accessibility/interfaces' import IsFromKeyboard from '../../lib/isFromKeyboard' @@ -146,7 +146,7 @@ class MenuItem extends UIComponent, IMenuItemState> { state = IsFromKeyboard.initial renderComponent({ ElementType, classes, accessibility, rest }) { - const { children, menu, submenuOpen, vertical, type } = this.props + const { children } = this.props return ( , IMenuItemState> { {...accessibility.keyHandlers.root} {...rest} > - {childrenExist(children) ? ( - children - ) : !menu ? ( - this.renderMenuItem(accessibility, classes) - ) : ( - , - styles: { - padding: '0px', - border: '', - }, - }} - // content={} - > - {this.renderMenuItem(accessibility, classes)} - - )} + {childrenExist(children) ? children : this.renderMenuItem(accessibility, classes)} ) } private renderMenuItem = (accessibility, classes) => { - const { content, icon, renderIcon } = this.props + const { content, icon, renderIcon, menu, type, vertical, submenuOpen } = this.props return ( - - {icon && - Icon.create(this.props.icon, { - defaultProps: { xSpacing: !!content ? 'after' : 'none' }, - render: renderIcon, - })} - {content} - + <> + + {icon && + Icon.create(this.props.icon, { + defaultProps: { xSpacing: !!content ? 'after' : 'none' }, + render: renderIcon, + })} + {content} + + {menu && submenuOpen ? ( + + ) : null} + ) } protected actionHandlers: AccessibilityActionHandlers = { diff --git a/src/themes/teams/components/Menu/menuVariables.ts b/src/themes/teams/components/Menu/menuVariables.ts index d299a5356e..95bfc59ee4 100644 --- a/src/themes/teams/components/Menu/menuVariables.ts +++ b/src/themes/teams/components/Menu/menuVariables.ts @@ -24,7 +24,7 @@ export interface IMenuVariables { export default (siteVars: any): IMenuVariables => { return { defaultColor: siteVars.gray02, - defaultBackgroundColor: 'transparent', + defaultBackgroundColor: '#FFF', defaultActiveColor: siteVars.black, defaultActiveBackgroundColor: siteVars.gray10, From bea260a87704d7c78bdf15abe92d678b4fea23b5 Mon Sep 17 00:00:00 2001 From: Juraj Kapsiar Date: Thu, 29 Nov 2018 05:35:11 +0100 Subject: [PATCH 07/55] initial keyboard support --- .../MenuExampleWithSubmenu.shorthand.tsx | 6 +- src/components/Menu/Menu.tsx | 18 ++---- src/components/Menu/MenuItem.tsx | 64 ++++++++++++++++--- .../Behaviors/Menu/menuItemBehavior.ts | 8 +++ .../Behaviors/Menu/submenuBehavior.ts | 31 +++++++++ src/lib/accessibility/index.ts | 1 + .../teams/components/Menu/menuStyles.ts | 2 +- test/specs/behaviors/behavior-test.tsx | 2 + 8 files changed, 104 insertions(+), 28 deletions(-) create mode 100644 src/lib/accessibility/Behaviors/Menu/submenuBehavior.ts diff --git a/docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx b/docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx index 187296aa78..d2d195cc3b 100644 --- a/docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx +++ b/docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx @@ -11,12 +11,12 @@ const items = [ { key: '2', content: 'item2', - menu: { items: [{ key: '1', content: 'item1' }, { key: '2', content: 'item2' }] }, + menu: { items: [{ key: '1', content: 'item2.1' }, { key: '2', content: 'item2.2' }] }, }, { key: '3', content: 'item3', - menu: { items: [{ key: '1', content: 'item1' }, { key: '2', content: 'item2' }] }, + menu: { items: [{ key: '1', content: 'item3.1' }, { key: '2', content: 'item3.2' }] }, }, ], }, @@ -30,7 +30,7 @@ const items = [ { key: '2', content: 'item2', - menu: { items: [{ key: '1', content: 'item1' }, { key: '2', content: 'item2' }] }, + menu: { items: [{ key: '1', content: 'item2.1' }, { key: '2', content: 'item2.2' }] }, }, ], }, diff --git a/src/components/Menu/Menu.tsx b/src/components/Menu/Menu.tsx index 6c6b6a4f79..30e313d2a7 100644 --- a/src/components/Menu/Menu.tsx +++ b/src/components/Menu/Menu.tsx @@ -109,7 +109,6 @@ class Menu extends AutoControlledComponent, any> { static Item = MenuItem state = { - submenuOpen: false, activeIndex: '', } @@ -117,13 +116,6 @@ class Menu extends AutoControlledComponent, any> { onClick: (e, itemProps) => { const { index } = itemProps - this.setState(prev => { - if (prev.activeIndex === index) { - return { submenuOpen: !prev.submenuOpen } - } - return { submenuOpen: true } - }) - this.trySetState({ activeIndex: index }) _.invoke(predefinedProps, 'onClick', e, itemProps) @@ -142,7 +134,7 @@ class Menu extends AutoControlledComponent, any> { underlined, vertical, } = this.props - const { activeIndex, submenuOpen } = this.state + const { activeIndex } = this.state return _.map(items, (item, index) => { const active = parseInt(activeIndex, 10) === index @@ -158,11 +150,9 @@ class Menu extends AutoControlledComponent, any> { vertical, index, active, - ...(active && { submenuOpen }), - ...(active && - submenuOpen && { - styles: { position: 'relative' }, - }), + ...(active && { + styles: { position: 'relative' }, + }), }, overrideProps: this.handleItemOverrides, render: renderItem, diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 051f0abaaf..e1d7ca86e3 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -3,11 +3,16 @@ import * as cx from 'classnames' import * as PropTypes from 'prop-types' import * as React from 'react' -import { childrenExist, createShorthandFactory, customPropTypes, UIComponent } from '../../lib' +import { + childrenExist, + createShorthandFactory, + customPropTypes, + AutoControlledComponent, +} from '../../lib' import Icon from '../Icon/Icon' import Menu from '../Menu/Menu' import Slot from '../Slot/Slot' -import { menuItemBehavior } from '../../lib/accessibility' +import { menuItemBehavior, submenuBehavior } from '../../lib/accessibility' import { Accessibility, AccessibilityActionHandlers } from '../../lib/accessibility/types' import IsFromKeyboard from '../../lib/isFromKeyboard' @@ -27,6 +32,7 @@ import { childrenComponentPropTypes, contentComponentPropsTypes, } from '../../lib/commonPropTypes' +import { focusAsync } from 'src/lib/accessibility/FocusZone' export interface MenuItemProps extends UIComponentProps, @@ -109,16 +115,20 @@ export interface MenuItemProps /** Indicates if the submenu is open */ submenuOpen?: boolean + + /** Default submenu open */ + defaultSubmenuOpen?: boolean } export interface MenuItemState { [IsFromKeyboard.propertyName]: boolean + submenuOpen: boolean } /** * A menu item is an actionable navigation item within a menu. */ -class MenuItem extends UIComponent, MenuItemState> { +class MenuItem extends AutoControlledComponent, MenuItemState> { static displayName = 'MenuItem' static className = 'ui-menu__item' @@ -147,6 +157,7 @@ class MenuItem extends UIComponent, MenuItemState> { renderWrapper: PropTypes.func, menu: customPropTypes.itemShorthand, submenuOpen: PropTypes.bool, + defaultSubmenuOpen: PropTypes.bool, } static defaultProps = { @@ -155,7 +166,14 @@ class MenuItem extends UIComponent, MenuItemState> { wrapper: { as: 'li' }, } - state = IsFromKeyboard.initial + static autoControlledProps = ['submenuOpen'] + + state = { + ...IsFromKeyboard.initial, + submenuOpen: false, + } + + private itemRef = React.createRef() renderComponent({ ElementType, classes, accessibility, rest }) { const { @@ -166,11 +184,13 @@ class MenuItem extends UIComponent, MenuItemState> { renderWrapper, wrapper, menu, - submenuOpen, type, vertical, + active, } = this.props + const { submenuOpen } = this.state + const menuItemInner = childrenExist(children) ? ( children ) : ( @@ -182,6 +202,7 @@ class MenuItem extends UIComponent, MenuItemState> { {...accessibility.attributes.anchor} {...accessibility.keyHandlers.anchor} {...rest} + ref={this.itemRef} > {icon && Icon.create(this.props.icon, { @@ -193,10 +214,10 @@ class MenuItem extends UIComponent, MenuItemState> { ) const maybeSubmenu = - menu && submenuOpen + menu && active && submenuOpen ? Menu.create(menu, { overrideProps: { - accessibility: null, + accessibility: submenuBehavior, vertical: true, type, styles: { @@ -214,12 +235,12 @@ class MenuItem extends UIComponent, MenuItemState> { return Slot.create(wrapper, { defaultProps: { className: cx('ui-menu__item__wrapper', classes.wrapper), - ...accessibility.attributes.root, - ...accessibility.keyHandlers.root, + ...accessibility.attributes.wrapper, + ...accessibility.keyHandlers.wrapper, }, render: renderWrapper, overrideProps: () => ({ - children: [menuItemInner, maybeSubmenu], + children: [menuItemInner, menu ? '>' : undefined, maybeSubmenu], }), }) } @@ -228,9 +249,15 @@ class MenuItem extends UIComponent, MenuItemState> { protected actionHandlers: AccessibilityActionHandlers = { performClick: event => this.handleClick(event), + closeMenu: event => this.closeMenu(event), + closeSubmenu: event => this.closeSubmenu(event), } private handleClick = e => { + const { active, menu } = this.props + if (menu) { + this.setState({ submenuOpen: active ? !this.state.submenuOpen : true }) + } _.invoke(this.props, 'onClick', e, this.props) } @@ -245,6 +272,23 @@ class MenuItem extends UIComponent, MenuItemState> { _.invoke(this.props, 'onFocus', e, this.props) } + + private closeMenu = e => { + const { menu } = this.props + const { submenuOpen } = this.state + if (menu && submenuOpen) { + this.setState({ submenuOpen: false }, () => focusAsync(this.itemRef.current)) + } + } + + private closeSubmenu = e => { + const { menu } = this.props + const { submenuOpen } = this.state + if (menu && submenuOpen) { + this.setState({ submenuOpen: false }, () => focusAsync(this.itemRef.current)) + e.stopPropagation() + } + } } MenuItem.create = createShorthandFactory(MenuItem, 'content') diff --git a/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts b/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts index f00c978834..8899b1f1f0 100644 --- a/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts +++ b/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts @@ -39,6 +39,14 @@ const menuItemBehavior: Accessibility = (props: any) => ({ keyCombinations: [{ keyCode: keyboardKey.Enter }, { keyCode: keyboardKey.Spacebar }], }, }, + wrapper: { + closeSubmenu: { + keyCombinations: [{ keyCode: keyboardKey.ArrowLeft }], + }, + closeMenu: { + keyCombinations: [{ keyCode: keyboardKey.Escape }], + }, + }, }, }) diff --git a/src/lib/accessibility/Behaviors/Menu/submenuBehavior.ts b/src/lib/accessibility/Behaviors/Menu/submenuBehavior.ts new file mode 100644 index 0000000000..2ef65e9e3e --- /dev/null +++ b/src/lib/accessibility/Behaviors/Menu/submenuBehavior.ts @@ -0,0 +1,31 @@ +import { Accessibility, FocusZoneMode } from '../../types' +import { FocusZoneDirection } from '../../FocusZone' + +/** + * @description + * The 'menu' role is used to identify an element that creates a list of common actions or functions that a user can invoke. + * + * @specification + * Adds role='menu'. + * Embeds FocusZone into component allowing circular arrow key navigation through the children of the component. + */ + +const submenuBehavior: Accessibility = (props: any) => ({ + attributes: { + root: { + role: 'menu', + }, + }, + focusZone: { + mode: FocusZoneMode.Embed, + props: { + isCircularNavigation: true, + preventDefaultWhenHandled: true, + shouldFocusFirstElementWhenReceivedFocus: true, + shouldFocusOnMount: true, + direction: FocusZoneDirection.vertical, + }, + }, +}) + +export default submenuBehavior diff --git a/src/lib/accessibility/index.ts b/src/lib/accessibility/index.ts index fc8c7f4904..047279ed5b 100644 --- a/src/lib/accessibility/index.ts +++ b/src/lib/accessibility/index.ts @@ -5,6 +5,7 @@ export { default as toggleButtonBehavior } from './Behaviors/Button/toggleButton export { default as imageBehavior } from './Behaviors/Image/imageBehavior' export { default as menuBehavior } from './Behaviors/Menu/menuBehavior' export { default as menuItemBehavior } from './Behaviors/Menu/menuItemBehavior' +export { default as submenuBehavior } from './Behaviors/Menu/submenuBehavior' export { default as basicListBehavior } from './Behaviors/List/listBehavior' export { default as basicListItemBehavior } from './Behaviors/List/basicListItemBehavior' export { default as listBehavior } from './Behaviors/List/listBehavior' diff --git a/src/themes/teams/components/Menu/menuStyles.ts b/src/themes/teams/components/Menu/menuStyles.ts index 46f8f9e0c6..3f9a439656 100644 --- a/src/themes/teams/components/Menu/menuStyles.ts +++ b/src/themes/teams/components/Menu/menuStyles.ts @@ -28,7 +28,7 @@ export default { ...solidBorder(variables.primaryBorderColor), }), borderRadius: pxToRem(4), - overflow: 'hidden', + // overflow: 'hidden', }), ...(underlined && { borderBottom: `2px solid ${variables.primaryUnderlinedBorderColor}`, diff --git a/test/specs/behaviors/behavior-test.tsx b/test/specs/behaviors/behavior-test.tsx index bc38e2ddc5..9119d3e3cc 100644 --- a/test/specs/behaviors/behavior-test.tsx +++ b/test/specs/behaviors/behavior-test.tsx @@ -14,6 +14,7 @@ import { inputBehavior, menuBehavior, menuItemBehavior, + submenuBehavior, popupBehavior, popupFocusTrapBehavior, dialogBehavior, @@ -44,6 +45,7 @@ testHelper.addBehavior('inputBehavior', inputBehavior) testHelper.addBehavior('imageBehavior', imageBehavior) testHelper.addBehavior('menuBehavior', menuBehavior) testHelper.addBehavior('menuItemBehavior', menuItemBehavior) +testHelper.addBehavior('submenuBehavior', submenuBehavior) testHelper.addBehavior('popupBehavior', popupBehavior) testHelper.addBehavior('popupFocusTrapBehavior', popupFocusTrapBehavior) testHelper.addBehavior('radioGroupBehavior', radioGroupBehavior) From 8f678ece86e66330655649026157604eecef90dd Mon Sep 17 00:00:00 2001 From: Juraj Kapsiar Date: Thu, 29 Nov 2018 07:14:32 +0100 Subject: [PATCH 08/55] left/right arrow handling --- src/components/Menu/MenuItem.tsx | 29 +++++++++++++++---- .../Behaviors/Menu/menuItemBehavior.ts | 21 +++++++++++--- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index e1d7ca86e3..fae70aa22e 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -249,8 +249,9 @@ class MenuItem extends AutoControlledComponent, MenuIt protected actionHandlers: AccessibilityActionHandlers = { performClick: event => this.handleClick(event), + openVerticalSubmenu: event => this.openVerticalSubmenu(event), + openHorizontalSubmenu: event => this.openHorizontalSubmenu(event), closeMenu: event => this.closeMenu(event), - closeSubmenu: event => this.closeSubmenu(event), } private handleClick = e => { @@ -281,11 +282,29 @@ class MenuItem extends AutoControlledComponent, MenuIt } } - private closeSubmenu = e => { - const { menu } = this.props + // private closeSubmenu = e => { + // const { menu } = this.props + // const { submenuOpen } = this.state + // if (menu && submenuOpen) { + // this.setState({ submenuOpen: false }, () => focusAsync(this.itemRef.current)) + // e.stopPropagation() + // } + // } + + private openVerticalSubmenu = e => { + const { menu, vertical } = this.props const { submenuOpen } = this.state - if (menu && submenuOpen) { - this.setState({ submenuOpen: false }, () => focusAsync(this.itemRef.current)) + if (menu && vertical && !submenuOpen) { + this.handleClick(e) + e.stopPropagation() + } + } + + private openHorizontalSubmenu = e => { + const { menu, vertical } = this.props + const { submenuOpen } = this.state + if (menu && !vertical && !submenuOpen) { + this.handleClick(e) e.stopPropagation() } } diff --git a/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts b/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts index 8899b1f1f0..1e8db21de0 100644 --- a/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts +++ b/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts @@ -38,13 +38,26 @@ const menuItemBehavior: Accessibility = (props: any) => ({ performClick: { keyCombinations: [{ keyCode: keyboardKey.Enter }, { keyCode: keyboardKey.Spacebar }], }, + openVerticalSubmenu: { + keyCombinations: [{ keyCode: keyboardKey.ArrowRight }], + }, + openHorizontalSubmenu: { + keyCombinations: [{ keyCode: keyboardKey.ArrowDown }], + }, }, wrapper: { - closeSubmenu: { - keyCombinations: [{ keyCode: keyboardKey.ArrowLeft }], - }, + // closeSubmenu: { + // keyCombinations: [{ keyCode: keyboardKey.ArrowLeft }, { keyCode: keyboardKey.ArrowRight }], + // }, + // closeMenu: { + // keyCombinations: [{ keyCode: keyboardKey.Escape }], + // }, closeMenu: { - keyCombinations: [{ keyCode: keyboardKey.Escape }], + keyCombinations: [ + { keyCode: keyboardKey.ArrowLeft }, + { keyCode: keyboardKey.ArrowRight }, + { keyCode: keyboardKey.Escape }, + ], }, }, }, From 0f4be8bcdc3d5d57eaf134a1e8257124753454c7 Mon Sep 17 00:00:00 2001 From: Juraj Kapsiar Date: Thu, 29 Nov 2018 08:32:16 +0100 Subject: [PATCH 09/55] simplify keyboard handlers --- src/components/Menu/MenuItem.tsx | 30 +++++++------------ .../Behaviors/Menu/menuItemBehavior.ts | 25 ++++++---------- 2 files changed, 20 insertions(+), 35 deletions(-) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index fae70aa22e..b550b9a224 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -240,7 +240,7 @@ class MenuItem extends AutoControlledComponent, MenuIt }, render: renderWrapper, overrideProps: () => ({ - children: [menuItemInner, menu ? '>' : undefined, maybeSubmenu], + children: [menuItemInner, maybeSubmenu], }), }) } @@ -249,9 +249,9 @@ class MenuItem extends AutoControlledComponent, MenuIt protected actionHandlers: AccessibilityActionHandlers = { performClick: event => this.handleClick(event), - openVerticalSubmenu: event => this.openVerticalSubmenu(event), - openHorizontalSubmenu: event => this.openHorizontalSubmenu(event), + openSubmenu: event => this.openSubmenu(event), closeMenu: event => this.closeMenu(event), + closeSubmenu: event => this.closeSubmenu(event), } private handleClick = e => { @@ -282,30 +282,22 @@ class MenuItem extends AutoControlledComponent, MenuIt } } - // private closeSubmenu = e => { - // const { menu } = this.props - // const { submenuOpen } = this.state - // if (menu && submenuOpen) { - // this.setState({ submenuOpen: false }, () => focusAsync(this.itemRef.current)) - // e.stopPropagation() - // } - // } - - private openVerticalSubmenu = e => { - const { menu, vertical } = this.props + private closeSubmenu = e => { + const { menu } = this.props const { submenuOpen } = this.state - if (menu && vertical && !submenuOpen) { - this.handleClick(e) + if (menu && submenuOpen) { + this.setState({ submenuOpen: false }, () => focusAsync(this.itemRef.current)) e.stopPropagation() } } - private openHorizontalSubmenu = e => { - const { menu, vertical } = this.props + private openSubmenu = e => { + const { menu } = this.props const { submenuOpen } = this.state - if (menu && !vertical && !submenuOpen) { + if (menu && !submenuOpen) { this.handleClick(e) e.stopPropagation() + e.preventDefault() } } } diff --git a/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts b/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts index 1e8db21de0..c3463ced39 100644 --- a/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts +++ b/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts @@ -38,26 +38,19 @@ const menuItemBehavior: Accessibility = (props: any) => ({ performClick: { keyCombinations: [{ keyCode: keyboardKey.Enter }, { keyCode: keyboardKey.Spacebar }], }, - openVerticalSubmenu: { - keyCombinations: [{ keyCode: keyboardKey.ArrowRight }], - }, - openHorizontalSubmenu: { - keyCombinations: [{ keyCode: keyboardKey.ArrowDown }], + + openSubmenu: { + keyCombinations: [ + { keyCode: props.vertical ? keyboardKey.ArrowRight : keyboardKey.ArrowDown }, + ], }, }, wrapper: { - // closeSubmenu: { - // keyCombinations: [{ keyCode: keyboardKey.ArrowLeft }, { keyCode: keyboardKey.ArrowRight }], - // }, - // closeMenu: { - // keyCombinations: [{ keyCode: keyboardKey.Escape }], - // }, closeMenu: { - keyCombinations: [ - { keyCode: keyboardKey.ArrowLeft }, - { keyCode: keyboardKey.ArrowRight }, - { keyCode: keyboardKey.Escape }, - ], + keyCombinations: [{ keyCode: keyboardKey.Escape }, { keyCode: keyboardKey.ArrowRight }], + }, + closeSubmenu: { + keyCombinations: [{ keyCode: keyboardKey.ArrowLeft }], }, }, }, From 46d52c6e04d4013b09f5581b0cac83a6db9ed7c2 Mon Sep 17 00:00:00 2001 From: manajdov Date: Fri, 30 Nov 2018 11:37:33 +0100 Subject: [PATCH 10/55] -fixed import in the MenuItem --- src/components/Menu/MenuItem.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index b550b9a224..931b1cb5a7 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -32,7 +32,7 @@ import { childrenComponentPropTypes, contentComponentPropsTypes, } from '../../lib/commonPropTypes' -import { focusAsync } from 'src/lib/accessibility/FocusZone' +import { focusAsync } from '../../lib/accessibility/FocusZone' export interface MenuItemProps extends UIComponentProps, From c1ad2da96f0edca8e18979c2ea9f61dc92c85979 Mon Sep 17 00:00:00 2001 From: manajdov Date: Fri, 30 Nov 2018 15:29:14 +0100 Subject: [PATCH 11/55] -added submenuIndicator -small fixes in the styles and the way the submenu is generated --- src/components/Menu/MenuItem.tsx | 19 +++++++----------- .../teams/components/Menu/menuItemStyles.ts | 20 +++++++++++++++++++ .../teams/components/Menu/menuVariables.ts | 6 ++++++ 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 931b1cb5a7..01924d8010 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -175,7 +175,7 @@ class MenuItem extends AutoControlledComponent, MenuIt private itemRef = React.createRef() - renderComponent({ ElementType, classes, accessibility, rest }) { + renderComponent({ ElementType, classes, accessibility, rest, styles }) { const { children, content, @@ -184,8 +184,8 @@ class MenuItem extends AutoControlledComponent, MenuIt renderWrapper, wrapper, menu, - type, - vertical, + primary, + secondary, active, } = this.props @@ -216,17 +216,12 @@ class MenuItem extends AutoControlledComponent, MenuIt const maybeSubmenu = menu && active && submenuOpen ? Menu.create(menu, { - overrideProps: { + defaultProps: { accessibility: submenuBehavior, vertical: true, - type, - styles: { - // background: 'white', - // zIndex: '1000', - position: 'absolute', - top: vertical ? '0' : '100%', - left: vertical ? '100%' : '0', - }, + primary, + secondary, + styles: styles.menu, }, }) : null diff --git a/src/themes/teams/components/Menu/menuItemStyles.ts b/src/themes/teams/components/Menu/menuItemStyles.ts index 2f2263e12e..51ab050629 100644 --- a/src/themes/teams/components/Menu/menuItemStyles.ts +++ b/src/themes/teams/components/Menu/menuItemStyles.ts @@ -266,8 +266,28 @@ const menuItemStyles: ComponentSlotStylesInput ({ + // background: 'white', + // zIndex: '1000', + position: 'absolute', + top: vertical ? '0' : '100%', + left: vertical ? '100%' : '0', + }), } export default menuItemStyles diff --git a/src/themes/teams/components/Menu/menuVariables.ts b/src/themes/teams/components/Menu/menuVariables.ts index 0aeccf9706..3713639ccc 100644 --- a/src/themes/teams/components/Menu/menuVariables.ts +++ b/src/themes/teams/components/Menu/menuVariables.ts @@ -19,6 +19,9 @@ export interface MenuVariables { iconsMenuItemSize?: string circularRadius: string lineHeightBase: string + + submenuIndicatorContent: string + submenuIndicatorRotationAngle: number } export default (siteVars: any): MenuVariables => { @@ -41,5 +44,8 @@ export default (siteVars: any): MenuVariables => { iconsMenuItemSize: pxToRem(32), circularRadius: pxToRem(999), lineHeightBase: siteVars.lineHeightBase, + + submenuIndicatorContent: '">"', + submenuIndicatorRotationAngle: 90, } } From acee5ee87d095e3100d34255ec6da1ce247afc2f Mon Sep 17 00:00:00 2001 From: manajdov Date: Fri, 30 Nov 2018 17:08:15 +0100 Subject: [PATCH 12/55] -clicking on leaf element should close the submenus (the same should be done for enter/space) -applied consistent (left-right) navigation for horizontal menu and (up-down) navigation for vertical menu --- src/components/Menu/Menu.tsx | 26 +++++++++++++++++-- src/components/Menu/MenuItem.tsx | 13 ++++++++-- .../Behaviors/Menu/menuBehavior.ts | 3 +++ 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/components/Menu/Menu.tsx b/src/components/Menu/Menu.tsx index 30e313d2a7..f4ac341333 100644 --- a/src/components/Menu/Menu.tsx +++ b/src/components/Menu/Menu.tsx @@ -13,7 +13,12 @@ import { menuBehavior } from '../../lib/accessibility' import { Accessibility } from '../../lib/accessibility/types' import { ComponentVariablesObject } from '../../themes/types' -import { Extendable, ShorthandRenderFunction, ShorthandValue } from '../../../types/utils' +import { + Extendable, + ShorthandRenderFunction, + ShorthandValue, + ComponentEventHandler, +} from '../../../types/utils' import { UIComponentProps, ChildrenComponentProps } from '../../lib/commonPropInterfaces' import { commonUIComponentPropTypes, childrenComponentPropTypes } from '../../lib/commonPropTypes' @@ -39,6 +44,14 @@ export interface MenuProps extends UIComponentProps, ChildrenComponent /** Shorthand array of props for Menu. */ items?: ShorthandValue[] + /** + * Called on click. + * + * @param {SyntheticEvent} event - React's original SyntheticEvent. + * @param {object} data - All props. + */ + onClick?: ComponentEventHandler + /** A menu can adjust its appearance to de-emphasize its contents. */ pills?: boolean @@ -163,11 +176,20 @@ class Menu extends AutoControlledComponent, any> { renderComponent({ ElementType, classes, accessibility, variables, rest }) { const { children } = this.props return ( - + {childrenExist(children) ? children : this.renderItems(variables)} ) } + + private handleClick = (e: React.SyntheticEvent) => { + _.invoke(this.props, 'onClick', e, { ...this.props, ...this.state }) + } } Menu.create = createShorthandFactory(Menu, 'items') diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 01924d8010..9861b6f72f 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -10,7 +10,7 @@ import { AutoControlledComponent, } from '../../lib' import Icon from '../Icon/Icon' -import Menu from '../Menu/Menu' +import Menu, { MenuProps } from '../Menu/Menu' import Slot from '../Slot/Slot' import { menuItemBehavior, submenuBehavior } from '../../lib/accessibility' import { Accessibility, AccessibilityActionHandlers } from '../../lib/accessibility/types' @@ -223,6 +223,9 @@ class MenuItem extends AutoControlledComponent, MenuIt secondary, styles: styles.menu, }, + overrideProps: { + onClick: this.handleSubmenuClicked, + }, }) : null @@ -252,7 +255,8 @@ class MenuItem extends AutoControlledComponent, MenuIt private handleClick = e => { const { active, menu } = this.props if (menu) { - this.setState({ submenuOpen: active ? !this.state.submenuOpen : true }) + this.trySetState({ submenuOpen: active ? !this.state.submenuOpen : true }) + e.stopPropagation() } _.invoke(this.props, 'onClick', e, this.props) } @@ -295,6 +299,11 @@ class MenuItem extends AutoControlledComponent, MenuIt e.preventDefault() } } + + private handleSubmenuClicked = (e: React.SyntheticEvent, props: MenuProps) => { + this.trySetState({ submenuOpen: false }) + _.invoke(props, 'onClick', props) + } } MenuItem.create = createShorthandFactory(MenuItem, 'content') diff --git a/src/lib/accessibility/Behaviors/Menu/menuBehavior.ts b/src/lib/accessibility/Behaviors/Menu/menuBehavior.ts index 377936f75f..e25b3d305e 100644 --- a/src/lib/accessibility/Behaviors/Menu/menuBehavior.ts +++ b/src/lib/accessibility/Behaviors/Menu/menuBehavior.ts @@ -1,4 +1,5 @@ import { Accessibility, FocusZoneMode } from '../../types' +import { FocusZoneDirection } from '../../FocusZone' /** * @description @@ -21,6 +22,8 @@ const menuBehavior: Accessibility = (props: any) => ({ isCircularNavigation: true, preventDefaultWhenHandled: true, shouldFocusFirstElementWhenReceivedFocus: true, + // TODO: check if this should be applied to other behaviors as well + direction: props.vertical ? FocusZoneDirection.vertical : FocusZoneDirection.horizontal, }, }, }) From 5575653335b79d11276d78d998271f743f57e111 Mon Sep 17 00:00:00 2001 From: manajdov Date: Fri, 7 Dec 2018 15:13:03 +0100 Subject: [PATCH 13/55] -implemented outside click to close all menus -implemented enter key on leaf menu item to close the menu --- src/components/Menu/MenuItem.tsx | 59 +++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 51176300b2..c746989017 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -2,6 +2,7 @@ import * as _ from 'lodash' import * as cx from 'classnames' import * as PropTypes from 'prop-types' import * as React from 'react' +import * as keyboardKey from 'keyboard-key' import { AutoControlledComponent, @@ -12,6 +13,7 @@ import { ChildrenComponentProps, ContentComponentProps, commonPropTypes, + EventStack, } from '../../lib' import Icon from '../Icon/Icon' import Menu, { MenuProps } from '../Menu/Menu' @@ -21,6 +23,7 @@ import { Accessibility, AccessibilityActionHandlers } from '../../lib/accessibil import IsFromKeyboard from '../../lib/isFromKeyboard' import { ComponentEventHandler, Extendable, ShorthandValue } from '../../../types/utils' import { focusAsync } from '../../lib/accessibility/FocusZone' +import { Ref } from '@stardust-ui/react' export interface MenuItemProps extends UIComponentProps, @@ -139,8 +142,23 @@ class MenuItem extends AutoControlledComponent, MenuIt submenuOpen: false, } + private outsideClickSubscription = EventStack.noSubscription + + private submenuDomElement = null private itemRef = React.createRef() + public componentDidMount() { + this.updateOutsideClickSubscription() + } + + public componentDidUpdate() { + this.updateOutsideClickSubscription() + } + + public componentWillUnmount() { + this.outsideClickSubscription.unsubscribe() + } + renderComponent({ ElementType, classes, accessibility, rest, styles }) { const { children, content, icon, wrapper, menu, primary, secondary, active } = this.props @@ -179,10 +197,21 @@ class MenuItem extends AutoControlledComponent, MenuIt }, overrideProps: { onClick: this.handleSubmenuClicked, + onKeyDown: this.handleSubmenuKeyDown, }, }) : null + const maybeSubmenuWithRef = maybeSubmenu ? ( + { + this.submenuDomElement = domElement + }} + > + {maybeSubmenu} + + ) : null + if (wrapper) { return Slot.create(wrapper, { defaultProps: { @@ -191,7 +220,7 @@ class MenuItem extends AutoControlledComponent, MenuIt ...accessibility.keyHandlers.wrapper, }, overrideProps: () => ({ - children: [menuItemInner, maybeSubmenu], + children: [menuItemInner, maybeSubmenuWithRef], }), }) } @@ -205,6 +234,24 @@ class MenuItem extends AutoControlledComponent, MenuIt closeSubmenu: event => this.closeSubmenu(event), } + private updateOutsideClickSubscription() { + this.outsideClickSubscription.unsubscribe() + + if (this.props.menu && this.state.submenuOpen) { + setTimeout(() => { + this.outsideClickSubscription = EventStack.subscribe('click', e => { + if ( + this.itemRef && + (!this.itemRef.current || !this.itemRef.current.contains(e.target)) && + (!this.submenuDomElement || !this.submenuDomElement.contains(e.target)) + ) { + this.state.submenuOpen && this.trySetState({ submenuOpen: false }) + } + }) + }) + } + } + private handleClick = e => { const { active, menu } = this.props if (menu) { @@ -257,6 +304,16 @@ class MenuItem extends AutoControlledComponent, MenuIt this.trySetState({ submenuOpen: false }) _.invoke(props, 'onClick', props) } + + private handleSubmenuKeyDown = (e: React.SyntheticEvent, props: MenuProps) => { + if ( + keyboardKey.getCode(e) === keyboardKey.Enter || + keyboardKey.getCode(e) === keyboardKey.Spacebar + ) { + this.trySetState({ submenuOpen: false }) + } + _.invoke(props, 'onKeyDown', props) + } } MenuItem.create = createShorthandFactory(MenuItem, 'content') From 7a17afb3d0ab3645182254a72945ab8de3e1245d Mon Sep 17 00:00:00 2001 From: manajdov Date: Fri, 7 Dec 2018 15:36:58 +0100 Subject: [PATCH 14/55] -fixed import --- src/components/Menu/MenuItem.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index c746989017..a6f7f51552 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -17,13 +17,13 @@ import { } from '../../lib' import Icon from '../Icon/Icon' import Menu, { MenuProps } from '../Menu/Menu' +import Ref from '../Ref/Ref' import Slot from '../Slot/Slot' import { menuItemBehavior, submenuBehavior } from '../../lib/accessibility' import { Accessibility, AccessibilityActionHandlers } from '../../lib/accessibility/types' import IsFromKeyboard from '../../lib/isFromKeyboard' import { ComponentEventHandler, Extendable, ShorthandValue } from '../../../types/utils' import { focusAsync } from '../../lib/accessibility/FocusZone' -import { Ref } from '@stardust-ui/react' export interface MenuItemProps extends UIComponentProps, From 62620ae937172bf38489b3f3c017ae3bb12a1e7b Mon Sep 17 00:00:00 2001 From: manajdov Date: Tue, 11 Dec 2018 17:56:09 +0100 Subject: [PATCH 15/55] -refactored MenuItem handlers - fixed issues -removed onClick handler for the Menu (not necessary for now) -added onKeyDown in the creation of the MenuItem in the Menu component for handling the action prop --- src/components/Menu/Menu.tsx | 35 ++++++------ src/components/Menu/MenuItem.tsx | 54 +++++++++---------- .../Behaviors/Menu/menuItemBehavior.ts | 16 +++--- src/lib/getKeyDownHandlers.ts | 3 +- 4 files changed, 49 insertions(+), 59 deletions(-) diff --git a/src/components/Menu/Menu.tsx b/src/components/Menu/Menu.tsx index f96c5a656e..6a6d5d33dd 100644 --- a/src/components/Menu/Menu.tsx +++ b/src/components/Menu/Menu.tsx @@ -1,6 +1,7 @@ import * as _ from 'lodash' import * as PropTypes from 'prop-types' import * as React from 'react' +import * as keyboardKey from 'keyboard-key' import { AutoControlledComponent, @@ -16,7 +17,7 @@ import { menuBehavior } from '../../lib/accessibility' import { Accessibility } from '../../lib/accessibility/types' import { ComponentVariablesObject } from '../../themes/types' -import { Extendable, ShorthandValue, ComponentEventHandler } from '../../../types/utils' +import { Extendable, ShorthandValue } from '../../../types/utils' export interface MenuProps extends UIComponentProps, ChildrenComponentProps { /** @@ -40,14 +41,6 @@ export interface MenuProps extends UIComponentProps, ChildrenComponentProps { /** Shorthand array of props for Menu. */ items?: ShorthandValue[] - /** - * Called on click. - * - * @param {SyntheticEvent} event - React's original SyntheticEvent. - * @param {object} data - All props. - */ - onClick?: ComponentEventHandler - /** A menu can adjust its appearance to de-emphasize its contents. */ pills?: boolean @@ -113,12 +106,23 @@ class Menu extends AutoControlledComponent, any> { handleItemOverrides = predefinedProps => ({ onClick: (e, itemProps) => { - const { index } = itemProps + const { index } = predefinedProps this.trySetState({ activeIndex: index }) _.invoke(predefinedProps, 'onClick', e, itemProps) }, + onKeyDown: (e, itemProps) => { + if ( + keyboardKey.getCode(e) === keyboardKey.Enter || + keyboardKey.getCode(e) === keyboardKey.Spacebar + ) { + // TODO check why itemProps is undefined (should be fixed by the changes added in getKeyDownHandler.ts + const { index } = predefinedProps + this.trySetState({ activeIndex: index }) + } + _.invoke(predefinedProps, 'onKeyDown', predefinedProps) + }, }) renderItems = (variables: ComponentVariablesObject) => { @@ -160,20 +164,11 @@ class Menu extends AutoControlledComponent, any> { renderComponent({ ElementType, classes, accessibility, variables, rest }) { const { children } = this.props return ( - + {childrenExist(children) ? children : this.renderItems(variables)} ) } - - private handleClick = (e: React.SyntheticEvent) => { - _.invoke(this.props, 'onClick', e, { ...this.props, ...this.state }) - } } Menu.create = createShorthandFactory(Menu, 'items') diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index a6f7f51552..7d80e02030 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -2,7 +2,6 @@ import * as _ from 'lodash' import * as cx from 'classnames' import * as PropTypes from 'prop-types' import * as React from 'react' -import * as keyboardKey from 'keyboard-key' import { AutoControlledComponent, @@ -16,7 +15,7 @@ import { EventStack, } from '../../lib' import Icon from '../Icon/Icon' -import Menu, { MenuProps } from '../Menu/Menu' +import Menu from '../Menu/Menu' import Ref from '../Ref/Ref' import Slot from '../Slot/Slot' import { menuItemBehavior, submenuBehavior } from '../../lib/accessibility' @@ -59,6 +58,14 @@ export interface MenuItemProps */ onClick?: ComponentEventHandler + /** + * Called on key down pressed. + * + * @param {SyntheticEvent} event - React's original SyntheticEvent. + * @param {object} data - All props. + */ + onKeyDown?: ComponentEventHandler + /** A menu can adjust its appearance to de-emphasize its contents. */ pills?: boolean @@ -169,12 +176,12 @@ class MenuItem extends AutoControlledComponent, MenuIt ) : ( {icon && @@ -184,7 +191,6 @@ class MenuItem extends AutoControlledComponent, MenuIt {content} ) - const maybeSubmenu = menu && active && submenuOpen ? Menu.create(menu, { @@ -195,10 +201,6 @@ class MenuItem extends AutoControlledComponent, MenuIt secondary, styles: styles.menu, }, - overrideProps: { - onClick: this.handleSubmenuClicked, - onKeyDown: this.handleSubmenuKeyDown, - }, }) : null @@ -221,6 +223,7 @@ class MenuItem extends AutoControlledComponent, MenuIt }, overrideProps: () => ({ children: [menuItemInner, maybeSubmenuWithRef], + onClick: this.handleClick, }), }) } @@ -228,7 +231,7 @@ class MenuItem extends AutoControlledComponent, MenuIt } protected actionHandlers: AccessibilityActionHandlers = { - performClick: event => this.handleClick(event), + performClick: event => this.performClick(event), openSubmenu: event => this.openSubmenu(event), closeMenu: event => this.closeMenu(event), closeSubmenu: event => this.closeSubmenu(event), @@ -252,12 +255,22 @@ class MenuItem extends AutoControlledComponent, MenuIt } } - private handleClick = e => { + private performClick = e => { const { active, menu } = this.props if (menu) { - this.trySetState({ submenuOpen: active ? !this.state.submenuOpen : true }) - e.stopPropagation() + if (this.submenuDomElement && this.submenuDomElement.contains(e.target)) { + // submenu was clicked, so we just close it and propagate + this.setState({ submenuOpen: false }, () => focusAsync(this.itemRef.current)) + } else { + // the menuItem element was clicked, so just toggle the open/close and stop propagation + this.trySetState({ submenuOpen: active ? !this.state.submenuOpen : true }) + e.stopPropagation() + } } + } + + private handleClick = e => { + this.performClick(e) _.invoke(this.props, 'onClick', e, this.props) } @@ -294,26 +307,11 @@ class MenuItem extends AutoControlledComponent, MenuIt const { menu } = this.props const { submenuOpen } = this.state if (menu && !submenuOpen) { - this.handleClick(e) + this.setState({ submenuOpen: true }) e.stopPropagation() e.preventDefault() } } - - private handleSubmenuClicked = (e: React.SyntheticEvent, props: MenuProps) => { - this.trySetState({ submenuOpen: false }) - _.invoke(props, 'onClick', props) - } - - private handleSubmenuKeyDown = (e: React.SyntheticEvent, props: MenuProps) => { - if ( - keyboardKey.getCode(e) === keyboardKey.Enter || - keyboardKey.getCode(e) === keyboardKey.Spacebar - ) { - this.trySetState({ submenuOpen: false }) - } - _.invoke(props, 'onKeyDown', props) - } } MenuItem.create = createShorthandFactory(MenuItem, 'content') diff --git a/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts b/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts index c3463ced39..30e04658e7 100644 --- a/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts +++ b/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts @@ -34,24 +34,22 @@ const menuItemBehavior: Accessibility = (props: any) => ({ handledProps: ['aria-label', 'aria-labelledby', 'aria-describedby'], keyActions: { - anchor: { + anchor: {}, + wrapper: { performClick: { keyCombinations: [{ keyCode: keyboardKey.Enter }, { keyCode: keyboardKey.Spacebar }], }, - - openSubmenu: { - keyCombinations: [ - { keyCode: props.vertical ? keyboardKey.ArrowRight : keyboardKey.ArrowDown }, - ], - }, - }, - wrapper: { closeMenu: { keyCombinations: [{ keyCode: keyboardKey.Escape }, { keyCode: keyboardKey.ArrowRight }], }, closeSubmenu: { keyCombinations: [{ keyCode: keyboardKey.ArrowLeft }], }, + openSubmenu: { + keyCombinations: [ + { keyCode: props.vertical ? keyboardKey.ArrowRight : keyboardKey.ArrowDown }, + ], + }, }, }, }) diff --git a/src/lib/getKeyDownHandlers.ts b/src/lib/getKeyDownHandlers.ts index a4dfe9f6e9..f4202ebdb7 100644 --- a/src/lib/getKeyDownHandlers.ts +++ b/src/lib/getKeyDownHandlers.ts @@ -41,8 +41,7 @@ const getKeyDownHandlers = ( eventHandler && eventHandler(event) }) - - _.invoke(props, 'onKeyDown', event) + _.invoke(props, 'onKeyDown', event, props) }, } } From d85b1645c46e6b50cdcd7ca65b6cbde74141e4da Mon Sep 17 00:00:00 2001 From: manajdov Date: Wed, 12 Dec 2018 16:27:49 +0100 Subject: [PATCH 16/55] -added setActiveIndex callback and removed onKeyDown in the creation of MenuItems in the Menu component --- src/components/Menu/Menu.tsx | 13 ++----------- src/components/Menu/MenuItem.tsx | 4 ++-- .../Behaviors/Menu/menuItemBehavior.ts | 1 - 3 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/components/Menu/Menu.tsx b/src/components/Menu/Menu.tsx index 6a6d5d33dd..f5f9eb863a 100644 --- a/src/components/Menu/Menu.tsx +++ b/src/components/Menu/Menu.tsx @@ -1,7 +1,6 @@ import * as _ from 'lodash' import * as PropTypes from 'prop-types' import * as React from 'react' -import * as keyboardKey from 'keyboard-key' import { AutoControlledComponent, @@ -112,16 +111,8 @@ class Menu extends AutoControlledComponent, any> { _.invoke(predefinedProps, 'onClick', e, itemProps) }, - onKeyDown: (e, itemProps) => { - if ( - keyboardKey.getCode(e) === keyboardKey.Enter || - keyboardKey.getCode(e) === keyboardKey.Spacebar - ) { - // TODO check why itemProps is undefined (should be fixed by the changes added in getKeyDownHandler.ts - const { index } = predefinedProps - this.trySetState({ activeIndex: index }) - } - _.invoke(predefinedProps, 'onKeyDown', predefinedProps) + setActiveIndex: index => { + this.trySetState({ activeIndex: index }) }, }) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 7d80e02030..b4094b656b 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -179,7 +179,6 @@ class MenuItem extends AutoControlledComponent, MenuIt onBlur={this.handleBlur} onFocus={this.handleFocus} {...accessibility.attributes.anchor} - {...accessibility.keyHandlers.anchor} {...rest} {...!wrapper && { onClick: this.handleClick }} ref={this.itemRef} @@ -231,7 +230,7 @@ class MenuItem extends AutoControlledComponent, MenuIt } protected actionHandlers: AccessibilityActionHandlers = { - performClick: event => this.performClick(event), + performClick: event => this.handleClick(event), openSubmenu: event => this.openSubmenu(event), closeMenu: event => this.closeMenu(event), closeSubmenu: event => this.closeSubmenu(event), @@ -308,6 +307,7 @@ class MenuItem extends AutoControlledComponent, MenuIt const { submenuOpen } = this.state if (menu && !submenuOpen) { this.setState({ submenuOpen: true }) + _.invoke(this.props, 'setActiveIndex', this.props.index) // or call onClick from the client... => Menu.onClick will change the active index e.stopPropagation() e.preventDefault() } diff --git a/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts b/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts index 30e04658e7..3c267d32af 100644 --- a/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts +++ b/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts @@ -34,7 +34,6 @@ const menuItemBehavior: Accessibility = (props: any) => ({ handledProps: ['aria-label', 'aria-labelledby', 'aria-describedby'], keyActions: { - anchor: {}, wrapper: { performClick: { keyCombinations: [{ keyCode: keyboardKey.Enter }, { keyCode: keyboardKey.Spacebar }], From e091af20184a8623ce2f60ad3d614aa3b957dd87 Mon Sep 17 00:00:00 2001 From: manajdov Date: Wed, 12 Dec 2018 20:31:01 +0100 Subject: [PATCH 17/55] -right arrow key is closing the submenus and goes to the next element if the menu is horizontal, or is focusing the first MenuItem if it is vertical --- src/components/Menu/Menu.tsx | 5 +++++ src/components/Menu/MenuItem.tsx | 13 +++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/components/Menu/Menu.tsx b/src/components/Menu/Menu.tsx index f5f9eb863a..8811d14f69 100644 --- a/src/components/Menu/Menu.tsx +++ b/src/components/Menu/Menu.tsx @@ -60,6 +60,8 @@ export interface MenuProps extends UIComponentProps, ChildrenComponentProps { /** A vertical menu displays elements vertically. */ vertical?: boolean + + parentRef: React.RefObject } /** @@ -88,6 +90,7 @@ class Menu extends AutoControlledComponent, any> { secondary: customPropTypes.every([customPropTypes.disallow(['primary']), PropTypes.bool]), underlined: PropTypes.bool, vertical: PropTypes.bool, + parentRef: PropTypes.any, } static defaultProps = { @@ -126,6 +129,7 @@ class Menu extends AutoControlledComponent, any> { secondary, underlined, vertical, + parentRef, } = this.props const { activeIndex } = this.state @@ -146,6 +150,7 @@ class Menu extends AutoControlledComponent, any> { ...(active && { styles: { position: 'relative' }, }), + parentRef, }, overrideProps: this.handleItemOverrides, }) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index b4094b656b..fc93ba467f 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -98,6 +98,8 @@ export interface MenuItemProps /** Default submenu open */ defaultSubmenuOpen?: boolean + + parentRef: React.RefObject } export interface MenuItemState { @@ -134,6 +136,7 @@ class MenuItem extends AutoControlledComponent, MenuIt menu: customPropTypes.itemShorthand, submenuOpen: PropTypes.bool, defaultSubmenuOpen: PropTypes.bool, + parentRef: PropTypes.any, } static defaultProps = { @@ -199,6 +202,7 @@ class MenuItem extends AutoControlledComponent, MenuIt primary, secondary, styles: styles.menu, + parentRef: this.itemRef, }, }) : null @@ -286,10 +290,15 @@ class MenuItem extends AutoControlledComponent, MenuIt } private closeMenu = e => { - const { menu } = this.props + const { menu, parentRef } = this.props const { submenuOpen } = this.state if (menu && submenuOpen) { - this.setState({ submenuOpen: false }, () => focusAsync(this.itemRef.current)) + this.setState({ submenuOpen: false }, () => { + // I this is the first MenuItem and it is vertical + if (!parentRef && this.props.vertical) { + focusAsync(this.itemRef.current) + } + }) } } From 279c7fa5dbc6d7e10be459372875d29afdac5f73 Mon Sep 17 00:00:00 2001 From: manajdov Date: Wed, 12 Dec 2018 20:43:49 +0100 Subject: [PATCH 18/55] -handled left arrow key --- src/components/Menu/Menu.tsx | 2 +- src/components/Menu/MenuItem.tsx | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/components/Menu/Menu.tsx b/src/components/Menu/Menu.tsx index 8811d14f69..7965cda7fe 100644 --- a/src/components/Menu/Menu.tsx +++ b/src/components/Menu/Menu.tsx @@ -61,7 +61,7 @@ export interface MenuProps extends UIComponentProps, ChildrenComponentProps { /** A vertical menu displays elements vertically. */ vertical?: boolean - parentRef: React.RefObject + parentRef?: React.RefObject } /** diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index fc93ba467f..b5624a8c4e 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -99,7 +99,7 @@ export interface MenuItemProps /** Default submenu open */ defaultSubmenuOpen?: boolean - parentRef: React.RefObject + parentRef?: React.RefObject } export interface MenuItemState { @@ -303,11 +303,18 @@ class MenuItem extends AutoControlledComponent, MenuIt } private closeSubmenu = e => { - const { menu } = this.props + const { menu, parentRef } = this.props const { submenuOpen } = this.state + const shouldStopPropagation = parentRef || this.props.vertical if (menu && submenuOpen) { - this.setState({ submenuOpen: false }, () => focusAsync(this.itemRef.current)) - e.stopPropagation() + this.setState({ submenuOpen: false }, () => { + if (shouldStopPropagation) { + focusAsync(this.itemRef.current) + } + }) + if (shouldStopPropagation) { + e.stopPropagation() + } } } From 8cc93911936aa10931fa1d06d9ddd56d8723ce6f Mon Sep 17 00:00:00 2001 From: manajdov Date: Thu, 13 Dec 2018 13:18:30 +0100 Subject: [PATCH 19/55] -changed ref -focus trap wip --- src/components/Menu/MenuItem.tsx | 49 +++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index b5624a8c4e..a9b27cc278 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -16,7 +16,6 @@ import { } from '../../lib' import Icon from '../Icon/Icon' import Menu from '../Menu/Menu' -import Ref from '../Ref/Ref' import Slot from '../Slot/Slot' import { menuItemBehavior, submenuBehavior } from '../../lib/accessibility' import { Accessibility, AccessibilityActionHandlers } from '../../lib/accessibility/types' @@ -99,6 +98,7 @@ export interface MenuItemProps /** Default submenu open */ defaultSubmenuOpen?: boolean + setActiveIndex?: (idx: string | number) => void parentRef?: React.RefObject } @@ -136,6 +136,7 @@ class MenuItem extends AutoControlledComponent, MenuIt menu: customPropTypes.itemShorthand, submenuOpen: PropTypes.bool, defaultSubmenuOpen: PropTypes.bool, + setActiveIndex: PropTypes.func, parentRef: PropTypes.any, } @@ -153,20 +154,24 @@ class MenuItem extends AutoControlledComponent, MenuIt } private outsideClickSubscription = EventStack.noSubscription + private outsideFocusSubscription = EventStack.noSubscription private submenuDomElement = null private itemRef = React.createRef() public componentDidMount() { this.updateOutsideClickSubscription() + this.updateOutsideFocusSubscription() } public componentDidUpdate() { this.updateOutsideClickSubscription() + this.updateOutsideFocusSubscription() } public componentWillUnmount() { this.outsideClickSubscription.unsubscribe() + this.outsideFocusSubscription.unsubscribe() } renderComponent({ ElementType, classes, accessibility, rest, styles }) { @@ -208,13 +213,13 @@ class MenuItem extends AutoControlledComponent, MenuIt : null const maybeSubmenuWithRef = maybeSubmenu ? ( - { +
{ this.submenuDomElement = domElement }} > {maybeSubmenu} - +
) : null if (wrapper) { @@ -245,19 +250,37 @@ class MenuItem extends AutoControlledComponent, MenuIt if (this.props.menu && this.state.submenuOpen) { setTimeout(() => { - this.outsideClickSubscription = EventStack.subscribe('click', e => { - if ( - this.itemRef && - (!this.itemRef.current || !this.itemRef.current.contains(e.target)) && - (!this.submenuDomElement || !this.submenuDomElement.contains(e.target)) - ) { - this.state.submenuOpen && this.trySetState({ submenuOpen: false }) - } - }) + this.outsideClickSubscription = EventStack.subscribe( + 'click', + this.outsideClickOrFocusHandler, + ) }) } } + private updateOutsideFocusSubscription() { + this.outsideFocusSubscription.unsubscribe() + + if (this.props.menu && this.state.submenuOpen) { + setTimeout(() => { + this.outsideFocusSubscription = EventStack.subscribe( + 'focus', + this.outsideClickOrFocusHandler, + ) + }) + } + } + + private outsideClickOrFocusHandler = e => { + if ( + this.itemRef && + (!this.itemRef.current || !this.itemRef.current.contains(e.target)) && + (!this.submenuDomElement || !this.submenuDomElement.contains(e.target)) + ) { + this.state.submenuOpen && this.trySetState({ submenuOpen: false }) + } + } + private performClick = e => { const { active, menu } = this.props if (menu) { From 920f2533f706fd07dc619069aafeaf8db09bb664 Mon Sep 17 00:00:00 2001 From: manajdov Date: Thu, 13 Dec 2018 18:29:55 +0100 Subject: [PATCH 20/55] -added Ref component instead of using the itemRef on the ElementType -removed subscription for focus --- src/components/Menu/MenuItem.tsx | 45 +++++++++++--------------------- 1 file changed, 15 insertions(+), 30 deletions(-) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index a9b27cc278..9f1588f6e8 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -22,6 +22,7 @@ import { Accessibility, AccessibilityActionHandlers } from '../../lib/accessibil import IsFromKeyboard from '../../lib/isFromKeyboard' import { ComponentEventHandler, Extendable, ShorthandValue } from '../../../types/utils' import { focusAsync } from '../../lib/accessibility/FocusZone' +import Ref from '../Ref/Ref' export interface MenuItemProps extends UIComponentProps, @@ -154,24 +155,20 @@ class MenuItem extends AutoControlledComponent, MenuIt } private outsideClickSubscription = EventStack.noSubscription - private outsideFocusSubscription = EventStack.noSubscription private submenuDomElement = null private itemRef = React.createRef() public componentDidMount() { this.updateOutsideClickSubscription() - this.updateOutsideFocusSubscription() } public componentDidUpdate() { this.updateOutsideClickSubscription() - this.updateOutsideFocusSubscription() } public componentWillUnmount() { this.outsideClickSubscription.unsubscribe() - this.outsideFocusSubscription.unsubscribe() } renderComponent({ ElementType, classes, accessibility, rest, styles }) { @@ -191,11 +188,15 @@ class MenuItem extends AutoControlledComponent, MenuIt {...!wrapper && { onClick: this.handleClick }} ref={this.itemRef} > - {icon && - Icon.create(this.props.icon, { - defaultProps: { xSpacing: !!content ? 'after' : 'none' }, - })} - {content} + + + {icon && + Icon.create(this.props.icon, { + defaultProps: { xSpacing: !!content ? 'after' : 'none' }, + })} + {content} + + ) const maybeSubmenu = @@ -213,13 +214,13 @@ class MenuItem extends AutoControlledComponent, MenuIt : null const maybeSubmenuWithRef = maybeSubmenu ? ( -
{ + { this.submenuDomElement = domElement }} > {maybeSubmenu} -
+ ) : null if (wrapper) { @@ -250,28 +251,12 @@ class MenuItem extends AutoControlledComponent, MenuIt if (this.props.menu && this.state.submenuOpen) { setTimeout(() => { - this.outsideClickSubscription = EventStack.subscribe( - 'click', - this.outsideClickOrFocusHandler, - ) + this.outsideClickSubscription = EventStack.subscribe('click', this.outsideClickHandler) }) } } - private updateOutsideFocusSubscription() { - this.outsideFocusSubscription.unsubscribe() - - if (this.props.menu && this.state.submenuOpen) { - setTimeout(() => { - this.outsideFocusSubscription = EventStack.subscribe( - 'focus', - this.outsideClickOrFocusHandler, - ) - }) - } - } - - private outsideClickOrFocusHandler = e => { + private outsideClickHandler = e => { if ( this.itemRef && (!this.itemRef.current || !this.itemRef.current.contains(e.target)) && From 9e3763ebcf21aed5cbf8ae5a8f0e98289fd46ab9 Mon Sep 17 00:00:00 2001 From: manajdov Date: Thu, 13 Dec 2018 18:53:02 +0100 Subject: [PATCH 21/55] -fixes --- src/components/Menu/MenuItem.tsx | 7 ++++--- src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts | 2 +- test/specs/components/Menu/MenuItem-test.tsx | 3 --- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 342bedc279..d280d14590 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -173,7 +173,7 @@ class MenuItem extends AutoControlledComponent, MenuIt } renderComponent({ ElementType, classes, accessibility, rest, styles }) { - const { children, content, icon, wrapper, menu, primary, secondary, active } = this.props + const { children, content, icon, wrapper, menu, primary, secondary, active, key } = this.props const { submenuOpen } = this.state @@ -226,9 +226,10 @@ class MenuItem extends AutoControlledComponent, MenuIt if (wrapper) { return Slot.create(wrapper, { defaultProps: { + key, className: cx('ui-menu__item__wrapper', classes.wrapper), - ...accessibility.attributes.wrapper, - ...accessibility.keyHandlers.wrapper, + ...accessibility.attributes.root, + ...accessibility.keyHandlers.root, }, overrideProps: () => ({ children: [menuItemInner, maybeSubmenuWithRef], diff --git a/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts b/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts index 3c267d32af..4b3e0e4efd 100644 --- a/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts +++ b/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts @@ -34,7 +34,7 @@ const menuItemBehavior: Accessibility = (props: any) => ({ handledProps: ['aria-label', 'aria-labelledby', 'aria-describedby'], keyActions: { - wrapper: { + root: { performClick: { keyCombinations: [{ keyCode: keyboardKey.Enter }, { keyCode: keyboardKey.Spacebar }], }, diff --git a/test/specs/components/Menu/MenuItem-test.tsx b/test/specs/components/Menu/MenuItem-test.tsx index d85082d9e4..820f658bc6 100644 --- a/test/specs/components/Menu/MenuItem-test.tsx +++ b/test/specs/components/Menu/MenuItem-test.tsx @@ -7,9 +7,6 @@ import { toolbarButtonBehavior, tabBehavior } from '../../../../src/lib/accessib describe('MenuItem', () => { isConformant(MenuItem, { - eventTargets: { - onClick: 'a', - }, usesWrapperSlot: true, }) From afbdbfaf428c8efe24fa488e85d7422d8d57b5d0 Mon Sep 17 00:00:00 2001 From: manajdov Date: Thu, 13 Dec 2018 19:21:12 +0100 Subject: [PATCH 22/55] -moved ref //TODO: figure out tests failing --- src/components/Menu/MenuItem.tsx | 34 +++++++++++++++----------------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index d280d14590..efea58531b 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -180,24 +180,22 @@ class MenuItem extends AutoControlledComponent, MenuIt const menuItemInner = childrenExist(children) ? ( children ) : ( - - - - {icon && - Icon.create(this.props.icon, { - defaultProps: { xSpacing: !!content ? 'after' : 'none' }, - })} - {content} - - - + + + {icon && + Icon.create(this.props.icon, { + defaultProps: { xSpacing: !!content ? 'after' : 'none' }, + })} + {content} + + ) const maybeSubmenu = menu && active && submenuOpen From d3ab05528861ebce2bb66fbedc484cff30ee1417 Mon Sep 17 00:00:00 2001 From: manajdov Date: Fri, 14 Dec 2018 11:26:55 +0100 Subject: [PATCH 23/55] -close menu on outside focus --- src/components/Menu/MenuItem.tsx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index efea58531b..90687fbeed 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -232,12 +232,19 @@ class MenuItem extends AutoControlledComponent, MenuIt overrideProps: () => ({ children: [menuItemInner, maybeSubmenuWithRef], onClick: this.handleClick, + onBlur: this.handleWrapperBlur, }), }) } return menuItemInner } + private handleWrapperBlur = e => { + if (!this.props.parentRef && !e.currentTarget.contains(e.relatedTarget)) { + this.setState({ submenuOpen: false }) + } + } + protected actionHandlers: AccessibilityActionHandlers = { performClick: event => this.handleClick(event), openSubmenu: event => this.openSubmenu(event), From 5653db39f1967a19b18083576fa1af31483cfce1 Mon Sep 17 00:00:00 2001 From: manajdov Date: Fri, 14 Dec 2018 11:29:44 +0100 Subject: [PATCH 24/55] -improved comments --- src/components/Menu/MenuItem.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 90687fbeed..6ab1a005b1 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -276,10 +276,10 @@ class MenuItem extends AutoControlledComponent, MenuIt const { active, menu } = this.props if (menu) { if (this.submenuDomElement && this.submenuDomElement.contains(e.target)) { - // submenu was clicked, so we just close it and propagate + // submenu was clicked => close it and propagate this.setState({ submenuOpen: false }, () => focusAsync(this.itemRef.current)) } else { - // the menuItem element was clicked, so just toggle the open/close and stop propagation + // the menuItem element was clicked => toggle the open/close and stop propagation this.trySetState({ submenuOpen: active ? !this.state.submenuOpen : true }) e.stopPropagation() } @@ -308,7 +308,7 @@ class MenuItem extends AutoControlledComponent, MenuIt const { submenuOpen } = this.state if (menu && submenuOpen) { this.setState({ submenuOpen: false }, () => { - // I this is the first MenuItem and it is vertical + // this is the first MenuItem and it is vertical if (!parentRef && this.props.vertical) { focusAsync(this.itemRef.current) } @@ -337,7 +337,7 @@ class MenuItem extends AutoControlledComponent, MenuIt const { submenuOpen } = this.state if (menu && !submenuOpen) { this.setState({ submenuOpen: true }) - _.invoke(this.props, 'setActiveIndex', this.props.index) // or call onClick from the client... => Menu.onClick will change the active index + _.invoke(this.props, 'setActiveIndex', this.props.index) e.stopPropagation() e.preventDefault() } From 4bb8ee8023a28b0910bbb60beabf2c8fbd9eca53 Mon Sep 17 00:00:00 2001 From: manajdov Date: Fri, 14 Dec 2018 13:04:21 +0100 Subject: [PATCH 25/55] -fixed escape key not focusing the active element -changed parentRef to inSubmenu boolean --- src/components/Menu/Menu.tsx | 9 +++---- src/components/Menu/MenuItem.tsx | 40 ++++++++++++++++++-------------- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/components/Menu/Menu.tsx b/src/components/Menu/Menu.tsx index be9ac3f99b..87673a0adc 100644 --- a/src/components/Menu/Menu.tsx +++ b/src/components/Menu/Menu.tsx @@ -62,7 +62,8 @@ export interface MenuProps extends UIComponentProps, ChildrenComponentProps { /** A vertical menu displays elements vertically. */ vertical?: boolean - parentRef?: React.RefObject + /** Indicates whether the element is part of submenu. */ + inSubmenu?: boolean } /** @@ -91,7 +92,7 @@ class Menu extends AutoControlledComponent, any> { secondary: customPropTypes.every([customPropTypes.disallow(['primary']), PropTypes.bool]), underlined: PropTypes.bool, vertical: PropTypes.bool, - parentRef: PropTypes.any, + inSubmenu: PropTypes.bool, } static defaultProps = { @@ -130,7 +131,7 @@ class Menu extends AutoControlledComponent, any> { secondary, underlined, vertical, - parentRef, + inSubmenu, } = this.props const { activeIndex } = this.state @@ -151,7 +152,7 @@ class Menu extends AutoControlledComponent, any> { ...(active && { styles: { position: 'relative' }, }), - parentRef, + inSubmenu, }, overrideProps: this.handleItemOverrides, }) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 6ab1a005b1..131c20e4c1 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -2,6 +2,7 @@ import * as _ from 'lodash' import cx from 'classnames' import * as PropTypes from 'prop-types' import * as React from 'react' +import * as keyboardKey from 'keyboard-key' import { AutoControlledComponent, @@ -100,8 +101,11 @@ export interface MenuItemProps /** Default submenu open */ defaultSubmenuOpen?: boolean - setActiveIndex?: (idx: string | number) => void - parentRef?: React.RefObject + /** Callback for setting the current menu item as active element in the menu. */ + setActiveIndex?: (idx: number) => void + + /** Indicates whether the menu item is part of submenu. */ + inSubmenu?: boolean } export interface MenuItemState { @@ -139,7 +143,7 @@ class MenuItem extends AutoControlledComponent, MenuIt submenuOpen: PropTypes.bool, defaultSubmenuOpen: PropTypes.bool, setActiveIndex: PropTypes.func, - parentRef: PropTypes.any, + inSubmenu: PropTypes.bool, } static defaultProps = { @@ -173,7 +177,7 @@ class MenuItem extends AutoControlledComponent, MenuIt } renderComponent({ ElementType, classes, accessibility, rest, styles }) { - const { children, content, icon, wrapper, menu, primary, secondary, active, key } = this.props + const { children, content, icon, wrapper, menu, primary, secondary, active } = this.props const { submenuOpen } = this.state @@ -206,7 +210,7 @@ class MenuItem extends AutoControlledComponent, MenuIt primary, secondary, styles: styles.menu, - parentRef: this.itemRef, + inSubmenu: true, }, }) : null @@ -224,13 +228,17 @@ class MenuItem extends AutoControlledComponent, MenuIt if (wrapper) { return Slot.create(wrapper, { defaultProps: { - key, className: cx('ui-menu__item__wrapper', classes.wrapper), ...accessibility.attributes.root, ...accessibility.keyHandlers.root, }, overrideProps: () => ({ - children: [menuItemInner, maybeSubmenuWithRef], + children: ( + <> + {menuItemInner} + {maybeSubmenuWithRef} + + ), onClick: this.handleClick, onBlur: this.handleWrapperBlur, }), @@ -240,7 +248,7 @@ class MenuItem extends AutoControlledComponent, MenuIt } private handleWrapperBlur = e => { - if (!this.props.parentRef && !e.currentTarget.contains(e.relatedTarget)) { + if (!this.props.inSubmenu && !e.currentTarget.contains(e.relatedTarget)) { this.setState({ submenuOpen: false }) } } @@ -304,22 +312,20 @@ class MenuItem extends AutoControlledComponent, MenuIt } private closeMenu = e => { - const { menu, parentRef } = this.props + const { menu, inSubmenu } = this.props const { submenuOpen } = this.state if (menu && submenuOpen) { - this.setState({ submenuOpen: false }, () => { - // this is the first MenuItem and it is vertical - if (!parentRef && this.props.vertical) { - focusAsync(this.itemRef.current) - } - }) + this.setState({ submenuOpen: false }) + if (!inSubmenu && (keyboardKey.getCode(e) === keyboardKey.Escape || this.props.vertical)) { + focusAsync(this.itemRef.current) + } } } private closeSubmenu = e => { - const { menu, parentRef } = this.props + const { menu, inSubmenu } = this.props const { submenuOpen } = this.state - const shouldStopPropagation = parentRef || this.props.vertical + const shouldStopPropagation = inSubmenu || this.props.vertical if (menu && submenuOpen) { this.setState({ submenuOpen: false }, () => { if (shouldStopPropagation) { From fac416123915f83afb4f5ce52d77450c73f66a5c Mon Sep 17 00:00:00 2001 From: manajdov Date: Fri, 14 Dec 2018 14:05:08 +0100 Subject: [PATCH 26/55] -fixing key problems -added dependency for generating id --- package.json | 3 ++- src/components/Menu/MenuItem.tsx | 3 ++- yarn.lock | 5 +++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 75c2f3976d..b31e653b04 100644 --- a/package.json +++ b/package.json @@ -70,7 +70,8 @@ "prop-types": "^15.6.1", "react-fela": "^7.2.0", "react-is": "^16.6.3", - "react-popper": "^1.0.2" + "react-popper": "^1.0.2", + "uniqid": "^5.0.3" }, "devDependencies": { "@babel/standalone": "^7.1.0", diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 131c20e4c1..fb43331cda 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -3,6 +3,7 @@ import cx from 'classnames' import * as PropTypes from 'prop-types' import * as React from 'react' import * as keyboardKey from 'keyboard-key' +import * as uniqid from 'uniqid' import { AutoControlledComponent, @@ -149,7 +150,7 @@ class MenuItem extends AutoControlledComponent, MenuIt static defaultProps = { as: 'a', accessibility: menuItemBehavior as Accessibility, - wrapper: { as: 'li' }, + wrapper: { as: 'li', key: uniqid() }, } static autoControlledProps = ['submenuOpen'] diff --git a/yarn.lock b/yarn.lock index 25e665d8cb..43ec421ea6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10479,6 +10479,11 @@ union-value@^1.0.0: is-extendable "^0.1.1" set-value "^0.4.3" +uniqid@^5.0.3: + version "5.0.3" + resolved "https://registry.yarnpkg.com/uniqid/-/uniqid-5.0.3.tgz#917968310edc868d50df6c44f783f32c7d80fac1" + integrity sha512-R2qx3X/LYWSdGRaluio4dYrPXAJACTqyUjuyXHoJLBUOIfmMcnYOyY2d6Y4clZcIz5lK6ZaI0Zzmm0cPfsIqzQ== + unique-filename@^1.1.0: version "1.1.0" resolved "https://registry.yarnpkg.com/unique-filename/-/unique-filename-1.1.0.tgz#d05f2fe4032560871f30e93cbe735eea201514f3" From 0e1941da84b769d932d7266dfafc46fbe6140fc8 Mon Sep 17 00:00:00 2001 From: manajdov Date: Fri, 14 Dec 2018 16:16:54 +0100 Subject: [PATCH 27/55] -refactored submenuRef element --- src/components/Menu/MenuItem.tsx | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index fb43331cda..674a8bada4 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -203,8 +203,13 @@ class MenuItem extends AutoControlledComponent, MenuIt ) const maybeSubmenu = - menu && active && submenuOpen - ? Menu.create(menu, { + menu && active && submenuOpen ? ( + { + this.submenuDomElement = domElement + }} + > + {Menu.create(menu, { defaultProps: { accessibility: submenuBehavior, vertical: true, @@ -213,18 +218,9 @@ class MenuItem extends AutoControlledComponent, MenuIt styles: styles.menu, inSubmenu: true, }, - }) - : null - - const maybeSubmenuWithRef = maybeSubmenu ? ( - { - this.submenuDomElement = domElement - }} - > - {maybeSubmenu} - - ) : null + })} + + ) : null if (wrapper) { return Slot.create(wrapper, { @@ -237,7 +233,7 @@ class MenuItem extends AutoControlledComponent, MenuIt children: ( <> {menuItemInner} - {maybeSubmenuWithRef} + {maybeSubmenu} ), onClick: this.handleClick, From 168b16ab0dc83e83eac66a6f8013ce1b7235b469 Mon Sep 17 00:00:00 2001 From: olfedias Date: Fri, 14 Dec 2018 18:34:38 +0200 Subject: [PATCH 28/55] fix broken tests --- package.json | 3 +-- src/components/Menu/MenuItem.tsx | 3 +-- test/specs/commonTests/isConformant.tsx | 21 ++++++++++++-------- test/specs/components/Menu/MenuItem-test.tsx | 12 ++++++++++- yarn.lock | 5 ----- 5 files changed, 26 insertions(+), 18 deletions(-) diff --git a/package.json b/package.json index b31e653b04..75c2f3976d 100644 --- a/package.json +++ b/package.json @@ -70,8 +70,7 @@ "prop-types": "^15.6.1", "react-fela": "^7.2.0", "react-is": "^16.6.3", - "react-popper": "^1.0.2", - "uniqid": "^5.0.3" + "react-popper": "^1.0.2" }, "devDependencies": { "@babel/standalone": "^7.1.0", diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 674a8bada4..9c02cc6e87 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -3,7 +3,6 @@ import cx from 'classnames' import * as PropTypes from 'prop-types' import * as React from 'react' import * as keyboardKey from 'keyboard-key' -import * as uniqid from 'uniqid' import { AutoControlledComponent, @@ -150,7 +149,7 @@ class MenuItem extends AutoControlledComponent, MenuIt static defaultProps = { as: 'a', accessibility: menuItemBehavior as Accessibility, - wrapper: { as: 'li', key: uniqid() }, + wrapper: { as: 'li' }, } static autoControlledProps = ['submenuOpen'] diff --git a/test/specs/commonTests/isConformant.tsx b/test/specs/commonTests/isConformant.tsx index 5836b629b2..055701bcab 100644 --- a/test/specs/commonTests/isConformant.tsx +++ b/test/specs/commonTests/isConformant.tsx @@ -19,6 +19,7 @@ import { FOCUSZONE_WRAP_ATTRIBUTE } from 'src/lib/accessibility/FocusZone/focusU export interface Conformant { eventTargets?: object + nestingLevel?: number requiredProps?: object exportedAtTopLevel?: boolean rendersPortal?: boolean @@ -39,6 +40,7 @@ export default (Component, options: Conformant = {}) => { const { eventTargets = {}, exportedAtTopLevel = true, + nestingLevel = 0, requiredProps = {}, rendersPortal = false, usesWrapperSlot = false, @@ -51,9 +53,12 @@ export default (Component, options: Conformant = {}) => { const getComponent = (wrapper: ReactWrapper) => { // FelaTheme wrapper and the component itself: let component = wrapper - .childAt(0) - .childAt(0) - .childAt(0) + + // TODO: Describe magic numbers + _.times(nestingLevel + 3, () => { + component = component.childAt(0) + }) + if (component.type() === FocusZone) { // `component` is component = component.childAt(0) // skip through @@ -63,10 +68,9 @@ export default (Component, options: Conformant = {}) => { } if (usesWrapperSlot) { - component = component - .childAt(0) - .childAt(0) - .childAt(0) + _.times(3, () => { + component = component.childAt(0) + }) } return component @@ -212,7 +216,8 @@ export default (Component, options: Conformant = {}) => { const wrapper = mount() const component = getComponent(wrapper) - + // console.log(wrapper.debug()) + // console.log(component.type()) try { expect(component.type()).toEqual(MyComponent) } catch (err) { diff --git a/test/specs/components/Menu/MenuItem-test.tsx b/test/specs/components/Menu/MenuItem-test.tsx index 820f658bc6..07c1c86f73 100644 --- a/test/specs/components/Menu/MenuItem-test.tsx +++ b/test/specs/components/Menu/MenuItem-test.tsx @@ -7,6 +7,10 @@ import { toolbarButtonBehavior, tabBehavior } from '../../../../src/lib/accessib describe('MenuItem', () => { isConformant(MenuItem, { + eventTargets: { + onClick: '.ui-menu__item__wrapper', + }, + nestingLevel: 2, usesWrapperSlot: true, }) @@ -16,7 +20,13 @@ describe('MenuItem', () => { .hostNodes() expect(menuItem.is('li')).toBe(true) - expect(menuItem.childAt(0).is('a')).toBe(true) + expect( + menuItem + .childAt(0) + .childAt(0) + .childAt(0) + .is('a'), + ).toBe(true) expect(menuItem.text()).toBe('Home') }) diff --git a/yarn.lock b/yarn.lock index 43ec421ea6..25e665d8cb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10479,11 +10479,6 @@ union-value@^1.0.0: is-extendable "^0.1.1" set-value "^0.4.3" -uniqid@^5.0.3: - version "5.0.3" - resolved "https://registry.yarnpkg.com/uniqid/-/uniqid-5.0.3.tgz#917968310edc868d50df6c44f783f32c7d80fac1" - integrity sha512-R2qx3X/LYWSdGRaluio4dYrPXAJACTqyUjuyXHoJLBUOIfmMcnYOyY2d6Y4clZcIz5lK6ZaI0Zzmm0cPfsIqzQ== - unique-filename@^1.1.0: version "1.1.0" resolved "https://registry.yarnpkg.com/unique-filename/-/unique-filename-1.1.0.tgz#d05f2fe4032560871f30e93cbe735eea201514f3" From 2baa1cb1bdf114378268cc83f241a1dfad81aa25 Mon Sep 17 00:00:00 2001 From: manajdov Date: Fri, 14 Dec 2018 18:20:38 +0100 Subject: [PATCH 29/55] -added comments in the tests -changed the submenuDomElement so submenuRef --- src/components/Menu/MenuItem.tsx | 12 ++++-------- test/specs/commonTests/isConformant.tsx | 16 ++++++++++++---- test/specs/components/Menu/MenuItem-test.tsx | 2 ++ 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 9c02cc6e87..9f1f36f9f6 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -161,7 +161,7 @@ class MenuItem extends AutoControlledComponent, MenuIt private outsideClickSubscription = EventStack.noSubscription - private submenuDomElement = null + private submenuRef = React.createRef() private itemRef = React.createRef() public componentDidMount() { @@ -203,11 +203,7 @@ class MenuItem extends AutoControlledComponent, MenuIt ) const maybeSubmenu = menu && active && submenuOpen ? ( - { - this.submenuDomElement = domElement - }} - > + {Menu.create(menu, { defaultProps: { accessibility: submenuBehavior, @@ -270,7 +266,7 @@ class MenuItem extends AutoControlledComponent, MenuIt if ( this.itemRef && (!this.itemRef.current || !this.itemRef.current.contains(e.target)) && - (!this.submenuDomElement || !this.submenuDomElement.contains(e.target)) + (!this.submenuRef.current || !this.submenuRef.current.contains(e.target)) ) { this.state.submenuOpen && this.trySetState({ submenuOpen: false }) } @@ -279,7 +275,7 @@ class MenuItem extends AutoControlledComponent, MenuIt private performClick = e => { const { active, menu } = this.props if (menu) { - if (this.submenuDomElement && this.submenuDomElement.contains(e.target)) { + if (this.submenuRef.current && this.submenuRef.current.contains(e.target)) { // submenu was clicked => close it and propagate this.setState({ submenuOpen: false }, () => focusAsync(this.itemRef.current)) } else { diff --git a/test/specs/commonTests/isConformant.tsx b/test/specs/commonTests/isConformant.tsx index 055701bcab..a024a4391e 100644 --- a/test/specs/commonTests/isConformant.tsx +++ b/test/specs/commonTests/isConformant.tsx @@ -54,13 +54,17 @@ export default (Component, options: Conformant = {}) => { // FelaTheme wrapper and the component itself: let component = wrapper - // TODO: Describe magic numbers + /** + * The wrapper is mounted with Provider, so in total there are three HOC components + * that we want to get rid of: ThemeProvider, the actual Component and FelaTheme, + * in order to be able to get to the actual rendered result of the component we are testing + */ _.times(nestingLevel + 3, () => { component = component.childAt(0) }) if (component.type() === FocusZone) { - // `component` is + // another HOC component is added: FocuZone component = component.childAt(0) // skip through if (component.prop(FOCUSZONE_WRAP_ATTRIBUTE)) { component = component.childAt(0) // skip the additional wrap
of the FocusZone @@ -68,6 +72,11 @@ export default (Component, options: Conformant = {}) => { } if (usesWrapperSlot) { + /** + * If there is a wrapper slot, then again, we need to get rid of all three HOC components: + * ThemeProvider, Wrapper (Slot), and FelaTheme in order to be able to get to the actual + * rendered result of the component we are testing + */ _.times(3, () => { component = component.childAt(0) }) @@ -216,8 +225,7 @@ export default (Component, options: Conformant = {}) => { const wrapper = mount() const component = getComponent(wrapper) - // console.log(wrapper.debug()) - // console.log(component.type()) + try { expect(component.type()).toEqual(MyComponent) } catch (err) { diff --git a/test/specs/components/Menu/MenuItem-test.tsx b/test/specs/components/Menu/MenuItem-test.tsx index 07c1c86f73..c24ea2547c 100644 --- a/test/specs/components/Menu/MenuItem-test.tsx +++ b/test/specs/components/Menu/MenuItem-test.tsx @@ -10,6 +10,7 @@ describe('MenuItem', () => { eventTargets: { onClick: '.ui-menu__item__wrapper', }, + // The ElementType is wrapped with Ref, which is adding two HOC in total nestingLevel: 2, usesWrapperSlot: true, }) @@ -20,6 +21,7 @@ describe('MenuItem', () => { .hostNodes() expect(menuItem.is('li')).toBe(true) + // The ElementType is wrapped with Ref, which is adding two HOC in total, that's why we need the three childAt(0) usages expect( menuItem .childAt(0) From f61533be26a5d5638705c2bb866b7db8129f922a Mon Sep 17 00:00:00 2001 From: manajdov Date: Sun, 16 Dec 2018 15:11:52 +0100 Subject: [PATCH 30/55] -renamed inSubmenu to submenu prop in the Menu --- src/components/Menu/Menu.tsx | 10 +++++----- src/components/Menu/MenuItem.tsx | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/Menu/Menu.tsx b/src/components/Menu/Menu.tsx index 87673a0adc..19131b0c6e 100644 --- a/src/components/Menu/Menu.tsx +++ b/src/components/Menu/Menu.tsx @@ -62,8 +62,8 @@ export interface MenuProps extends UIComponentProps, ChildrenComponentProps { /** A vertical menu displays elements vertically. */ vertical?: boolean - /** Indicates whether the element is part of submenu. */ - inSubmenu?: boolean + /** Indicates whether the menu is submenu. */ + submenu?: boolean } /** @@ -92,7 +92,7 @@ class Menu extends AutoControlledComponent, any> { secondary: customPropTypes.every([customPropTypes.disallow(['primary']), PropTypes.bool]), underlined: PropTypes.bool, vertical: PropTypes.bool, - inSubmenu: PropTypes.bool, + submenu: PropTypes.bool, } static defaultProps = { @@ -131,7 +131,7 @@ class Menu extends AutoControlledComponent, any> { secondary, underlined, vertical, - inSubmenu, + submenu, } = this.props const { activeIndex } = this.state @@ -152,7 +152,7 @@ class Menu extends AutoControlledComponent, any> { ...(active && { styles: { position: 'relative' }, }), - inSubmenu, + inSubmenu: submenu, }, overrideProps: this.handleItemOverrides, }) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 9f1f36f9f6..170dbe90a1 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -211,7 +211,7 @@ class MenuItem extends AutoControlledComponent, MenuIt primary, secondary, styles: styles.menu, - inSubmenu: true, + submenu: true, }, })} From 636e51daa38f8e14ae1806311159b0c5441e0f68 Mon Sep 17 00:00:00 2001 From: manajdov Date: Sun, 16 Dec 2018 15:29:58 +0100 Subject: [PATCH 31/55] -fixed with the auto-controlled prop in the Menu --- src/components/Menu/Menu.tsx | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/components/Menu/Menu.tsx b/src/components/Menu/Menu.tsx index 19131b0c6e..9baa53c8ae 100644 --- a/src/components/Menu/Menu.tsx +++ b/src/components/Menu/Menu.tsx @@ -104,13 +104,9 @@ class Menu extends AutoControlledComponent, any> { static Item = MenuItem - state = { - activeIndex: '', - } - handleItemOverrides = predefinedProps => ({ onClick: (e, itemProps) => { - const { index } = predefinedProps + const { index } = itemProps this.trySetState({ activeIndex: index }) @@ -149,9 +145,6 @@ class Menu extends AutoControlledComponent, any> { vertical, index, active, - ...(active && { - styles: { position: 'relative' }, - }), inSubmenu: submenu, }, overrideProps: this.handleItemOverrides, From b97bcce5621ab560be374debf74a7e83af377733 Mon Sep 17 00:00:00 2001 From: manajdov Date: Sun, 16 Dec 2018 15:53:08 +0100 Subject: [PATCH 32/55] -added state interface in the Menu -improved menu variables' names --- src/components/Menu/Menu.tsx | 9 ++++-- src/components/Menu/MenuItem.tsx | 2 +- .../teams/components/Menu/menuItemStyles.ts | 28 +++++++++---------- .../teams/components/Menu/menuStyles.ts | 3 +- .../teams/components/Menu/menuVariables.ts | 20 ++++++------- 5 files changed, 33 insertions(+), 29 deletions(-) diff --git a/src/components/Menu/Menu.tsx b/src/components/Menu/Menu.tsx index 9baa53c8ae..4321fa9e3c 100644 --- a/src/components/Menu/Menu.tsx +++ b/src/components/Menu/Menu.tsx @@ -66,10 +66,14 @@ export interface MenuProps extends UIComponentProps, ChildrenComponentProps { submenu?: boolean } +export interface MenuState { + activeIndex?: number | string +} + /** * A menu displays grouped navigation actions. */ -class Menu extends AutoControlledComponent, any> { +class Menu extends AutoControlledComponent, MenuState> { static displayName = 'Menu' static className = 'ui-menu' @@ -132,7 +136,8 @@ class Menu extends AutoControlledComponent, any> { const { activeIndex } = this.state return _.map(items, (item, index) => { - const active = parseInt(activeIndex, 10) === index + const active = + typeof activeIndex === 'string' ? parseInt(activeIndex, 10) : activeIndex === index return MenuItem.create(item, { defaultProps: { iconOnly, diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 170dbe90a1..0608c5fc8a 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -102,7 +102,7 @@ export interface MenuItemProps defaultSubmenuOpen?: boolean /** Callback for setting the current menu item as active element in the menu. */ - setActiveIndex?: (idx: number) => void + setActiveIndex?: (idx: number | string) => void /** Indicates whether the menu item is part of submenu. */ inSubmenu?: boolean diff --git a/src/themes/teams/components/Menu/menuItemStyles.ts b/src/themes/teams/components/Menu/menuItemStyles.ts index 85a514d0d1..d98017d23b 100644 --- a/src/themes/teams/components/Menu/menuItemStyles.ts +++ b/src/themes/teams/components/Menu/menuItemStyles.ts @@ -23,7 +23,7 @@ const getActionStyles = ({ (underlined && !isFromKeyboard) || iconOnly ? { color, - background: v.defaultBackgroundColor, + background: v.backgroundColor, } : primary ? { @@ -32,7 +32,7 @@ const getActionStyles = ({ } : { color, - background: v.defaultActiveBackgroundColor, + background: v.activeBackgroundColor, } const itemSeparator: ComponentSlotStyleFunction = ({ @@ -52,7 +52,7 @@ const itemSeparator: ComponentSlotStyleFunction { return { - defaultColor: siteVars.gray02, - defaultBackgroundColor: '#FFF', + color: siteVars.gray02, + backgroundColor: siteVars.white, - defaultActiveColor: siteVars.black, - defaultActiveBackgroundColor: siteVars.gray10, - defaultBorderColor: siteVars.gray08, + activeColor: siteVars.black, + activeBackgroundColor: siteVars.gray10, + borderColor: siteVars.gray08, primaryActiveColor: siteVars.white, primaryActiveBackgroundColor: siteVars.brand08, From eec33c1e882960b5398d36d6b7e68c00a8ec9c46 Mon Sep 17 00:00:00 2001 From: manajdov Date: Sun, 16 Dec 2018 16:07:17 +0100 Subject: [PATCH 33/55] -fixed variables in examples --- .../MenuExampleIconOnlyPrimaryInverted.shorthand.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/src/examples/components/Menu/Variations/MenuExampleIconOnlyPrimaryInverted.shorthand.tsx b/docs/src/examples/components/Menu/Variations/MenuExampleIconOnlyPrimaryInverted.shorthand.tsx index 733f7a7b52..726c14ac9b 100644 --- a/docs/src/examples/components/Menu/Variations/MenuExampleIconOnlyPrimaryInverted.shorthand.tsx +++ b/docs/src/examples/components/Menu/Variations/MenuExampleIconOnlyPrimaryInverted.shorthand.tsx @@ -14,9 +14,9 @@ const MenuExampleIconOnlyPrimaryInverted = () => ( items={items} primary variables={siteVars => ({ - defaultColor: siteVars.gray06, - defaultBackgroundColor: siteVars.brand, - typePrimaryActiveBorderColor: siteVars.white, + color: siteVars.gray06, + backgroundColor: siteVars.brand, + primaryActiveBorderColor: siteVars.white, })} /> ) From 950cd54475f3ae06b24bc8edb7707efe6110e4f3 Mon Sep 17 00:00:00 2001 From: manajdov Date: Mon, 17 Dec 2018 14:59:34 +0100 Subject: [PATCH 34/55] -remove state initialization in the MenuItem component --- src/components/Menu/MenuItem.tsx | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 0608c5fc8a..f9a2bc6441 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -154,11 +154,6 @@ class MenuItem extends AutoControlledComponent, MenuIt static autoControlledProps = ['submenuOpen'] - state = { - isFromKeyboard: false, - submenuOpen: false, - } - private outsideClickSubscription = EventStack.noSubscription private submenuRef = React.createRef() From 04a531366e6312947a15619ed249c7b5841b94fd Mon Sep 17 00:00:00 2001 From: manajdov Date: Mon, 17 Dec 2018 17:55:02 +0100 Subject: [PATCH 35/55] -added new handler for escape -changed submenu examples titles --- docs/src/examples/components/Menu/Types/index.tsx | 4 ++-- src/components/Menu/MenuItem.tsx | 14 +++++++++++++- .../Behaviors/Menu/menuItemBehavior.ts | 5 ++++- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/docs/src/examples/components/Menu/Types/index.tsx b/docs/src/examples/components/Menu/Types/index.tsx index a589b627a9..221c7fc22a 100644 --- a/docs/src/examples/components/Menu/Types/index.tsx +++ b/docs/src/examples/components/Menu/Types/index.tsx @@ -20,12 +20,12 @@ const Types = () => ( examplePath="components/Menu/Types/MenuExampleVertical" /> diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index f9a2bc6441..e4cc486143 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -244,6 +244,7 @@ class MenuItem extends AutoControlledComponent, MenuIt performClick: event => this.handleClick(event), openSubmenu: event => this.openSubmenu(event), closeMenu: event => this.closeMenu(event), + closeMenuAndFocusNextParentItem: event => this.closeMenuAndFocusNextParentItem(event), closeSubmenu: event => this.closeSubmenu(event), } @@ -303,7 +304,18 @@ class MenuItem extends AutoControlledComponent, MenuIt const { submenuOpen } = this.state if (menu && submenuOpen) { this.setState({ submenuOpen: false }) - if (!inSubmenu && (keyboardKey.getCode(e) === keyboardKey.Escape || this.props.vertical)) { + if (!inSubmenu) { + focusAsync(this.itemRef.current) + } + } + } + + private closeMenuAndFocusNextParentItem = e => { + const { menu, inSubmenu } = this.props + const { submenuOpen } = this.state + if (menu && submenuOpen) { + this.setState({ submenuOpen: false }) + if (!inSubmenu && this.props.vertical) { focusAsync(this.itemRef.current) } } diff --git a/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts b/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts index 4b3e0e4efd..e90bf23376 100644 --- a/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts +++ b/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts @@ -39,7 +39,10 @@ const menuItemBehavior: Accessibility = (props: any) => ({ keyCombinations: [{ keyCode: keyboardKey.Enter }, { keyCode: keyboardKey.Spacebar }], }, closeMenu: { - keyCombinations: [{ keyCode: keyboardKey.Escape }, { keyCode: keyboardKey.ArrowRight }], + keyCombinations: [{ keyCode: keyboardKey.Escape }], + }, + closeMenuAndFocusNextParentItem: { + keyCombinations: [{ keyCode: keyboardKey.ArrowRight }], }, closeSubmenu: { keyCombinations: [{ keyCode: keyboardKey.ArrowLeft }], From f5e66f0c6658a120d205083fbe331ce46f5d91ab Mon Sep 17 00:00:00 2001 From: manajdov Date: Mon, 17 Dec 2018 18:27:12 +0100 Subject: [PATCH 36/55] -refactored conditions using doesNodeContainClick --- src/components/Menu/MenuItem.tsx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index e4cc486143..520cbdcfca 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -2,13 +2,13 @@ import * as _ from 'lodash' import cx from 'classnames' import * as PropTypes from 'prop-types' import * as React from 'react' -import * as keyboardKey from 'keyboard-key' import { AutoControlledComponent, childrenExist, createShorthandFactory, customPropTypes, + doesNodeContainClick, UIComponentProps, ChildrenComponentProps, ContentComponentProps, @@ -260,9 +260,8 @@ class MenuItem extends AutoControlledComponent, MenuIt private outsideClickHandler = e => { if ( - this.itemRef && - (!this.itemRef.current || !this.itemRef.current.contains(e.target)) && - (!this.submenuRef.current || !this.submenuRef.current.contains(e.target)) + !doesNodeContainClick(this.itemRef.current, e) && + !doesNodeContainClick(this.submenuRef.current, e) ) { this.state.submenuOpen && this.trySetState({ submenuOpen: false }) } @@ -271,7 +270,7 @@ class MenuItem extends AutoControlledComponent, MenuIt private performClick = e => { const { active, menu } = this.props if (menu) { - if (this.submenuRef.current && this.submenuRef.current.contains(e.target)) { + if (doesNodeContainClick(this.submenuRef.current, e)) { // submenu was clicked => close it and propagate this.setState({ submenuOpen: false }, () => focusAsync(this.itemRef.current)) } else { From 4c8e2d66c5aaf66de08c7d10a8374ea972c580d4 Mon Sep 17 00:00:00 2001 From: manajdov Date: Tue, 18 Dec 2018 15:03:45 +0100 Subject: [PATCH 37/55] -renamed submenu* props to menu in the MenuItem component -changed setActiveIndex with onActiveChanged -introduced different styles for the hovering vs active elements --- ...nuExampleVerticalWithSubmenu.shorthand.tsx | 18 +---- .../MenuExampleWithSubmenu.shorthand.tsx | 18 +---- src/components/Menu/Menu.tsx | 10 ++- src/components/Menu/MenuItem.tsx | 80 +++++++++---------- .../Behaviors/Menu/menuItemBehavior.ts | 6 +- .../teams/components/Menu/menuItemStyles.ts | 38 +++++++-- .../teams/components/Menu/menuVariables.ts | 8 ++ 7 files changed, 96 insertions(+), 82 deletions(-) diff --git a/docs/src/examples/components/Menu/Types/MenuExampleVerticalWithSubmenu.shorthand.tsx b/docs/src/examples/components/Menu/Types/MenuExampleVerticalWithSubmenu.shorthand.tsx index 1e8dae4410..807b6725a9 100644 --- a/docs/src/examples/components/Menu/Types/MenuExampleVerticalWithSubmenu.shorthand.tsx +++ b/docs/src/examples/components/Menu/Types/MenuExampleVerticalWithSubmenu.shorthand.tsx @@ -1,5 +1,5 @@ import React from 'react' -import { Menu, Provider } from '@stardust-ui/react' +import { Menu } from '@stardust-ui/react' const items = [ { @@ -20,20 +20,6 @@ const items = [ { key: 'events', content: 'Upcoming Events' }, ] -const MenuExampleVerticalWithSubmenu = () => ( - - - -) +const MenuExampleVerticalWithSubmenu = () => export default MenuExampleVerticalWithSubmenu diff --git a/docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx b/docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx index d2d195cc3b..4899c64fef 100644 --- a/docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx +++ b/docs/src/examples/components/Menu/Types/MenuExampleWithSubmenu.shorthand.tsx @@ -1,5 +1,5 @@ import React from 'react' -import { Menu, Provider } from '@stardust-ui/react' +import { Menu } from '@stardust-ui/react' const items = [ { @@ -38,20 +38,6 @@ const items = [ { key: 'events', content: 'Upcoming Events' }, ] -const MenuExampleWithSubMenu = () => ( - - - -) +const MenuExampleWithSubMenu = () => export default MenuExampleWithSubMenu diff --git a/src/components/Menu/Menu.tsx b/src/components/Menu/Menu.tsx index 4321fa9e3c..70c4ca03c4 100644 --- a/src/components/Menu/Menu.tsx +++ b/src/components/Menu/Menu.tsx @@ -116,8 +116,14 @@ class Menu extends AutoControlledComponent, MenuState> { _.invoke(predefinedProps, 'onClick', e, itemProps) }, - setActiveIndex: index => { - this.trySetState({ activeIndex: index }) + onActiveChanged: (e, props) => { + const { index, active } = props + if (active) { + this.trySetState({ activeIndex: index }) + } else if (this.state.activeIndex === index) { + this.trySetState({ activeIndex: null }) + } + _.invoke(predefinedProps, 'onActiveChanged', e, props) }, }) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index 520cbdcfca..ce7d4d2bd3 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -95,14 +95,14 @@ export interface MenuItemProps /** Shorthand for the submenu. */ menu?: ShorthandValue - /** Indicates if the submenu is open */ - submenuOpen?: boolean + /** Indicates if the menu inside the item is open. */ + menuOpen?: boolean - /** Default submenu open */ - defaultSubmenuOpen?: boolean + /** Default menu open */ + defaultMenuOpen?: boolean /** Callback for setting the current menu item as active element in the menu. */ - setActiveIndex?: (idx: number | string) => void + onActiveChanged?: ComponentEventHandler /** Indicates whether the menu item is part of submenu. */ inSubmenu?: boolean @@ -110,7 +110,7 @@ export interface MenuItemProps export interface MenuItemState { isFromKeyboard: boolean - submenuOpen: boolean + menuOpen: boolean } /** @@ -140,9 +140,9 @@ class MenuItem extends AutoControlledComponent, MenuIt vertical: PropTypes.bool, wrapper: PropTypes.oneOfType([PropTypes.node, PropTypes.object]), menu: customPropTypes.itemShorthand, - submenuOpen: PropTypes.bool, - defaultSubmenuOpen: PropTypes.bool, - setActiveIndex: PropTypes.func, + menuOpen: PropTypes.bool, + defaultMenuOpen: PropTypes.bool, + onActiveChanged: PropTypes.func, inSubmenu: PropTypes.bool, } @@ -152,11 +152,11 @@ class MenuItem extends AutoControlledComponent, MenuIt wrapper: { as: 'li' }, } - static autoControlledProps = ['submenuOpen'] + static autoControlledProps = ['menuOpen'] private outsideClickSubscription = EventStack.noSubscription - private submenuRef = React.createRef() + private menuRef = React.createRef() private itemRef = React.createRef() public componentDidMount() { @@ -174,7 +174,7 @@ class MenuItem extends AutoControlledComponent, MenuIt renderComponent({ ElementType, classes, accessibility, rest, styles }) { const { children, content, icon, wrapper, menu, primary, secondary, active } = this.props - const { submenuOpen } = this.state + const { menuOpen } = this.state const menuItemInner = childrenExist(children) ? ( children @@ -197,8 +197,8 @@ class MenuItem extends AutoControlledComponent, MenuIt ) const maybeSubmenu = - menu && active && submenuOpen ? ( - + menu && active && menuOpen ? ( + {Menu.create(menu, { defaultProps: { accessibility: submenuBehavior, @@ -236,22 +236,22 @@ class MenuItem extends AutoControlledComponent, MenuIt private handleWrapperBlur = e => { if (!this.props.inSubmenu && !e.currentTarget.contains(e.relatedTarget)) { - this.setState({ submenuOpen: false }) + this.setState({ menuOpen: false }) } } protected actionHandlers: AccessibilityActionHandlers = { performClick: event => this.handleClick(event), - openSubmenu: event => this.openSubmenu(event), - closeMenu: event => this.closeMenu(event), + openMenu: event => this.openMenu(event), + closeAllMenus: event => this.closeAllMenus(event), closeMenuAndFocusNextParentItem: event => this.closeMenuAndFocusNextParentItem(event), - closeSubmenu: event => this.closeSubmenu(event), + closeMenu: event => this.closeMenu(event), } private updateOutsideClickSubscription() { this.outsideClickSubscription.unsubscribe() - if (this.props.menu && this.state.submenuOpen) { + if (this.props.menu && this.state.menuOpen) { setTimeout(() => { this.outsideClickSubscription = EventStack.subscribe('click', this.outsideClickHandler) }) @@ -261,21 +261,21 @@ class MenuItem extends AutoControlledComponent, MenuIt private outsideClickHandler = e => { if ( !doesNodeContainClick(this.itemRef.current, e) && - !doesNodeContainClick(this.submenuRef.current, e) + !doesNodeContainClick(this.menuRef.current, e) ) { - this.state.submenuOpen && this.trySetState({ submenuOpen: false }) + this.state.menuOpen && this.trySetState({ menuOpen: false }) } } private performClick = e => { const { active, menu } = this.props if (menu) { - if (doesNodeContainClick(this.submenuRef.current, e)) { + if (doesNodeContainClick(this.menuRef.current, e)) { // submenu was clicked => close it and propagate - this.setState({ submenuOpen: false }, () => focusAsync(this.itemRef.current)) + this.setState({ menuOpen: false }, () => focusAsync(this.itemRef.current)) } else { // the menuItem element was clicked => toggle the open/close and stop propagation - this.trySetState({ submenuOpen: active ? !this.state.submenuOpen : true }) + this.trySetState({ menuOpen: active ? !this.state.menuOpen : true }) e.stopPropagation() } } @@ -298,11 +298,11 @@ class MenuItem extends AutoControlledComponent, MenuIt _.invoke(this.props, 'onFocus', e, this.props) } - private closeMenu = e => { + private closeAllMenus = e => { const { menu, inSubmenu } = this.props - const { submenuOpen } = this.state - if (menu && submenuOpen) { - this.setState({ submenuOpen: false }) + const { menuOpen } = this.state + if (menu && menuOpen) { + this.setState({ menuOpen: false }) if (!inSubmenu) { focusAsync(this.itemRef.current) } @@ -311,21 +311,21 @@ class MenuItem extends AutoControlledComponent, MenuIt private closeMenuAndFocusNextParentItem = e => { const { menu, inSubmenu } = this.props - const { submenuOpen } = this.state - if (menu && submenuOpen) { - this.setState({ submenuOpen: false }) + const { menuOpen } = this.state + if (menu && menuOpen) { + this.setState({ menuOpen: false }) if (!inSubmenu && this.props.vertical) { focusAsync(this.itemRef.current) } } } - private closeSubmenu = e => { + private closeMenu = e => { const { menu, inSubmenu } = this.props - const { submenuOpen } = this.state + const { menuOpen } = this.state const shouldStopPropagation = inSubmenu || this.props.vertical - if (menu && submenuOpen) { - this.setState({ submenuOpen: false }, () => { + if (menu && menuOpen) { + this.setState({ menuOpen: false }, () => { if (shouldStopPropagation) { focusAsync(this.itemRef.current) } @@ -336,12 +336,12 @@ class MenuItem extends AutoControlledComponent, MenuIt } } - private openSubmenu = e => { + private openMenu = e => { const { menu } = this.props - const { submenuOpen } = this.state - if (menu && !submenuOpen) { - this.setState({ submenuOpen: true }) - _.invoke(this.props, 'setActiveIndex', this.props.index) + const { menuOpen } = this.state + if (menu && !menuOpen) { + this.setState({ menuOpen: true }) + _.invoke(this.props, 'onActiveChanged', e, { ...this.props, active: true }) e.stopPropagation() e.preventDefault() } diff --git a/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts b/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts index e90bf23376..e2332aa3b4 100644 --- a/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts +++ b/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts @@ -38,16 +38,16 @@ const menuItemBehavior: Accessibility = (props: any) => ({ performClick: { keyCombinations: [{ keyCode: keyboardKey.Enter }, { keyCode: keyboardKey.Spacebar }], }, - closeMenu: { + closeAllMenus: { keyCombinations: [{ keyCode: keyboardKey.Escape }], }, closeMenuAndFocusNextParentItem: { keyCombinations: [{ keyCode: keyboardKey.ArrowRight }], }, - closeSubmenu: { + closeMenu: { keyCombinations: [{ keyCode: keyboardKey.ArrowLeft }], }, - openSubmenu: { + openMenu: { keyCombinations: [ { keyCode: props.vertical ? keyboardKey.ArrowRight : keyboardKey.ArrowDown }, ], diff --git a/src/themes/teams/components/Menu/menuItemStyles.ts b/src/themes/teams/components/Menu/menuItemStyles.ts index d98017d23b..c3dc8c2b19 100644 --- a/src/themes/teams/components/Menu/menuItemStyles.ts +++ b/src/themes/teams/components/Menu/menuItemStyles.ts @@ -20,7 +20,7 @@ const getActionStyles = ({ variables: MenuVariables color: string }): ICSSInJSStyle => - (underlined && !isFromKeyboard) || iconOnly + underlined || iconOnly ? { color, background: v.backgroundColor, @@ -35,6 +35,35 @@ const getActionStyles = ({ background: v.activeBackgroundColor, } +const getFocusedStyles = ({ + props, + variables: v, + color, +}: { + props: MenuItemPropsAndState + variables: MenuVariables + color: string +}): ICSSInJSStyle => { + const { primary, underlined, iconOnly, isFromKeyboard, active } = props + if (active) return {} + return { + ...((underlined && !isFromKeyboard) || iconOnly + ? { + color, + background: v.backgroundColor, + } + : primary + ? { + color: v.primaryFocusedColor, + background: v.primaryFocusedBackgroundColor, + } + : { + color, + background: v.focusedBackgroundColor, + }), + } +} + const itemSeparator: ComponentSlotStyleFunction = ({ props, variables: v, @@ -191,10 +220,10 @@ const menuItemStyles: ComponentSlotStylesInput ({ - // background: 'white', - // zIndex: '1000', + zIndex: '1000', position: 'absolute', top: vertical ? '0' : '100%', left: vertical ? '100%' : '0', diff --git a/src/themes/teams/components/Menu/menuVariables.ts b/src/themes/teams/components/Menu/menuVariables.ts index fb778e4455..3558f204b1 100644 --- a/src/themes/teams/components/Menu/menuVariables.ts +++ b/src/themes/teams/components/Menu/menuVariables.ts @@ -6,12 +6,16 @@ export interface MenuVariables { activeColor: string activeBackgroundColor: string + focusedBackgroundColor: string borderColor: string primaryActiveColor: string primaryActiveBackgroundColor: string primaryActiveBorderColor: string + primaryFocusedColor: string + primaryFocusedBackgroundColor: string + primaryBorderColor: string primaryHoverBorderColor: string primaryUnderlinedBorderColor: string @@ -30,12 +34,16 @@ export default (siteVars: any): MenuVariables => { activeColor: siteVars.black, activeBackgroundColor: siteVars.gray10, + focusedBackgroundColor: siteVars.gray14, borderColor: siteVars.gray08, primaryActiveColor: siteVars.white, primaryActiveBackgroundColor: siteVars.brand08, primaryActiveBorderColor: siteVars.brand, + primaryFocusedColor: siteVars.white, + primaryFocusedBackgroundColor: siteVars.brand12, + primaryBorderColor: siteVars.brand08, primaryHoverBorderColor: siteVars.gray08, primaryUnderlinedBorderColor: siteVars.gray08, From 2ece67bff7c01e34dba0f5ad8e8b9450f53c736c Mon Sep 17 00:00:00 2001 From: manajdov Date: Tue, 18 Dec 2018 18:46:23 +0100 Subject: [PATCH 38/55] -improved example -fixed issue with the condition for the active prop --- docs/src/examples/components/Menu/Types/index.tsx | 6 +----- src/components/Menu/Menu.tsx | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/docs/src/examples/components/Menu/Types/index.tsx b/docs/src/examples/components/Menu/Types/index.tsx index 221c7fc22a..9eb56ac99b 100644 --- a/docs/src/examples/components/Menu/Types/index.tsx +++ b/docs/src/examples/components/Menu/Types/index.tsx @@ -24,11 +24,7 @@ const Types = () => ( description="A menu can have submenus." examplePath="components/Menu/Types/MenuExampleWithSubmenu" /> - + ) diff --git a/src/components/Menu/Menu.tsx b/src/components/Menu/Menu.tsx index 70c4ca03c4..85bebbdce1 100644 --- a/src/components/Menu/Menu.tsx +++ b/src/components/Menu/Menu.tsx @@ -143,7 +143,7 @@ class Menu extends AutoControlledComponent, MenuState> { return _.map(items, (item, index) => { const active = - typeof activeIndex === 'string' ? parseInt(activeIndex, 10) : activeIndex === index + (typeof activeIndex === 'string' ? parseInt(activeIndex, 10) : activeIndex) === index return MenuItem.create(item, { defaultProps: { iconOnly, From a97a2febcd4271a79fb27aa9d519a5e4fce229fa Mon Sep 17 00:00:00 2001 From: manajdov Date: Tue, 18 Dec 2018 18:51:05 +0100 Subject: [PATCH 39/55] -exported MenuState -added correct typings to the menuStyles --- src/index.ts | 2 +- src/themes/teams/components/Menu/menuStyles.ts | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/index.ts b/src/index.ts index af883aeb8b..523f5ae664 100644 --- a/src/index.ts +++ b/src/index.ts @@ -74,7 +74,7 @@ export { default as Layout, LayoutPropsWithDefaults, LayoutProps } from './compo export { default as List, ListProps } from './components/List/List' export { default as ListItem, ListItemState, ListItemProps } from './components/List/ListItem' -export { default as Menu, MenuProps } from './components/Menu/Menu' +export { default as Menu, MenuProps, MenuState } from './components/Menu/Menu' export { default as MenuItem, MenuItemState, MenuItemProps } from './components/Menu/MenuItem' export { default as Popup, PopupState, PopupProps } from './components/Popup/Popup' diff --git a/src/themes/teams/components/Menu/menuStyles.ts b/src/themes/teams/components/Menu/menuStyles.ts index f21091389c..d3fc40d934 100644 --- a/src/themes/teams/components/Menu/menuStyles.ts +++ b/src/themes/teams/components/Menu/menuStyles.ts @@ -1,6 +1,9 @@ import { pxToRem } from '../../utils' import { ComponentSlotStylesInput, ICSSInJSStyle } from '../../../types' -import { MenuProps } from '../../../../components/Menu/Menu' +import { MenuProps, MenuState } from '../../../../components/Menu/Menu' +import { MenuVariables } from './menuVariables' + +type MenuPropsAndState = MenuProps & MenuState const solidBorder = (color: string) => ({ border: `1px solid ${color}`, @@ -39,4 +42,4 @@ export default { listStyleType: 'none', } }, -} as ComponentSlotStylesInput +} as ComponentSlotStylesInput From c590f3e3b78feaee340fc7db201adb05082c9a82 Mon Sep 17 00:00:00 2001 From: manajdov Date: Tue, 18 Dec 2018 19:14:23 +0100 Subject: [PATCH 40/55] -fixed underlined active + hovered style --- src/themes/teams/components/Menu/menuItemStyles.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/themes/teams/components/Menu/menuItemStyles.ts b/src/themes/teams/components/Menu/menuItemStyles.ts index c3dc8c2b19..367cf8e404 100644 --- a/src/themes/teams/components/Menu/menuItemStyles.ts +++ b/src/themes/teams/components/Menu/menuItemStyles.ts @@ -45,7 +45,7 @@ const getFocusedStyles = ({ color: string }): ICSSInJSStyle => { const { primary, underlined, iconOnly, isFromKeyboard, active } = props - if (active) return {} + if (active && !underlined) return {} return { ...((underlined && !isFromKeyboard) || iconOnly ? { From 77342eaa42be0c5d69be04bdbb90654cce9ca40e Mon Sep 17 00:00:00 2001 From: manajdov Date: Wed, 19 Dec 2018 11:51:30 +0100 Subject: [PATCH 41/55] -fixed border corner clipped by adding custom styles for the first child menu items and the last child menu items in vertical menu --- .../teams/components/Menu/menuItemStyles.ts | 39 +++++++++++++++---- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/src/themes/teams/components/Menu/menuItemStyles.ts b/src/themes/teams/components/Menu/menuItemStyles.ts index 367cf8e404..36ef09fbf5 100644 --- a/src/themes/teams/components/Menu/menuItemStyles.ts +++ b/src/themes/teams/components/Menu/menuItemStyles.ts @@ -83,14 +83,6 @@ const itemSeparator: ComponentSlotStyleFunction Date: Wed, 19 Dec 2018 15:40:03 +0100 Subject: [PATCH 42/55] -addressed comments on PR --- src/components/Menu/MenuItem.tsx | 29 +++++++------------ .../Behaviors/Menu/menuItemBehavior.ts | 2 +- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx index ce7d4d2bd3..143734a42c 100644 --- a/src/components/Menu/MenuItem.tsx +++ b/src/components/Menu/MenuItem.tsx @@ -243,8 +243,8 @@ class MenuItem extends AutoControlledComponent, MenuIt protected actionHandlers: AccessibilityActionHandlers = { performClick: event => this.handleClick(event), openMenu: event => this.openMenu(event), - closeAllMenus: event => this.closeAllMenus(event), - closeMenuAndFocusNextParentItem: event => this.closeMenuAndFocusNextParentItem(event), + closeAllMenus: event => this.closeAllMenus(event, false), + closeAllMenusAndFocusNextParentItem: event => this.closeAllMenus(event, true), closeMenu: event => this.closeMenu(event), } @@ -259,11 +259,12 @@ class MenuItem extends AutoControlledComponent, MenuIt } private outsideClickHandler = e => { + if (!this.state.menuOpen) return if ( !doesNodeContainClick(this.itemRef.current, e) && !doesNodeContainClick(this.menuRef.current, e) ) { - this.state.menuOpen && this.trySetState({ menuOpen: false }) + this.trySetState({ menuOpen: false }) } } @@ -298,25 +299,15 @@ class MenuItem extends AutoControlledComponent, MenuIt _.invoke(this.props, 'onFocus', e, this.props) } - private closeAllMenus = e => { + private closeAllMenus = (e, focusNextParent: boolean) => { const { menu, inSubmenu } = this.props const { menuOpen } = this.state if (menu && menuOpen) { - this.setState({ menuOpen: false }) - if (!inSubmenu) { - focusAsync(this.itemRef.current) - } - } - } - - private closeMenuAndFocusNextParentItem = e => { - const { menu, inSubmenu } = this.props - const { menuOpen } = this.state - if (menu && menuOpen) { - this.setState({ menuOpen: false }) - if (!inSubmenu && this.props.vertical) { - focusAsync(this.itemRef.current) - } + this.setState({ menuOpen: false }, () => { + if (!inSubmenu && (!focusNextParent || this.props.vertical)) { + focusAsync(this.itemRef.current) + } + }) } } diff --git a/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts b/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts index e2332aa3b4..730b783f9c 100644 --- a/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts +++ b/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts @@ -41,7 +41,7 @@ const menuItemBehavior: Accessibility = (props: any) => ({ closeAllMenus: { keyCombinations: [{ keyCode: keyboardKey.Escape }], }, - closeMenuAndFocusNextParentItem: { + closeAllMenusAndFocusNextParentItem: { keyCombinations: [{ keyCode: keyboardKey.ArrowRight }], }, closeMenu: { From 54f0a69083c69bce253995794378e5b3a722a631 Mon Sep 17 00:00:00 2001 From: Miroslav Stastny Date: Mon, 17 Dec 2018 14:33:49 +0100 Subject: [PATCH 43/55] chore: prepare release 0.15.0 [ci skip] --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a37147e870..6140d7bc1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] + +## [v0.15.0](https://github.com/stardust-ui/react/tree/v0.15.0) (2018-12-17) +[Compare changes](https://github.com/stardust-ui/react/compare/v0.14.0...v0.15.0) + ### BREAKING CHANGES - `type` prop is replaced with `color` in `Divider` component @layershifter ([#558](https://github.com/stardust-ui/react/pull/558)) - Remove `createColorVariants` and `setColorLightness` utils @layershifter ([#583](https://github.com/stardust-ui/react/pull/583)) From 990948342ac2d492472cb77e401a5c8f58c9acfb Mon Sep 17 00:00:00 2001 From: Miroslav Stastny Date: Mon, 17 Dec 2018 14:38:53 +0100 Subject: [PATCH 44/55] 0.15.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 75c2f3976d..a9d2b60649 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@stardust-ui/react", - "version": "0.14.0", + "version": "0.15.0", "description": "A themable React component library.", "jsnext:main": "dist/es/index.js", "main": "dist/commonjs/index.js", From fa2e9bd9666bdc5c6ac0488492e03c5dea09b36d Mon Sep 17 00:00:00 2001 From: Sofiya Huts <8460706+sophieH29@users.noreply.github.com> Date: Mon, 17 Dec 2018 17:22:07 +0100 Subject: [PATCH 45/55] fix(Prototype): Fix Popover prototype after breaking changes (#623) --- .../ChatMessageWithPopover.tsx | 6 ----- .../ChatWithPopover.tsx | 25 ++++++++++++++++--- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/docs/src/prototypes/chatMessageWithPopover/ChatMessageWithPopover.tsx b/docs/src/prototypes/chatMessageWithPopover/ChatMessageWithPopover.tsx index ea3454ed1f..62f17dc69b 100644 --- a/docs/src/prototypes/chatMessageWithPopover/ChatMessageWithPopover.tsx +++ b/docs/src/prototypes/chatMessageWithPopover/ChatMessageWithPopover.tsx @@ -4,11 +4,6 @@ import * as React from 'react' import cx from 'classnames' import Popover from './Popover' -const janeAvatar = { - image: 'public/images/avatar/small/ade.jpg', - status: { color: 'green', icon: 'check' }, -} - interface ChatMessageWithPopoverProps { className?: string } @@ -51,7 +46,6 @@ class ChatMessageWithPopover extends React.Component<
), }} - avatar={janeAvatar} onFocus={this.handleFocus} onBlur={this.handleBlur} className={cx(this.props.className, this.state.focused ? 'focused' : '')} diff --git a/docs/src/prototypes/chatMessageWithPopover/ChatWithPopover.tsx b/docs/src/prototypes/chatMessageWithPopover/ChatWithPopover.tsx index 6796bae0fa..06c4e3b1b5 100644 --- a/docs/src/prototypes/chatMessageWithPopover/ChatWithPopover.tsx +++ b/docs/src/prototypes/chatMessageWithPopover/ChatWithPopover.tsx @@ -1,7 +1,12 @@ -import { Chat, Provider } from '@stardust-ui/react' +import { Chat, Provider, Avatar } from '@stardust-ui/react' import * as React from 'react' import ChatMessageWithPopover from './ChatMessageWithPopover' +const janeAvatar = { + image: 'public/images/avatar/small/ade.jpg', + status: { color: 'green', icon: 'check' }, +} + const ChatWithPopover = () => ( ( > }, - { key: 'b', content: }, - { key: 'c', content: }, + { + key: 'a', + message: { content: }, + gutter: { content: }, + }, + { + key: 'b', + message: { content: }, + gutter: { content: }, + }, + { + key: 'c', + message: { content: }, + gutter: { content: }, + }, ]} /> From df0b11a08cf1cfe000fbd34fb050e1097ed064e3 Mon Sep 17 00:00:00 2001 From: kuzhelov Date: Tue, 18 Dec 2018 12:50:35 +0100 Subject: [PATCH 46/55] chore: cache results of vulnerability scans (#621) * implement caching strategy * adjust file name of scan marker * add yarn lock hash to marker file name * add change to build config * fix dir name in build config * improve caching strategy * just restore cache * temporary remove lint and tests * try * fix caching strategy * try * try * try * try epoch * create file on scan * return lint and test steps * introduce comment for the caching approach taken * remove unnecessary function * simplify expression for marker file name --- .circleci/config.yml | 9 +++++ build/gulp/tasks/test-vulns.ts | 65 ++++++++++++++++++++++++++++++++++ gulpfile.ts | 1 + package.json | 2 +- 4 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 build/gulp/tasks/test-vulns.ts diff --git a/.circleci/config.yml b/.circleci/config.yml index 328faa1561..25f0d08b77 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -49,9 +49,18 @@ jobs: - run: name: Report coverage command: bash <(curl -s https://codecov.io/bash) + + - restore_cache: + key: v1-vuln-scans-{{ checksum "yarn.lock" }} - run: name: Vulnerability Tests command: yarn test:vulns + # https://discuss.circleci.com/t/add-mechanism-to-update-existing-cache-key/9014/12 + - save_cache: + key: v1-vuln-scans-{{ checksum "yarn.lock" }}-{{ epoch }} + paths: + - .vuln-scans + - run: name: Visual Tests command: yarn test:visual diff --git a/build/gulp/tasks/test-vulns.ts b/build/gulp/tasks/test-vulns.ts new file mode 100644 index 0000000000..b20351292e --- /dev/null +++ b/build/gulp/tasks/test-vulns.ts @@ -0,0 +1,65 @@ +import * as fs from 'fs' +import { task } from 'gulp' +import * as path from 'path' +import debug from 'debug' + +import config from '../../../config' +import sh from '../sh' + +const { paths } = config + +const SCAN_RESULTS_DIR_NAME = '.vuln-scans' +const SCAN_RESULTS_DIR_PATH = paths.base(SCAN_RESULTS_DIR_NAME) + +const log = message => debug.log(message) +log.success = message => debug.log(`✔ ${message}`) + +const ensureDirExists = path => { + if (!fs.existsSync(path)) { + sh(`mkdir -p ${path}`) + } +} + +const getTodayScanFilePath = () => { + const now = new Date() + + const year = now.getUTCFullYear() + const month = now.getUTCMonth() + 1 + const date = now.getUTCDate() + + const fileName = `snyk-scanned-${year}-${month}-${date}` + + return path.resolve(SCAN_RESULTS_DIR_PATH, fileName) +} + +const recentlyChecked = () => { + const recentCheckFilePath = getTodayScanFilePath() + return fs.existsSync(recentCheckFilePath) +} + +const registerRecentSucessfulScan = async () => { + ensureDirExists(SCAN_RESULTS_DIR_PATH) + + const recentScanFilePath = getTodayScanFilePath() + await sh(`touch ${recentScanFilePath}`) +} + +/** + * The following strategy is used to perform vulnerabilites scan + * - check if there is marker of recent sucessful scan + * - if this marker exists, skip checks + * - if there is no marker, perform check + * - if check is successful, create successful check marker + */ +task('test:vulns', async () => { + if (recentlyChecked()) { + log.success('Vulnerabilities check was already performed recently, skipping..') + return + } + + log('Scanning dependency packages for vulnerabilities..') + await sh(`yarn snyk test`) + log.success('Vulnerability scan is successfully passed.') + + registerRecentSucessfulScan() +}) diff --git a/gulpfile.ts b/gulpfile.ts index ba23bc51e9..c34184f67a 100644 --- a/gulpfile.ts +++ b/gulpfile.ts @@ -14,6 +14,7 @@ require('./build/gulp/tasks/screener') require('./build/gulp/tasks/git') require('./build/gulp/tasks/test-unit') require('./build/gulp/tasks/test-projects') +require('./build/gulp/tasks/test-vulns') // global tasks task('build', series('dll', parallel('dist', 'build:docs'))) diff --git a/package.json b/package.json index a9d2b60649..d645944e84 100644 --- a/package.json +++ b/package.json @@ -35,7 +35,7 @@ "pretest": "yarn satisfied", "test": "gulp test", "test:watch": "gulp test:watch", - "test:vulns": "snyk test", + "test:vulns": "gulp test:vulns", "test:visual": "gulp screener", "test:projects": "gulp test:projects", "generate:component": "gulp generate:component" From 1d8e16874eefca1e25606727cf4b849ef7c31881 Mon Sep 17 00:00:00 2001 From: Alexandru Buliga Date: Tue, 18 Dec 2018 14:16:35 +0200 Subject: [PATCH 47/55] feat(text): color prop (#597) * feat(text): color prop * addressed comments * changelog * amended changelog * made text color override other props that change color --- CHANGELOG.md | 3 ++ .../Variations/TextExampleColor.shorthand.tsx | 18 ++++++++++ .../Text/Variations/TextExampleColor.tsx | 20 +++++++++++ .../components/Text/Variations/index.tsx | 5 +++ src/components/Text/Text.tsx | 9 +++-- src/lib/colorUtils.ts | 11 ++++++ src/lib/index.ts | 1 + src/themes/teams/colors.ts | 1 - .../teams/components/Divider/dividerStyles.ts | 35 ++++++++----------- .../components/Divider/dividerVariables.ts | 14 ++++---- .../teams/components/Text/textStyles.ts | 4 +++ .../teams/components/Text/textVariables.ts | 7 ++++ src/themes/types.ts | 10 ++++++ 13 files changed, 109 insertions(+), 29 deletions(-) create mode 100644 docs/src/examples/components/Text/Variations/TextExampleColor.shorthand.tsx create mode 100644 docs/src/examples/components/Text/Variations/TextExampleColor.tsx create mode 100644 src/lib/colorUtils.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6140d7bc1c..e2293b5161 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Features +- Add `color` prop to `Text` component @Bugaa92 ([#597](https://github.com/stardust-ui/react/pull/597)) + ## [v0.15.0](https://github.com/stardust-ui/react/tree/v0.15.0) (2018-12-17) [Compare changes](https://github.com/stardust-ui/react/compare/v0.14.0...v0.15.0) diff --git a/docs/src/examples/components/Text/Variations/TextExampleColor.shorthand.tsx b/docs/src/examples/components/Text/Variations/TextExampleColor.shorthand.tsx new file mode 100644 index 0000000000..7997c1b1e0 --- /dev/null +++ b/docs/src/examples/components/Text/Variations/TextExampleColor.shorthand.tsx @@ -0,0 +1,18 @@ +import React from 'react' +import _ from 'lodash' +import { Text, ProviderConsumer } from '@stardust-ui/react' + +const TextExampleColor = () => ( + + _.keys({ ...emphasisColors, ...naturalColors }).map(color => ( + <> + +
+ + )) + } + /> +) + +export default TextExampleColor diff --git a/docs/src/examples/components/Text/Variations/TextExampleColor.tsx b/docs/src/examples/components/Text/Variations/TextExampleColor.tsx new file mode 100644 index 0000000000..79b78d5ae2 --- /dev/null +++ b/docs/src/examples/components/Text/Variations/TextExampleColor.tsx @@ -0,0 +1,20 @@ +import React from 'react' +import _ from 'lodash' +import { Text, ProviderConsumer } from '@stardust-ui/react' + +const TextExampleColor = () => ( + + _.keys({ ...emphasisColors, ...naturalColors }).map(color => ( + <> + + {_.startCase(color)} + +
+ + )) + } + /> +) + +export default TextExampleColor diff --git a/docs/src/examples/components/Text/Variations/index.tsx b/docs/src/examples/components/Text/Variations/index.tsx index b2721770bf..73d9b34082 100644 --- a/docs/src/examples/components/Text/Variations/index.tsx +++ b/docs/src/examples/components/Text/Variations/index.tsx @@ -4,6 +4,11 @@ import ExampleSection from 'docs/src/components/ComponentDoc/ExampleSection' const Variations = () => ( + , any> { static displayName = 'Text' static propTypes = { - ...commonPropTypes.createCommon(), + ...commonPropTypes.createCommon({ color: true }), atMention: PropTypes.oneOfType([PropTypes.bool, PropTypes.oneOf(['me'])]), disabled: PropTypes.bool, error: PropTypes.bool, diff --git a/src/lib/colorUtils.ts b/src/lib/colorUtils.ts new file mode 100644 index 0000000000..471b27aa54 --- /dev/null +++ b/src/lib/colorUtils.ts @@ -0,0 +1,11 @@ +import * as _ from 'lodash' +import { SiteVariablesInput, ColorVariants, ColorValues } from '../themes/types' + +export const mapColorsToScheme = ( + siteVars: SiteVariablesInput, + mapper: keyof ColorVariants | ((color: ColorVariants) => T), +): ColorValues => + _.mapValues( + { ...siteVars.emphasisColors, ...siteVars.naturalColors }, + typeof mapper === 'number' ? String(mapper) : (mapper as any), + ) as ColorValues diff --git a/src/lib/index.ts b/src/lib/index.ts index 4415498664..7ad00ecc6e 100644 --- a/src/lib/index.ts +++ b/src/lib/index.ts @@ -3,6 +3,7 @@ import * as commonPropTypes from './commonPropTypes' export { default as AutoControlledComponent } from './AutoControlledComponent' export { default as childrenExist } from './childrenExist' +export { mapColorsToScheme } from './colorUtils' export { default as UIComponent } from './UIComponent' export { EventStack } from './eventStack' export { felaRenderer, felaRtlRenderer } from './felaRenderer' diff --git a/src/themes/teams/colors.ts b/src/themes/teams/colors.ts index ab7bec2375..d342ea9d54 100644 --- a/src/themes/teams/colors.ts +++ b/src/themes/teams/colors.ts @@ -88,7 +88,6 @@ export const naturalColors: NaturalColors = { 800: '#F9D844', 900: '#F8D22A', }, - darkOrange: { 50: '#F9ECEA', 100: '#ECBCB3', diff --git a/src/themes/teams/components/Divider/dividerStyles.ts b/src/themes/teams/components/Divider/dividerStyles.ts index b912214933..6cfc94488b 100644 --- a/src/themes/teams/components/Divider/dividerStyles.ts +++ b/src/themes/teams/components/Divider/dividerStyles.ts @@ -2,37 +2,32 @@ import * as _ from 'lodash' import { childrenExist } from '../../../../lib' import { pxToRem } from '../../utils' -import { ComponentSlotStylesInput, ICSSInJSStyle, ICSSPseudoElementStyle } from '../../../types' +import { ComponentSlotStylesInput, ICSSInJSStyle } from '../../../types' import { DividerPropsWithDefaults } from '../../../../components/Divider/Divider' +import { DividerVariables } from './dividerVariables' -const dividerBorderStyle = (size, color): ICSSInJSStyle => ({ - height: `${size + 1}px`, - background: color, -}) - -const beforeAndAfter = (color, size, type, variables): ICSSPseudoElementStyle => ({ +const beforeAndAfter = ( + color: string, + size: number, + variables: DividerVariables, +): ICSSInJSStyle => ({ content: '""', flex: 1, - ...dividerBorderStyle(size, variables.dividerColor), - ...(color && { - ...dividerBorderStyle(size, _.get(variables.colors, color)), - }), + height: `${size + 1}px`, + background: _.get(variables.colors, color, variables.dividerColor), }) -const dividerStyles: ComponentSlotStylesInput = { +const dividerStyles: ComponentSlotStylesInput = { root: ({ props, variables }): ICSSInJSStyle => { - const { children, color, fitted, size, type, important, content } = props + const { children, color, fitted, size, important, content } = props return { - color: variables.textColor, + color: _.get(variables.colors, color, variables.textColor), display: 'flex', alignItems: 'center', ...(!fitted && { paddingTop: variables.dividerPadding, paddingBottom: variables.dividerPadding, }), - ...(color && { - color: _.get(variables.colors, color), - }), ...(important && { fontWeight: variables.importantFontWeight, }), @@ -42,17 +37,17 @@ const dividerStyles: ComponentSlotStylesInput = { fontSize: pxToRem(12 + size), lineHeight: variables.textLineHeight, '::before': { - ...beforeAndAfter(color, size, type, variables), + ...beforeAndAfter(color, size, variables), marginRight: pxToRem(20), }, '::after': { - ...beforeAndAfter(color, size, type, variables), + ...beforeAndAfter(color, size, variables), marginLeft: pxToRem(20), }, } : { '::before': { - ...beforeAndAfter(color, size, type, variables), + ...beforeAndAfter(color, size, variables), }, }), } diff --git a/src/themes/teams/components/Divider/dividerVariables.ts b/src/themes/teams/components/Divider/dividerVariables.ts index 915a1a6907..ad11315098 100644 --- a/src/themes/teams/components/Divider/dividerVariables.ts +++ b/src/themes/teams/components/Divider/dividerVariables.ts @@ -1,23 +1,25 @@ import * as _ from 'lodash' -import { pxToRem } from '../../utils' +import { FontWeightProperty } from 'csstype' -import { EmphasisColors, NaturalColors } from '../../../types' +import { pxToRem } from '../../utils' +import { ColorValues } from '../../../types' +import { mapColorsToScheme } from '../../../../lib' export interface DividerVariables { - colors: Record + colors: ColorValues dividerColor: string textColor: string textFontSize: string textLineHeight: string - importantFontWeight: string + importantFontWeight: FontWeightProperty dividerPadding: string } export default (siteVars: any): DividerVariables => { - const colorVariant = '500' + const colorVariant = 500 return { - colors: _.mapValues({ ...siteVars.emphasisColors, ...siteVars.naturalColors }, colorVariant), + colors: mapColorsToScheme(siteVars, colorVariant), dividerColor: siteVars.gray09, textColor: siteVars.gray03, textFontSize: siteVars.fontSizeSmall, diff --git a/src/themes/teams/components/Text/textStyles.ts b/src/themes/teams/components/Text/textStyles.ts index 24861465a2..4c1ac02d67 100644 --- a/src/themes/teams/components/Text/textStyles.ts +++ b/src/themes/teams/components/Text/textStyles.ts @@ -1,3 +1,5 @@ +import * as _ from 'lodash' + import { ComponentStyleFunctionParam, ICSSInJSStyle } from '../../../types' import { truncateStyle } from '../../../../styles/customCSS' import { TextVariables } from './textVariables' @@ -7,6 +9,7 @@ export default { root: ({ props: { atMention, + color, disabled, error, size, @@ -43,6 +46,7 @@ export default { fontWeight: v.importantWeight, color: v.importantColor, }), + ...(color && { color: _.get(v.colors, color) }), ...(weight === 'light' && { fontWeight: v.fontWeightLight, diff --git a/src/themes/teams/components/Text/textVariables.ts b/src/themes/teams/components/Text/textVariables.ts index 6330b4b256..75292294de 100644 --- a/src/themes/teams/components/Text/textVariables.ts +++ b/src/themes/teams/components/Text/textVariables.ts @@ -1,4 +1,8 @@ +import { ColorValues } from '../../../types' +import { mapColorsToScheme } from '../../../../lib' + export interface TextVariables { + colors: ColorValues atMentionMeColor: string atMentionMeFontWeight: number atMentionOtherColor: string @@ -29,7 +33,10 @@ export interface TextVariables { } export default (siteVariables): TextVariables => { + const colorVariant = 500 + return { + colors: mapColorsToScheme(siteVariables, colorVariant), atMentionOtherColor: siteVariables.brand06, atMentionMeColor: siteVariables.orange04, atMentionMeFontWeight: siteVariables.fontWeightBold, diff --git a/src/themes/types.ts b/src/themes/types.ts index 076b84951f..646d876224 100644 --- a/src/themes/types.ts +++ b/src/themes/types.ts @@ -71,6 +71,16 @@ type EmphasisColorsStrict = Partial<{ export type EmphasisColors = Extendable +/** + * A type for extracting the color names. + */ +type ColorNames = keyof (EmphasisColorsStrict & NaturalColorsStrict) + +/** + * A type for an extendable set of ColorNames properties of type T + */ +export type ColorValues = Extendable>, T> + /** * A type for a base colors. */ From fa78e86780e6c43cb6f478e4f6881a13960d59e0 Mon Sep 17 00:00:00 2001 From: Alexandru Buliga Date: Tue, 18 Dec 2018 18:03:41 +0200 Subject: [PATCH 48/55] feat(header): header and header description color prop (#628) * feat(header): header and header description color prop * changelog * fixed examples * addressed PR comments --- CHANGELOG.md | 1 + .../HeaderExampleColor.shorthand.tsx | 21 +++++++++++++++++++ .../Header/Variations/HeaderExampleColor.tsx | 20 ++++++++++++++++++ .../components/Header/Variations/index.tsx | 5 +++++ src/components/Header/Header.tsx | 6 ++++-- src/components/Header/HeaderDescription.tsx | 6 ++++-- .../Header/headerDescriptionStyles.ts | 14 +++++++++---- .../Header/headerDescriptionVariables.ts | 18 +++++++++++++--- .../teams/components/Header/headerStyles.ts | 19 +++++++++++------ .../components/Header/headerVariables.ts | 6 ++++++ 10 files changed, 99 insertions(+), 17 deletions(-) create mode 100644 docs/src/examples/components/Header/Variations/HeaderExampleColor.shorthand.tsx create mode 100644 docs/src/examples/components/Header/Variations/HeaderExampleColor.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index e2293b5161..721a2c2f48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Features - Add `color` prop to `Text` component @Bugaa92 ([#597](https://github.com/stardust-ui/react/pull/597)) +- Add `color` prop to `Header` and `HeaderDescription` components @Bugaa92 ([#628](https://github.com/stardust-ui/react/pull/628)) ## [v0.15.0](https://github.com/stardust-ui/react/tree/v0.15.0) (2018-12-17) diff --git a/docs/src/examples/components/Header/Variations/HeaderExampleColor.shorthand.tsx b/docs/src/examples/components/Header/Variations/HeaderExampleColor.shorthand.tsx new file mode 100644 index 0000000000..0ddbdd56f3 --- /dev/null +++ b/docs/src/examples/components/Header/Variations/HeaderExampleColor.shorthand.tsx @@ -0,0 +1,21 @@ +import React from 'react' +import _ from 'lodash' +import { Header, ProviderConsumer } from '@stardust-ui/react' + +const HeaderExampleColor = () => ( + + _.keys({ ...emphasisColors, ...naturalColors }).map(color => ( +
+ )) + } + /> +) + +export default HeaderExampleColor diff --git a/docs/src/examples/components/Header/Variations/HeaderExampleColor.tsx b/docs/src/examples/components/Header/Variations/HeaderExampleColor.tsx new file mode 100644 index 0000000000..9bd112cd00 --- /dev/null +++ b/docs/src/examples/components/Header/Variations/HeaderExampleColor.tsx @@ -0,0 +1,20 @@ +import React from 'react' +import _ from 'lodash' +import { Header, ProviderConsumer } from '@stardust-ui/react' + +const HeaderExampleColor = () => ( + + _.keys({ ...emphasisColors, ...naturalColors }).map(color => ( +
+ {_.startCase(color)} + + {`Description of ${_.lowerCase(color)} color`} + +
+ )) + } + /> +) + +export default HeaderExampleColor diff --git a/docs/src/examples/components/Header/Variations/index.tsx b/docs/src/examples/components/Header/Variations/index.tsx index ca7a86be9c..346dfbbfc2 100644 --- a/docs/src/examples/components/Header/Variations/index.tsx +++ b/docs/src/examples/components/Header/Variations/index.tsx @@ -19,6 +19,11 @@ const Variations = () => ( description="Headers may be aligned to the left, right, center or be justified." examplePath="components/Header/Variations/HeaderExampleTextAlign" /> + ) diff --git a/src/components/Header/Header.tsx b/src/components/Header/Header.tsx index d9ade9af52..a563d68fca 100644 --- a/src/components/Header/Header.tsx +++ b/src/components/Header/Header.tsx @@ -9,6 +9,7 @@ import { ChildrenComponentProps, ContentComponentProps, commonPropTypes, + ColorComponentProps, } from '../../lib' import HeaderDescription from './HeaderDescription' import { Extendable, ShorthandValue } from '../../../types/utils' @@ -16,7 +17,8 @@ import { Extendable, ShorthandValue } from '../../../types/utils' export interface HeaderProps extends UIComponentProps, ChildrenComponentProps, - ContentComponentProps { + ContentComponentProps, + ColorComponentProps { /** Shorthand for Header.Description. */ description?: ShorthandValue @@ -40,7 +42,7 @@ class Header extends UIComponent, any> { static displayName = 'Header' static propTypes = { - ...commonPropTypes.createCommon(), + ...commonPropTypes.createCommon({ color: true }), description: customPropTypes.itemShorthand, textAlign: PropTypes.oneOf(['left', 'center', 'right', 'justified']), } diff --git a/src/components/Header/HeaderDescription.tsx b/src/components/Header/HeaderDescription.tsx index 1943948043..adc25a6209 100644 --- a/src/components/Header/HeaderDescription.tsx +++ b/src/components/Header/HeaderDescription.tsx @@ -8,13 +8,15 @@ import { ChildrenComponentProps, ContentComponentProps, commonPropTypes, + ColorComponentProps, } from '../../lib' import { Extendable } from '../../../types/utils' export interface HeaderDescriptionProps extends UIComponentProps, ChildrenComponentProps, - ContentComponentProps {} + ContentComponentProps, + ColorComponentProps {} /** * A header's description provides more detailed information. @@ -27,7 +29,7 @@ class HeaderDescription extends UIComponent, static displayName = 'HeaderDescription' static propTypes = { - ...commonPropTypes.createCommon(), + ...commonPropTypes.createCommon({ color: true }), } static defaultProps = { diff --git a/src/themes/teams/components/Header/headerDescriptionStyles.ts b/src/themes/teams/components/Header/headerDescriptionStyles.ts index b49c56d101..9efcdd2d67 100644 --- a/src/themes/teams/components/Header/headerDescriptionStyles.ts +++ b/src/themes/teams/components/Header/headerDescriptionStyles.ts @@ -1,11 +1,17 @@ +import * as _ from 'lodash' + import { pxToRem } from '../../utils' -import { ICSSInJSStyle } from '../../../types' +import { ICSSInJSStyle, ComponentSlotStylesInput } from '../../../types' +import { HeaderDescriptionProps } from '../../../../components/Header/HeaderDescription' +import { HeaderDescriptionVariables } from './headerDescriptionVariables' -export default { - root: ({ variables: v }): ICSSInJSStyle => ({ +const headerStyles: ComponentSlotStylesInput = { + root: ({ props: p, variables: v }): ICSSInJSStyle => ({ display: 'block', + color: _.get(v.colors, p.color, v.color), fontSize: pxToRem(22), - color: v.color, fontWeight: 400, }), } + +export default headerStyles diff --git a/src/themes/teams/components/Header/headerDescriptionVariables.ts b/src/themes/teams/components/Header/headerDescriptionVariables.ts index 3de2fffb02..c41a19513c 100644 --- a/src/themes/teams/components/Header/headerDescriptionVariables.ts +++ b/src/themes/teams/components/Header/headerDescriptionVariables.ts @@ -1,3 +1,15 @@ -export default siteVariables => ({ - color: siteVariables.gray04, -}) +import { ColorValues } from '../../../types' +import { mapColorsToScheme } from '../../../../lib' + +export interface HeaderDescriptionVariables { + colors: ColorValues + color: string +} + +export default (siteVariables: any): HeaderDescriptionVariables => { + const colorVariant = 500 + return { + colors: mapColorsToScheme(siteVariables, colorVariant), + color: siteVariables.gray04, + } +} diff --git a/src/themes/teams/components/Header/headerStyles.ts b/src/themes/teams/components/Header/headerStyles.ts index 668937a72d..ecf3789006 100644 --- a/src/themes/teams/components/Header/headerStyles.ts +++ b/src/themes/teams/components/Header/headerStyles.ts @@ -1,10 +1,17 @@ -import { ICSSInJSStyle } from '../../../types' +import * as _ from 'lodash' +import { TextAlignProperty } from 'csstype' -export default { - root: ({ props, variables: v }): ICSSInJSStyle => ({ - color: v.color, - textAlign: props.textAlign, +import { ICSSInJSStyle, ComponentSlotStylesInput } from '../../../types' +import { HeaderProps } from '../../../../components/Header/Header' +import { HeaderVariables } from './headerVariables' + +const headerStyles: ComponentSlotStylesInput = { + root: ({ props: p, variables: v }): ICSSInJSStyle => ({ display: 'block', - ...(props.description && { marginBottom: 0 }), + color: _.get(v.colors, p.color, v.color), + textAlign: p.textAlign as TextAlignProperty, + ...(p.description && { marginBottom: 0 }), }), } + +export default headerStyles diff --git a/src/themes/teams/components/Header/headerVariables.ts b/src/themes/teams/components/Header/headerVariables.ts index 2147b544b9..7891afe646 100644 --- a/src/themes/teams/components/Header/headerVariables.ts +++ b/src/themes/teams/components/Header/headerVariables.ts @@ -1,10 +1,16 @@ +import { ColorValues } from '../../../types' +import { mapColorsToScheme } from '../../../../lib' + export interface HeaderVariables { + colors: ColorValues color: string descriptionColor: string } export default (siteVars: any): HeaderVariables => { + const colorVariant = 500 return { + colors: mapColorsToScheme(siteVars, colorVariant), color: siteVars.black, descriptionColor: undefined, } From 489530107bd22eb4484e4d0a7296615f62c6c21b Mon Sep 17 00:00:00 2001 From: kuzhelov Date: Tue, 18 Dec 2018 20:11:51 +0100 Subject: [PATCH 49/55] fix(Popup): allow to 'detach' from trigger and RTL adjustments (#612) * introduce offset prop * correct description of supported values * update changelog * introduce fix * ensure RTL is properly applied to complex offset expressions * rename method to make logic more expressive * add unit tests * remove unnecessary grid props from offset example * update changelog --- CHANGELOG.md | 3 ++ .../PopupExampleOffset.shorthand.tsx | 45 +++++++---------- src/components/Popup/Popup.tsx | 19 +++---- src/components/Popup/positioningHelper.ts | 29 ++++++++++- test/specs/components/Popup/Popup-test.tsx | 50 ++++++++++++++++++- 5 files changed, 103 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 721a2c2f48..af197727ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Fixes +- Ensure `Popup` properly flips values of `offset` prop in RTL @kuzhelov ([#612](https://github.com/stardust-ui/react/pull/612)) + ### Features - Add `color` prop to `Text` component @Bugaa92 ([#597](https://github.com/stardust-ui/react/pull/597)) - Add `color` prop to `Header` and `HeaderDescription` components @Bugaa92 ([#628](https://github.com/stardust-ui/react/pull/628)) diff --git a/docs/src/examples/components/Popup/Variations/PopupExampleOffset.shorthand.tsx b/docs/src/examples/components/Popup/Variations/PopupExampleOffset.shorthand.tsx index 8376ee024c..d0901e6e01 100644 --- a/docs/src/examples/components/Popup/Variations/PopupExampleOffset.shorthand.tsx +++ b/docs/src/examples/components/Popup/Variations/PopupExampleOffset.shorthand.tsx @@ -1,5 +1,5 @@ import React from 'react' -import { Button, Grid, Popup, Alignment, Position } from '@stardust-ui/react' +import { Button, Grid, Popup } from '@stardust-ui/react' const renderButton = rotateArrowUp => (