From 2b3138a61567822cf420df17eb38fcfeef8ca79e Mon Sep 17 00:00:00 2001 From: Sofiya Huts Date: Thu, 28 Mar 2019 16:40:25 +0100 Subject: [PATCH 01/13] feat(Menu): Make Submenu open state controlled --- ...ExampleWithSubmenuControlled.shorthand.tsx | 50 +++++++++++++++++++ .../examples/components/Menu/Usages/index.tsx | 5 ++ .../react/src/components/Menu/MenuItem.tsx | 31 +++++++++--- .../react/src/lib/AutoControlledComponent.tsx | 5 +- 4 files changed, 82 insertions(+), 9 deletions(-) create mode 100644 docs/src/examples/components/Menu/Usages/MenuExampleWithSubmenuControlled.shorthand.tsx diff --git a/docs/src/examples/components/Menu/Usages/MenuExampleWithSubmenuControlled.shorthand.tsx b/docs/src/examples/components/Menu/Usages/MenuExampleWithSubmenuControlled.shorthand.tsx new file mode 100644 index 0000000000..cd54fbe979 --- /dev/null +++ b/docs/src/examples/components/Menu/Usages/MenuExampleWithSubmenuControlled.shorthand.tsx @@ -0,0 +1,50 @@ +import * as React from 'react' +import { Menu } from '@stardust-ui/react' + +class MenuWithSubmenuControlledExample extends React.Component { + state = { editorialsMenuOpen: false } + + handleMenuOpenChange = (e, { menuOpen }) => { + this.setState({ editorialsMenuOpen: menuOpen }) + } + + renderItems = () => { + return [ + { + key: 'editorials', + content: 'Editorials', + menuOpen: this.state.editorialsMenuOpen, + onMenuOpenChange: this.handleMenuOpenChange, + menu: { + items: [ + { key: '1', content: 'item1' }, + { + key: '2', + content: 'item2', + menu: [{ key: '1', content: 'item2.1' }, { key: '2', content: 'item2.2' }], + }, + { + key: '3', + content: 'item3', + menu: [{ key: '1', content: 'item3.1' }, { key: '2', content: 'item3.2' }], + }, + ], + }, + }, + { key: 'events', content: 'Upcoming Events' }, + ] + } + + render() { + return ( + <> + {`Editorials menu item requested to change its submenu open state to "${ + this.state.editorialsMenuOpen + }".`} + + + ) + } +} + +export default MenuWithSubmenuControlledExample diff --git a/docs/src/examples/components/Menu/Usages/index.tsx b/docs/src/examples/components/Menu/Usages/index.tsx index 6dbf167317..0d31490c6d 100644 --- a/docs/src/examples/components/Menu/Usages/index.tsx +++ b/docs/src/examples/components/Menu/Usages/index.tsx @@ -19,6 +19,11 @@ const Usages = () => ( description="A menu can have submenus." examplePath="components/Menu/Usages/MenuExampleWithSubmenu" /> + ) diff --git a/packages/react/src/components/Menu/MenuItem.tsx b/packages/react/src/components/Menu/MenuItem.tsx index e253997199..fd27747be0 100644 --- a/packages/react/src/components/Menu/MenuItem.tsx +++ b/packages/react/src/components/Menu/MenuItem.tsx @@ -122,6 +122,13 @@ export interface MenuItemProps /** Shorthand for the submenu indicator. */ indicator?: ShorthandValue + + /** + * Event for request to change 'open' value. + * @param {SyntheticEvent} event - React's original SyntheticEvent. + * @param {object} data - All props and proposed value. + */ + onMenuOpenChange?: ComponentEventHandler } export interface MenuItemState { @@ -165,6 +172,7 @@ class MenuItem extends AutoControlledComponent, MenuIt onActiveChanged: PropTypes.func, inSubmenu: PropTypes.bool, indicator: customPropTypes.itemShorthand, + onMenuOpenChange: PropTypes.func, } static defaultProps = { @@ -270,7 +278,7 @@ class MenuItem extends AutoControlledComponent, MenuIt private handleWrapperBlur = e => { if (!this.props.inSubmenu && !e.currentTarget.contains(e.relatedTarget)) { - this.setState({ menuOpen: false }) + this.trySetMenuOpen(false, e) } } @@ -288,19 +296,20 @@ class MenuItem extends AutoControlledComponent, MenuIt !doesNodeContainClick(this.itemRef.current, e) && !doesNodeContainClick(this.menuRef.current, e) ) { - this.trySetState({ menuOpen: false }) + this.trySetMenuOpen(false, e) } } private performClick = e => { const { active, menu } = this.props + if (menu) { if (doesNodeContainClick(this.menuRef.current, e)) { // submenu was clicked => close it and propagate - this.setState({ menuOpen: false }, () => focusAsync(this.itemRef.current)) + this.trySetMenuOpen(false, e, () => focusAsync(this.itemRef.current)) } else { // the menuItem element was clicked => toggle the open/close and stop propagation - this.trySetState({ menuOpen: active ? !this.state.menuOpen : true }) + this.trySetMenuOpen(active ? !this.state.menuOpen : true, e) e.stopPropagation() e.preventDefault() } @@ -335,7 +344,7 @@ class MenuItem extends AutoControlledComponent, MenuIt const { menu, inSubmenu } = this.props const { menuOpen } = this.state if (menu && menuOpen) { - this.setState({ menuOpen: false }, () => { + this.trySetMenuOpen(false, e, () => { if (!inSubmenu && (!focusNextParent || this.props.vertical)) { focusAsync(this.itemRef.current) } @@ -348,7 +357,7 @@ class MenuItem extends AutoControlledComponent, MenuIt const { menuOpen } = this.state const shouldStopPropagation = inSubmenu || this.props.vertical if (menu && menuOpen) { - this.setState({ menuOpen: false }, () => { + this.trySetMenuOpen(false, e, () => { if (shouldStopPropagation) { focusAsync(this.itemRef.current) } @@ -363,12 +372,20 @@ class MenuItem extends AutoControlledComponent, MenuIt const { menu } = this.props const { menuOpen } = this.state if (menu && !menuOpen) { - this.setState({ menuOpen: true }) + this.trySetMenuOpen(true, e) _.invoke(this.props, 'onActiveChanged', e, { ...this.props, active: true }) e.stopPropagation() e.preventDefault() } } + + private trySetMenuOpen(newValue: boolean, eventArgs: any, onStateChanged?: any) { + this.trySetState({ menuOpen: newValue }, null, onStateChanged) + _.invoke(this.props, 'onMenuOpenChange', eventArgs, { + ...this.props, + ...{ menuOpen: newValue }, + }) + } } MenuItem.create = createShorthandFactory({ Component: MenuItem, mappedProp: 'content' }) diff --git a/packages/react/src/lib/AutoControlledComponent.tsx b/packages/react/src/lib/AutoControlledComponent.tsx index bca09ea0d8..22f86a9da1 100644 --- a/packages/react/src/lib/AutoControlledComponent.tsx +++ b/packages/react/src/lib/AutoControlledComponent.tsx @@ -187,8 +187,9 @@ export default class AutoControlledComponent

extends UIComponent * Second argument is a state object that is always passed to setState. * @param {object} maybeState State that corresponds to controlled props. * @param {object} [state] Actual state, useful when you also need to setState. + * @param {object} onStateChanged Callback which is called after setState applied. */ - trySetState = (maybeState, state?) => { + trySetState = (maybeState, state?, onStateChanged?: () => void) => { const { autoControlledProps } = this.constructor as any if (process.env.NODE_ENV !== 'production') { const { name } = this.constructor @@ -218,6 +219,6 @@ export default class AutoControlledComponent

extends UIComponent if (state) newState = { ...newState, ...state } - if (Object.keys(newState).length > 0) this.setState(newState) + if (Object.keys(newState).length > 0) this.setState(newState, onStateChanged) } } From 0dab9833983028405b6e29fdcd169f8d58093c03 Mon Sep 17 00:00:00 2001 From: Sofiya Huts Date: Fri, 29 Mar 2019 14:57:53 +0100 Subject: [PATCH 02/13] Improvements after CR and discussions --- ...nd.steps.ts => MenuExampleSlot.shorthand.steps.ts} | 0 ...le.shorthand.tsx => MenuExampleSlot.shorthand.tsx} | 0 docs/src/examples/components/Menu/Slots/index.tsx | 2 +- packages/react/src/components/Menu/Menu.tsx | 3 ++- packages/react/src/components/Menu/MenuItem.tsx | 11 ++++++++--- packages/react/src/components/Ref/Ref.tsx | 4 +++- packages/react/src/components/Ref/RefFindNode.tsx | 4 +++- packages/react/src/components/Ref/RefForward.tsx | 4 +++- 8 files changed, 20 insertions(+), 8 deletions(-) rename docs/src/examples/components/Menu/Slots/{MenuExample.shorthand.steps.ts => MenuExampleSlot.shorthand.steps.ts} (100%) rename docs/src/examples/components/Menu/Slots/{MenuExample.shorthand.tsx => MenuExampleSlot.shorthand.tsx} (100%) diff --git a/docs/src/examples/components/Menu/Slots/MenuExample.shorthand.steps.ts b/docs/src/examples/components/Menu/Slots/MenuExampleSlot.shorthand.steps.ts similarity index 100% rename from docs/src/examples/components/Menu/Slots/MenuExample.shorthand.steps.ts rename to docs/src/examples/components/Menu/Slots/MenuExampleSlot.shorthand.steps.ts diff --git a/docs/src/examples/components/Menu/Slots/MenuExample.shorthand.tsx b/docs/src/examples/components/Menu/Slots/MenuExampleSlot.shorthand.tsx similarity index 100% rename from docs/src/examples/components/Menu/Slots/MenuExample.shorthand.tsx rename to docs/src/examples/components/Menu/Slots/MenuExampleSlot.shorthand.tsx diff --git a/docs/src/examples/components/Menu/Slots/index.tsx b/docs/src/examples/components/Menu/Slots/index.tsx index 84a50c785f..60db64fc89 100644 --- a/docs/src/examples/components/Menu/Slots/index.tsx +++ b/docs/src/examples/components/Menu/Slots/index.tsx @@ -7,7 +7,7 @@ const Slots = () => ( , MenuIt private itemRef = React.createRef() renderComponent({ ElementType, classes, accessibility, unhandledProps, styles }) { + console.log('[MenuItem] renderComponent') const { children, content, @@ -277,6 +279,7 @@ class MenuItem extends AutoControlledComponent, MenuIt } private handleWrapperBlur = e => { + console.log('[MenuItem] handleWrapperBlur') if (!this.props.inSubmenu && !e.currentTarget.contains(e.relatedTarget)) { this.trySetMenuOpen(false, e) } @@ -291,6 +294,7 @@ class MenuItem extends AutoControlledComponent, MenuIt } private outsideClickHandler = e => { + console.log('[MenuItem] outsideClickHandler') if (!this.state.menuOpen) return if ( !doesNodeContainClick(this.itemRef.current, e) && @@ -380,10 +384,11 @@ class MenuItem extends AutoControlledComponent, MenuIt } private trySetMenuOpen(newValue: boolean, eventArgs: any, onStateChanged?: any) { - this.trySetState({ menuOpen: newValue }, null, onStateChanged) + this.trySetState({ menuOpen: newValue }) + onStateChanged && onStateChanged() _.invoke(this.props, 'onMenuOpenChange', eventArgs, { ...this.props, - ...{ menuOpen: newValue }, + menuOpen: newValue, }) } } diff --git a/packages/react/src/components/Ref/Ref.tsx b/packages/react/src/components/Ref/Ref.tsx index c23b8354cc..3c629276a4 100644 --- a/packages/react/src/components/Ref/Ref.tsx +++ b/packages/react/src/components/Ref/Ref.tsx @@ -1,8 +1,10 @@ +import * as customPropTypes from '@stardust-ui/react-proptypes' + import * as PropTypes from 'prop-types' import * as React from 'react' import { isForwardRef } from 'react-is' -import { ChildrenComponentProps, customPropTypes } from '../../lib' +import { ChildrenComponentProps } from '../../lib' import { ReactProps } from '../../types' import RefFindNode from './RefFindNode' import RefForward from './RefForward' diff --git a/packages/react/src/components/Ref/RefFindNode.tsx b/packages/react/src/components/Ref/RefFindNode.tsx index f10506c61c..8541fc94b4 100644 --- a/packages/react/src/components/Ref/RefFindNode.tsx +++ b/packages/react/src/components/Ref/RefFindNode.tsx @@ -1,8 +1,10 @@ +import * as customPropTypes from '@stardust-ui/react-proptypes' + import * as PropTypes from 'prop-types' import * as React from 'react' import * as ReactDOM from 'react-dom' -import { ChildrenComponentProps, customPropTypes, handleRef } from '../../lib' +import { ChildrenComponentProps, handleRef } from '../../lib' export interface RefFindNodeProps extends ChildrenComponentProps> { /** diff --git a/packages/react/src/components/Ref/RefForward.tsx b/packages/react/src/components/Ref/RefForward.tsx index 2cf21d6258..595875d989 100644 --- a/packages/react/src/components/Ref/RefForward.tsx +++ b/packages/react/src/components/Ref/RefForward.tsx @@ -1,7 +1,9 @@ +import * as customPropTypes from '@stardust-ui/react-proptypes' + import * as PropTypes from 'prop-types' import * as React from 'react' -import { ChildrenComponentProps, customPropTypes, handleRef } from '../../lib' +import { ChildrenComponentProps, handleRef } from '../../lib' export interface RefForwardProps extends ChildrenComponentProps> { /** From fd49061ecf2bc142cf6098a06b4c007d03a4aca3 Mon Sep 17 00:00:00 2001 From: Sofiya Huts Date: Fri, 29 Mar 2019 15:08:29 +0100 Subject: [PATCH 03/13] Update autocontrolled component --- packages/react/src/lib/AutoControlledComponent.tsx | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/react/src/lib/AutoControlledComponent.tsx b/packages/react/src/lib/AutoControlledComponent.tsx index 22f86a9da1..1ee7c5a7f2 100644 --- a/packages/react/src/lib/AutoControlledComponent.tsx +++ b/packages/react/src/lib/AutoControlledComponent.tsx @@ -187,14 +187,14 @@ export default class AutoControlledComponent

extends UIComponent * Second argument is a state object that is always passed to setState. * @param {object} maybeState State that corresponds to controlled props. * @param {object} [state] Actual state, useful when you also need to setState. - * @param {object} onStateChanged Callback which is called after setState applied. + * @param {object} callback Callback which is called after setState applied. */ - trySetState = (maybeState, state?, onStateChanged?: () => void) => { + trySetState = (state: Partial, callback?: () => void) => { const { autoControlledProps } = this.constructor as any if (process.env.NODE_ENV !== 'production') { const { name } = this.constructor // warn about failed attempts to setState for keys not listed in autoControlledProps - const illegalKeys = _.difference(_.keys(maybeState), autoControlledProps) + const illegalKeys = _.difference(_.keys(state), autoControlledProps) if (!_.isEmpty(illegalKeys)) { console.error( [ @@ -206,19 +206,17 @@ export default class AutoControlledComponent

extends UIComponent } } - let newState = Object.keys(maybeState).reduce((acc, prop) => { + const newState = Object.keys(state).reduce((acc, prop) => { // ignore props defined by the parent if (this.props[prop] !== undefined) return acc // ignore props not listed in auto controlled props if (autoControlledProps.indexOf(prop) === -1) return acc - acc[prop] = maybeState[prop] + acc[prop] = state[prop] return acc }, {}) - if (state) newState = { ...newState, ...state } - - if (Object.keys(newState).length > 0) this.setState(newState, onStateChanged) + if (Object.keys(newState).length > 0) this.setState(newState, callback) } } From f257d26af619b454912cd12ef508bea630d934c6 Mon Sep 17 00:00:00 2001 From: Sofiya Huts Date: Fri, 29 Mar 2019 15:12:07 +0100 Subject: [PATCH 04/13] Improvements --- packages/react/src/components/Menu/MenuItem.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/react/src/components/Menu/MenuItem.tsx b/packages/react/src/components/Menu/MenuItem.tsx index d87b6d450b..e26471ca8d 100644 --- a/packages/react/src/components/Menu/MenuItem.tsx +++ b/packages/react/src/components/Menu/MenuItem.tsx @@ -385,6 +385,10 @@ class MenuItem extends AutoControlledComponent, MenuIt private trySetMenuOpen(newValue: boolean, eventArgs: any, onStateChanged?: any) { this.trySetState({ menuOpen: newValue }) + // The reason why post-effect is not passed as callback to trySetState method + // is that in 'controlled' mode the post-effect is applied before final re-rendering + // which cause a broken behavior: for e.g. when it is needed to focus submenu trigger on ESC. + // TODO: all DOM post-effects should be applied at componentDidMount & componentDidUpdated stages. onStateChanged && onStateChanged() _.invoke(this.props, 'onMenuOpenChange', eventArgs, { ...this.props, From 47e9223c1bb351e484eb7b02add3fc54ca364ccf Mon Sep 17 00:00:00 2001 From: Sofiya Huts <8460706+sophieH29@users.noreply.github.com> Date: Fri, 29 Mar 2019 15:13:43 +0100 Subject: [PATCH 05/13] Update Menu.tsx --- packages/react/src/components/Menu/Menu.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react/src/components/Menu/Menu.tsx b/packages/react/src/components/Menu/Menu.tsx index 67ce24aa04..59baf024df 100644 --- a/packages/react/src/components/Menu/Menu.tsx +++ b/packages/react/src/components/Menu/Menu.tsx @@ -1,5 +1,4 @@ import * as customPropTypes from '@stardust-ui/react-proptypes' - import * as _ from 'lodash' import * as PropTypes from 'prop-types' import * as React from 'react' From c7e52d7662c4a0f5478c4b1d60565dffba56a058 Mon Sep 17 00:00:00 2001 From: Sofiya Huts Date: Fri, 29 Mar 2019 16:40:02 +0100 Subject: [PATCH 06/13] on esc close one menu --- packages/react/src/components/Menu/MenuItem.tsx | 10 +++------- .../accessibility/Behaviors/Menu/menuItemBehavior.ts | 6 +++--- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/packages/react/src/components/Menu/MenuItem.tsx b/packages/react/src/components/Menu/MenuItem.tsx index b776da2ad5..d851f9fea7 100644 --- a/packages/react/src/components/Menu/MenuItem.tsx +++ b/packages/react/src/components/Menu/MenuItem.tsx @@ -1,7 +1,5 @@ import * as customPropTypes from '@stardust-ui/react-proptypes' - import { documentRef, EventListener } from '@stardust-ui/react-component-event-listener' -import * as customPropTypes from '@stardust-ui/react-proptypes' import * as _ from 'lodash' import cx from 'classnames' import * as PropTypes from 'prop-types' @@ -188,7 +186,6 @@ class MenuItem extends AutoControlledComponent, MenuIt private itemRef = React.createRef() renderComponent({ ElementType, classes, accessibility, unhandledProps, styles }) { - console.log('[MenuItem] renderComponent') const { children, content, @@ -279,7 +276,6 @@ class MenuItem extends AutoControlledComponent, MenuIt } private handleWrapperBlur = e => { - console.log('[MenuItem] handleWrapperBlur') if (!this.props.inSubmenu && !e.currentTarget.contains(e.relatedTarget)) { this.trySetMenuOpen(false, e) } @@ -291,10 +287,10 @@ class MenuItem extends AutoControlledComponent, MenuIt closeAllMenus: event => this.closeAllMenus(event, false), closeAllMenusAndFocusNextParentItem: event => this.closeAllMenus(event, true), closeMenu: event => this.closeMenu(event), + closeMenuAndFocusTrigger: event => this.closeMenu(event, true), } private outsideClickHandler = e => { - console.log('[MenuItem] outsideClickHandler') if (!this.state.menuOpen) return if ( !doesNodeContainClick(this.itemRef.current, e) && @@ -356,13 +352,13 @@ class MenuItem extends AutoControlledComponent, MenuIt } } - private closeMenu = e => { + private closeMenu = (e, forceTriggerFocus?: boolean) => { const { menu, inSubmenu } = this.props const { menuOpen } = this.state const shouldStopPropagation = inSubmenu || this.props.vertical if (menu && menuOpen) { this.trySetMenuOpen(false, e, () => { - if (shouldStopPropagation) { + if (forceTriggerFocus || shouldStopPropagation) { focusAsync(this.itemRef.current) } }) diff --git a/packages/react/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts b/packages/react/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts index 0662de4773..8ca08306aa 100644 --- a/packages/react/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts +++ b/packages/react/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts @@ -55,12 +55,12 @@ const menuItemBehavior: Accessibility = (props: any) => ({ performClick: { keyCombinations: [{ keyCode: keyboardKey.Enter }, { keyCode: keyboardKey.Spacebar }], }, - closeAllMenus: { - keyCombinations: [{ keyCode: keyboardKey.Escape }], - }, closeAllMenusAndFocusNextParentItem: { keyCombinations: [{ keyCode: keyboardKey.ArrowRight }], }, + closeMenuAndFocusTrigger: { + keyCombinations: [{ keyCode: keyboardKey.Escape }], + }, closeMenu: { keyCombinations: [{ keyCode: keyboardKey.ArrowLeft }], }, From 452c04f21f0bc78e5ade2a3d112a1f0330f2ad35 Mon Sep 17 00:00:00 2001 From: Sofiya Huts Date: Fri, 29 Mar 2019 16:42:21 +0100 Subject: [PATCH 07/13] small improvement --- packages/react/src/components/Menu/MenuItem.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/components/Menu/MenuItem.tsx b/packages/react/src/components/Menu/MenuItem.tsx index d851f9fea7..774fe45fad 100644 --- a/packages/react/src/components/Menu/MenuItem.tsx +++ b/packages/react/src/components/Menu/MenuItem.tsx @@ -1,5 +1,5 @@ -import * as customPropTypes from '@stardust-ui/react-proptypes' import { documentRef, EventListener } from '@stardust-ui/react-component-event-listener' +import * as customPropTypes from '@stardust-ui/react-proptypes' import * as _ from 'lodash' import cx from 'classnames' import * as PropTypes from 'prop-types' From 4a2d1876d067bde569828667868c26ccde6019c3 Mon Sep 17 00:00:00 2001 From: Sofiya Huts Date: Fri, 29 Mar 2019 16:43:44 +0100 Subject: [PATCH 08/13] small improvement --- packages/react/src/components/Accordion/Accordion.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/components/Accordion/Accordion.tsx b/packages/react/src/components/Accordion/Accordion.tsx index 739aacd0e7..bb65fc2589 100644 --- a/packages/react/src/components/Accordion/Accordion.tsx +++ b/packages/react/src/components/Accordion/Accordion.tsx @@ -146,7 +146,7 @@ class Accordion extends AutoControlledComponent, any> const { index } = titleProps const activeIndex = this.computeNewIndex(index) - this.trySetState({ activeIndex }, index) + this.trySetState({ activeIndex }) _.invoke(predefinedProps, 'onClick', e, titleProps) _.invoke(this.props, 'onTitleClick', e, titleProps) From 3ba0657255a17b02b64e6da72f55ba108db7fd16 Mon Sep 17 00:00:00 2001 From: Sofiya Huts Date: Fri, 29 Mar 2019 17:58:15 +0100 Subject: [PATCH 09/13] improvements after CR --- ...ExampleWithSubmenuControlled.shorthand.tsx | 56 ++++++++----------- .../Popup/Types/PopupExample.shorthand.tsx | 17 +++++- .../react/src/lib/AutoControlledComponent.tsx | 25 +-------- 3 files changed, 40 insertions(+), 58 deletions(-) diff --git a/docs/src/examples/components/Menu/Usages/MenuExampleWithSubmenuControlled.shorthand.tsx b/docs/src/examples/components/Menu/Usages/MenuExampleWithSubmenuControlled.shorthand.tsx index cd54fbe979..8749cbbd12 100644 --- a/docs/src/examples/components/Menu/Usages/MenuExampleWithSubmenuControlled.shorthand.tsx +++ b/docs/src/examples/components/Menu/Usages/MenuExampleWithSubmenuControlled.shorthand.tsx @@ -1,47 +1,35 @@ import * as React from 'react' -import { Menu } from '@stardust-ui/react' +import { Menu, Header } from '@stardust-ui/react' class MenuWithSubmenuControlledExample extends React.Component { - state = { editorialsMenuOpen: false } + state = { menuOpen: false } handleMenuOpenChange = (e, { menuOpen }) => { - this.setState({ editorialsMenuOpen: menuOpen }) - } - - renderItems = () => { - return [ - { - key: 'editorials', - content: 'Editorials', - menuOpen: this.state.editorialsMenuOpen, - onMenuOpenChange: this.handleMenuOpenChange, - menu: { - items: [ - { key: '1', content: 'item1' }, - { - key: '2', - content: 'item2', - menu: [{ key: '1', content: 'item2.1' }, { key: '2', content: 'item2.2' }], - }, - { - key: '3', - content: 'item3', - menu: [{ key: '1', content: 'item3.1' }, { key: '2', content: 'item3.2' }], - }, - ], - }, - }, - { key: 'events', content: 'Upcoming Events' }, - ] + this.setState({ menuOpen }) } render() { return ( <> - {`Editorials menu item requested to change its submenu open state to "${ - this.state.editorialsMenuOpen - }".`} -

+
+ 
{JSON.stringify(this.state, null, 2)}
+ ) } diff --git a/docs/src/examples/components/Popup/Types/PopupExample.shorthand.tsx b/docs/src/examples/components/Popup/Types/PopupExample.shorthand.tsx index 67b5e8bf9f..904c91c9b2 100644 --- a/docs/src/examples/components/Popup/Types/PopupExample.shorthand.tsx +++ b/docs/src/examples/components/Popup/Types/PopupExample.shorthand.tsx @@ -1,6 +1,19 @@ import * as React from 'react' -import { Button, Popup } from '@stardust-ui/react' +import { Button, Popup, Dialog, popupFocusTrapBehavior } from '@stardust-ui/react' -const PopupExample = () => } content="Hello from popup!" /> +const PopupExample = () => ( + } + accessibility={popupFocusTrapBehavior} + content={ + } + /> + } + /> +) export default PopupExample diff --git a/packages/react/src/lib/AutoControlledComponent.tsx b/packages/react/src/lib/AutoControlledComponent.tsx index 1ee7c5a7f2..3b4fa3dfbe 100644 --- a/packages/react/src/lib/AutoControlledComponent.tsx +++ b/packages/react/src/lib/AutoControlledComponent.tsx @@ -189,31 +189,12 @@ export default class AutoControlledComponent

extends UIComponent * @param {object} [state] Actual state, useful when you also need to setState. * @param {object} callback Callback which is called after setState applied. */ - trySetState = (state: Partial, callback?: () => void) => { - const { autoControlledProps } = this.constructor as any - if (process.env.NODE_ENV !== 'production') { - const { name } = this.constructor - // warn about failed attempts to setState for keys not listed in autoControlledProps - const illegalKeys = _.difference(_.keys(state), autoControlledProps) - if (!_.isEmpty(illegalKeys)) { - console.error( - [ - `${name} called trySetState() with controlled props: "${illegalKeys}".`, - 'State will not be set.', - 'Only props in static autoControlledProps will be set on state.', - ].join(' '), - ) - } - } - - const newState = Object.keys(state).reduce((acc, prop) => { + trySetState = (maybeState: Partial, callback?: () => void) => { + const newState = Object.keys(maybeState).reduce((acc, prop) => { // ignore props defined by the parent if (this.props[prop] !== undefined) return acc - // ignore props not listed in auto controlled props - if (autoControlledProps.indexOf(prop) === -1) return acc - - acc[prop] = state[prop] + acc[prop] = maybeState[prop] return acc }, {}) From 94db879e16070757b3ef3596b5de6491dbe3c2c0 Mon Sep 17 00:00:00 2001 From: Sofiya Huts Date: Fri, 29 Mar 2019 18:21:29 +0100 Subject: [PATCH 10/13] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ece85cdefe..497780299e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Features - Add predefined icon set for the usages in the `Input`, `Dropdown` and `AccordionTitle` components @mnajdova ([#1120](https://github.com/stardust-ui/react/pull/1120)) - Add `Popup` styles to Teams Dark and High Contrast themes @kuzhelov ([#1121](https://github.com/stardust-ui/react/pull/1121)) +- Make `MenuItem`'s submenu open state controlled @sophieH29 ([#1125](https://github.com/stardust-ui/react/pull/1125)) ### BREAKING CHANGES - `color` and `backgroundColor` variables were moved from `PopupContent` to `popup` slot of `Popup` component @kuzhelov ([#1121](https://github.com/stardust-ui/react/pull/1121)) From e92ad2566b72dc1ba3296ae0b26e93149f75e805 Mon Sep 17 00:00:00 2001 From: Sofiya Huts Date: Mon, 1 Apr 2019 10:07:01 +0200 Subject: [PATCH 11/13] small improvements --- packages/react/src/components/Menu/MenuItem.tsx | 1 - .../test/specs/lib/AutoControlledComponent-test.tsx | 11 ----------- 2 files changed, 12 deletions(-) diff --git a/packages/react/src/components/Menu/MenuItem.tsx b/packages/react/src/components/Menu/MenuItem.tsx index 774fe45fad..a76a87b705 100644 --- a/packages/react/src/components/Menu/MenuItem.tsx +++ b/packages/react/src/components/Menu/MenuItem.tsx @@ -284,7 +284,6 @@ class MenuItem extends AutoControlledComponent, MenuIt protected actionHandlers: AccessibilityActionHandlers = { performClick: event => this.handleClick(event), openMenu: event => this.openMenu(event), - closeAllMenus: event => this.closeAllMenus(event, false), closeAllMenusAndFocusNextParentItem: event => this.closeAllMenus(event, true), closeMenu: event => this.closeMenu(event), closeMenuAndFocusTrigger: event => this.closeMenu(event, true), diff --git a/packages/react/test/specs/lib/AutoControlledComponent-test.tsx b/packages/react/test/specs/lib/AutoControlledComponent-test.tsx index 2f8c33a900..8337316d98 100644 --- a/packages/react/test/specs/lib/AutoControlledComponent-test.tsx +++ b/packages/react/test/specs/lib/AutoControlledComponent-test.tsx @@ -65,17 +65,6 @@ describe('extending AutoControlledComponent', () => { expect(wrapper.state()).toHaveProperty(randomProp, randomValue) }) - test('does not set state for non autoControlledProps', () => { - consoleUtil.disableOnce() - - TestClass = createTestClass({ autoControlledProps: [], state: {} }) - const wrapper = shallow() - - getAutoControlledInstance(wrapper).trySetState({ ['system']: 'compress' }) - - expect(wrapper.state()).toEqual({}) - }) - test('does not set state for props defined by the parent', () => { consoleUtil.disableOnce() From 31945f6c1ef85c1e66b85c283a81398876a88093 Mon Sep 17 00:00:00 2001 From: Sofiya Huts Date: Mon, 1 Apr 2019 10:19:20 +0200 Subject: [PATCH 12/13] small improvements --- .../Popup/Types/PopupExample.shorthand.tsx | 17 ++--------------- packages/react/src/components/Menu/MenuItem.tsx | 4 ++-- 2 files changed, 4 insertions(+), 17 deletions(-) diff --git a/docs/src/examples/components/Popup/Types/PopupExample.shorthand.tsx b/docs/src/examples/components/Popup/Types/PopupExample.shorthand.tsx index 904c91c9b2..67b5e8bf9f 100644 --- a/docs/src/examples/components/Popup/Types/PopupExample.shorthand.tsx +++ b/docs/src/examples/components/Popup/Types/PopupExample.shorthand.tsx @@ -1,19 +1,6 @@ import * as React from 'react' -import { Button, Popup, Dialog, popupFocusTrapBehavior } from '@stardust-ui/react' +import { Button, Popup } from '@stardust-ui/react' -const PopupExample = () => ( - } - accessibility={popupFocusTrapBehavior} - content={ -

} - /> - } - /> -) +const PopupExample = () => } content="Hello from popup!" /> export default PopupExample diff --git a/packages/react/src/components/Menu/MenuItem.tsx b/packages/react/src/components/Menu/MenuItem.tsx index a76a87b705..f174b1e7b6 100644 --- a/packages/react/src/components/Menu/MenuItem.tsx +++ b/packages/react/src/components/Menu/MenuItem.tsx @@ -378,14 +378,14 @@ class MenuItem extends AutoControlledComponent, MenuIt } } - private trySetMenuOpen(newValue: boolean, eventArgs: any, onStateChanged?: any) { + private trySetMenuOpen(newValue: boolean, e: React.SyntheticEvent, onStateChanged?: any) { this.trySetState({ menuOpen: newValue }) // The reason why post-effect is not passed as callback to trySetState method // is that in 'controlled' mode the post-effect is applied before final re-rendering // which cause a broken behavior: for e.g. when it is needed to focus submenu trigger on ESC. // TODO: all DOM post-effects should be applied at componentDidMount & componentDidUpdated stages. onStateChanged && onStateChanged() - _.invoke(this.props, 'onMenuOpenChange', eventArgs, { + _.invoke(this.props, 'onMenuOpenChange', e, { ...this.props, menuOpen: newValue, }) From ac3c1d971e195f59a34cc5f5f54508ae246d2745 Mon Sep 17 00:00:00 2001 From: Sofiya Huts Date: Mon, 1 Apr 2019 10:56:22 +0200 Subject: [PATCH 13/13] fix test --- .../src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts b/packages/react/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts index 8ca08306aa..0ad110e6c2 100644 --- a/packages/react/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts +++ b/packages/react/src/lib/accessibility/Behaviors/Menu/menuItemBehavior.ts @@ -19,7 +19,7 @@ import * as _ from 'lodash' * Adds attribute 'aria-haspopup=true' to 'root' component's part if 'menu' property is set. * Adds attribute 'aria-disabled=true' to 'root' component's part based on the property 'disabled'. This can be overriden by providing 'aria-disabled' property directly to the component. * Triggers 'performClick' action with 'Enter' or 'Spacebar' on 'wrapper'. - * Triggers 'closeAllMenus' action with 'Escape' on 'wrapper'. + * Triggers 'closeMenuAndFocusTrigger' action with 'Escape' on 'wrapper'. * Triggers 'closeAllMenusAndFocusNextParentItem' action with 'ArrowRight' on 'wrapper'. * Triggers 'closeMenu' action with 'ArrowLeft' on 'wrapper'. * Triggers 'openMenu' action with 'ArrowDown' on 'wrapper', when orientation is horizontal.