From 6f35db047c6c99a7c08f26dcb4fe11bdd10fb8f8 Mon Sep 17 00:00:00 2001 From: Clint Andrew Hall Date: Mon, 27 Jul 2020 12:11:35 -0400 Subject: [PATCH] [Canvas][tech-debt] Refactor Toolbar (completes Kill Recompose.pure) --- x-pack/plugins/canvas/i18n/components.ts | 7 + .../canvas/public/components/navbar/navbar.js | 16 -- .../public/components/navbar/navbar.scss | 7 - .../__snapshots__/toolbar.stories.storyshot | 229 ++++++++++++++++++ .../toolbar/__examples__/toolbar.stories.tsx | 40 ++- .../canvas/public/components/toolbar/index.js | 49 ---- .../{navbar/index.js => toolbar/index.ts} | 6 +- .../{toolbar.tsx => toolbar.component.tsx} | 121 +++++---- .../public/components/toolbar/toolbar.scss | 8 + .../public/components/toolbar/toolbar.ts | 28 +++ .../public/components/toolbar/tray/index.ts | 5 +- .../public/components/toolbar/tray/tray.tsx | 11 +- x-pack/plugins/canvas/public/style/index.scss | 1 - .../storybook/decorators/router_decorator.tsx | 24 +- 14 files changed, 372 insertions(+), 180 deletions(-) delete mode 100644 x-pack/plugins/canvas/public/components/navbar/navbar.js delete mode 100644 x-pack/plugins/canvas/public/components/navbar/navbar.scss create mode 100644 x-pack/plugins/canvas/public/components/toolbar/__examples__/__snapshots__/toolbar.stories.storyshot delete mode 100644 x-pack/plugins/canvas/public/components/toolbar/index.js rename x-pack/plugins/canvas/public/components/{navbar/index.js => toolbar/index.ts} (66%) rename x-pack/plugins/canvas/public/components/toolbar/{toolbar.tsx => toolbar.component.tsx} (64%) create mode 100644 x-pack/plugins/canvas/public/components/toolbar/toolbar.ts diff --git a/x-pack/plugins/canvas/i18n/components.ts b/x-pack/plugins/canvas/i18n/components.ts index 9b1d60f38eb5e..03d6ade7bea69 100644 --- a/x-pack/plugins/canvas/i18n/components.ts +++ b/x-pack/plugins/canvas/i18n/components.ts @@ -913,6 +913,13 @@ export const ComponentStrings = { i18n.translate('xpack.canvas.toolbar.workpadManagerCloseButtonLabel', { defaultMessage: 'Close', }), + getErrorMessage: (message: string) => + i18n.translate('xpack.canvas.toolbar.errorMessage', { + defaultMessage: 'TOOLBAR ERROR: {message}', + values: { + message, + }, + }), }, ToolbarTray: { getCloseTrayAriaLabel: () => diff --git a/x-pack/plugins/canvas/public/components/navbar/navbar.js b/x-pack/plugins/canvas/public/components/navbar/navbar.js deleted file mode 100644 index dcf6389acd4a3..0000000000000 --- a/x-pack/plugins/canvas/public/components/navbar/navbar.js +++ /dev/null @@ -1,16 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import React from 'react'; -import PropTypes from 'prop-types'; - -export const Navbar = ({ children }) => { - return
{children}
; -}; - -Navbar.propTypes = { - children: PropTypes.node, -}; diff --git a/x-pack/plugins/canvas/public/components/navbar/navbar.scss b/x-pack/plugins/canvas/public/components/navbar/navbar.scss deleted file mode 100644 index 7b490822763d2..0000000000000 --- a/x-pack/plugins/canvas/public/components/navbar/navbar.scss +++ /dev/null @@ -1,7 +0,0 @@ -.canvasNavbar { - width: 100%; - height: $euiSizeXL * 2; - background-color: darken($euiColorLightestShade, 5%); - position: relative; - z-index: 200; -} diff --git a/x-pack/plugins/canvas/public/components/toolbar/__examples__/__snapshots__/toolbar.stories.storyshot b/x-pack/plugins/canvas/public/components/toolbar/__examples__/__snapshots__/toolbar.stories.storyshot new file mode 100644 index 0000000000000..eec0de3c784f1 --- /dev/null +++ b/x-pack/plugins/canvas/public/components/toolbar/__examples__/__snapshots__/toolbar.stories.storyshot @@ -0,0 +1,229 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Storyshots components/Toolbar element selected 1`] = ` +
+
+
+
+ +
+
+
+ +
+
+ +
+
+ +
+
+
+ +
+
+
+
+`; + +exports[`Storyshots components/Toolbar no element selected 1`] = ` +
+
+
+
+ +
+
+
+ +
+
+ +
+
+ +
+
+
+
+
+`; diff --git a/x-pack/plugins/canvas/public/components/toolbar/__examples__/toolbar.stories.tsx b/x-pack/plugins/canvas/public/components/toolbar/__examples__/toolbar.stories.tsx index 5907c932ddabb..bd6ad7c8dc499 100644 --- a/x-pack/plugins/canvas/public/components/toolbar/__examples__/toolbar.stories.tsx +++ b/x-pack/plugins/canvas/public/components/toolbar/__examples__/toolbar.stories.tsx @@ -4,36 +4,30 @@ * you may not use this file except in compliance with the Elastic License. */ -/* - TODO: uncomment and fix this test to address storybook errors as a result of nested component dependencies - https://github.com/elastic/kibana/issues/58289 - */ - -/* -import { action } from '@storybook/addon-actions'; import { storiesOf } from '@storybook/react'; import React from 'react'; -import { Toolbar } from '../toolbar'; +import { Toolbar } from '../toolbar.component'; + +// @ts-expect-error untyped local +import { getDefaultElement } from '../../../state/defaults'; storiesOf('components/Toolbar', module) - .addDecorator(story => ( -
- {story()} -
- )) - .add('with null metric', () => ( + .add('no element selected', () => ( + )) + .add('element selected', () => ( + )); -*/ diff --git a/x-pack/plugins/canvas/public/components/toolbar/index.js b/x-pack/plugins/canvas/public/components/toolbar/index.js deleted file mode 100644 index 16860063f8a45..0000000000000 --- a/x-pack/plugins/canvas/public/components/toolbar/index.js +++ /dev/null @@ -1,49 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import { connect } from 'react-redux'; -import PropTypes from 'prop-types'; -import { pure, compose, withState, getContext, withHandlers } from 'recompose'; -import { canUserWrite } from '../../state/selectors/app'; - -import { - getWorkpad, - getWorkpadName, - getSelectedPageIndex, - getSelectedElement, - isWriteable, -} from '../../state/selectors/workpad'; - -import { Toolbar as Component } from './toolbar'; - -const mapStateToProps = (state) => ({ - workpadName: getWorkpadName(state), - workpadId: getWorkpad(state).id, - totalPages: getWorkpad(state).pages.length, - selectedPageNumber: getSelectedPageIndex(state) + 1, - selectedElement: getSelectedElement(state), - isWriteable: isWriteable(state) && canUserWrite(state), -}); - -export const Toolbar = compose( - pure, - connect(mapStateToProps), - getContext({ - router: PropTypes.object, - }), - withHandlers({ - nextPage: (props) => () => { - const pageNumber = Math.min(props.selectedPageNumber + 1, props.totalPages); - props.router.navigateTo('loadWorkpad', { id: props.workpadId, page: pageNumber }); - }, - previousPage: (props) => () => { - const pageNumber = Math.max(1, props.selectedPageNumber - 1); - props.router.navigateTo('loadWorkpad', { id: props.workpadId, page: pageNumber }); - }, - }), - withState('tray', 'setTray', (props) => props.tray), - withState('showWorkpadManager', 'setShowWorkpadManager', false) -)(Component); diff --git a/x-pack/plugins/canvas/public/components/navbar/index.js b/x-pack/plugins/canvas/public/components/toolbar/index.ts similarity index 66% rename from x-pack/plugins/canvas/public/components/navbar/index.js rename to x-pack/plugins/canvas/public/components/toolbar/index.ts index 6948ada93155d..dfa730307dafb 100644 --- a/x-pack/plugins/canvas/public/components/navbar/index.js +++ b/x-pack/plugins/canvas/public/components/toolbar/index.ts @@ -4,7 +4,5 @@ * you may not use this file except in compliance with the Elastic License. */ -import { pure } from 'recompose'; -import { Navbar as Component } from './navbar'; - -export const Navbar = pure(Component); +export { Toolbar } from './toolbar'; +export { Toolbar as ToolbarComponent } from './toolbar.component'; diff --git a/x-pack/plugins/canvas/public/components/toolbar/toolbar.tsx b/x-pack/plugins/canvas/public/components/toolbar/toolbar.component.tsx similarity index 64% rename from x-pack/plugins/canvas/public/components/toolbar/toolbar.tsx rename to x-pack/plugins/canvas/public/components/toolbar/toolbar.component.tsx index c5475b2559444..6905b3ed23d3f 100644 --- a/x-pack/plugins/canvas/public/components/toolbar/toolbar.tsx +++ b/x-pack/plugins/canvas/public/components/toolbar/toolbar.component.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React from 'react'; +import React, { FC, useState, useContext, useEffect } from 'react'; import PropTypes from 'prop-types'; import { EuiButtonEmpty, @@ -16,72 +16,77 @@ import { EuiModalFooter, EuiButton, } from '@elastic/eui'; -import { CanvasElement } from '../../../types'; - -import { ComponentStrings } from '../../../i18n'; -// @ts-expect-error untyped local -import { Navbar } from '../navbar'; // @ts-expect-error untyped local import { WorkpadManager } from '../workpad_manager'; +import { RouterContext } from '../router'; import { PageManager } from '../page_manager'; // @ts-expect-error untyped local import { Expression } from '../expression'; import { Tray } from './tray'; +import { CanvasElement } from '../../../types'; +import { ComponentStrings } from '../../../i18n'; + const { Toolbar: strings } = ComponentStrings; -enum TrayType { - pageManager = 'pageManager', - expression = 'expression', -} +type TrayType = 'pageManager' | 'expression'; interface Props { - workpadName: string; isWriteable: boolean; - canUserWrite: boolean; - tray: TrayType | null; - setTray: (tray: TrayType | null) => void; - - previousPage: () => void; - nextPage: () => void; + selectedElement?: CanvasElement; selectedPageNumber: number; totalPages: number; - - selectedElement: CanvasElement; - - showWorkpadManager: boolean; - setShowWorkpadManager: (show: boolean) => void; + workpadId: string; + workpadName: string; } -export const Toolbar = (props: Props) => { - const { - selectedElement, - tray, - setTray, - previousPage, - nextPage, - selectedPageNumber, - workpadName, - totalPages, - showWorkpadManager, - setShowWorkpadManager, - isWriteable, - } = props; +export const Toolbar: FC = ({ + isWriteable, + selectedElement, + selectedPageNumber, + totalPages, + workpadId, + workpadName, +}) => { + const [activeTray, setActiveTray] = useState(null); + const [showWorkpadManager, setShowWorkpadManager] = useState(false); + const router = useContext(RouterContext); + + // While the tray doesn't get activated if the workpad isn't writeable, + // this effect will ensure that if the tray is open and the workpad + // changes its writeable state, the tray will close. + useEffect(() => { + if (!isWriteable && activeTray === 'expression') { + setActiveTray(null); + } + }, [isWriteable, activeTray]); - const elementIsSelected = Boolean(selectedElement); + if (!router) { + return
{strings.getErrorMessage('Router Undefined')}
; + } - const done = () => setTray(null); + const nextPage = () => { + const page = Math.min(selectedPageNumber + 1, totalPages); + router.navigateTo('loadWorkpad', { id: workpadId, page }); + }; - if (!isWriteable && tray === TrayType.expression) { - done(); - } + const previousPage = () => { + const page = Math.max(1, selectedPageNumber - 1); + router.navigateTo('loadWorkpad', { id: workpadId, page }); + }; - const showHideTray = (exp: TrayType) => { - if (tray && tray === exp) { - return done(); + const elementIsSelected = Boolean(selectedElement); + + const toggleTray = (tray: TrayType) => { + if (activeTray === tray) { + setActiveTray(null); + } else { + if (!isWriteable && tray === 'expression') { + return; + } + setActiveTray(tray); } - setTray(exp); }; const closeWorkpadManager = () => setShowWorkpadManager(false); @@ -102,13 +107,13 @@ export const Toolbar = (props: Props) => { const trays = { pageManager: , - expression: !elementIsSelected ? null : , + expression: !elementIsSelected ? null : setActiveTray(null)} />, }; return (
- {tray !== null && {trays[tray]}} - + {activeTray !== null && setActiveTray(null)}>{trays[activeTray]}} +
openWorkpadManager()}> @@ -126,7 +131,7 @@ export const Toolbar = (props: Props) => { /> - showHideTray(TrayType.pageManager)}> + toggleTray('pageManager')}> {strings.getPageButtonLabel(selectedPageNumber, totalPages)} @@ -145,7 +150,7 @@ export const Toolbar = (props: Props) => { showHideTray(TrayType.expression)} + onClick={() => toggleTray('expression')} data-test-subj="canvasExpressionEditorButton" > {strings.getEditorButtonLabel()} @@ -153,23 +158,17 @@ export const Toolbar = (props: Props) => { )} - - +
{showWorkpadManager && workpadManager}
); }; Toolbar.propTypes = { - workpadName: PropTypes.string, - tray: PropTypes.string, - setTray: PropTypes.func.isRequired, - nextPage: PropTypes.func.isRequired, - previousPage: PropTypes.func.isRequired, + isWriteable: PropTypes.bool.isRequired, + selectedElement: PropTypes.object, selectedPageNumber: PropTypes.number.isRequired, totalPages: PropTypes.number.isRequired, - selectedElement: PropTypes.object, - showWorkpadManager: PropTypes.bool.isRequired, - setShowWorkpadManager: PropTypes.func.isRequired, - isWriteable: PropTypes.bool.isRequired, + workpadId: PropTypes.string.isRequired, + workpadName: PropTypes.string.isRequired, }; diff --git a/x-pack/plugins/canvas/public/components/toolbar/toolbar.scss b/x-pack/plugins/canvas/public/components/toolbar/toolbar.scss index 7303f43dd269f..41bc718dcfec1 100644 --- a/x-pack/plugins/canvas/public/components/toolbar/toolbar.scss +++ b/x-pack/plugins/canvas/public/components/toolbar/toolbar.scss @@ -24,3 +24,11 @@ padding: $euiSizeM; height: 100%; } + +.canvasToolbar__container { + width: 100%; + height: $euiSizeXL * 2; + background-color: darken($euiColorLightestShade, 5%); + position: relative; + z-index: 200; +} diff --git a/x-pack/plugins/canvas/public/components/toolbar/toolbar.ts b/x-pack/plugins/canvas/public/components/toolbar/toolbar.ts new file mode 100644 index 0000000000000..f93b42cb442b8 --- /dev/null +++ b/x-pack/plugins/canvas/public/components/toolbar/toolbar.ts @@ -0,0 +1,28 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { connect } from 'react-redux'; +import { canUserWrite } from '../../state/selectors/app'; + +import { + getWorkpad, + getWorkpadName, + getSelectedPageIndex, + getSelectedElement, + isWriteable, +} from '../../state/selectors/workpad'; + +import { Toolbar as ToolbarComponent } from './toolbar.component'; +import { State } from '../../../types'; + +export const Toolbar = connect((state: State) => ({ + workpadName: getWorkpadName(state), + workpadId: getWorkpad(state).id, + totalPages: getWorkpad(state).pages.length, + selectedPageNumber: getSelectedPageIndex(state) + 1, + selectedElement: getSelectedElement(state), + isWriteable: isWriteable(state) && canUserWrite(state), +}))(ToolbarComponent); diff --git a/x-pack/plugins/canvas/public/components/toolbar/tray/index.ts b/x-pack/plugins/canvas/public/components/toolbar/tray/index.ts index 1343bc8d01e9a..18c45190cbd48 100644 --- a/x-pack/plugins/canvas/public/components/toolbar/tray/index.ts +++ b/x-pack/plugins/canvas/public/components/toolbar/tray/index.ts @@ -4,7 +4,4 @@ * you may not use this file except in compliance with the Elastic License. */ -import { pure } from 'recompose'; -import { Tray as Component } from './tray'; - -export const Tray = pure(Component); +export { Tray } from './tray'; diff --git a/x-pack/plugins/canvas/public/components/toolbar/tray/tray.tsx b/x-pack/plugins/canvas/public/components/toolbar/tray/tray.tsx index 2c0b4e69c240b..0699d30833ecd 100644 --- a/x-pack/plugins/canvas/public/components/toolbar/tray/tray.tsx +++ b/x-pack/plugins/canvas/public/components/toolbar/tray/tray.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { ReactNode, Fragment, MouseEventHandler } from 'react'; +import React, { ReactNode, MouseEventHandler } from 'react'; import PropTypes from 'prop-types'; import { EuiFlexGroup, EuiFlexItem, EuiButtonIcon } from '@elastic/eui'; @@ -18,7 +18,7 @@ interface Props { export const Tray = ({ children, done }: Props) => { return ( - + <> { /> -
{children}
- + ); }; Tray.propTypes = { - children: PropTypes.node, - done: PropTypes.func, + children: PropTypes.node.isRequired, + done: PropTypes.func.isRequired, }; diff --git a/x-pack/plugins/canvas/public/style/index.scss b/x-pack/plugins/canvas/public/style/index.scss index ccaf6f1c1a478..abdbb09f8cff8 100644 --- a/x-pack/plugins/canvas/public/style/index.scss +++ b/x-pack/plugins/canvas/public/style/index.scss @@ -35,7 +35,6 @@ @import '../components/function_form/function_form'; @import '../components/hover_annotation/hover_annotation'; @import '../components/loading/loading'; -@import '../components/navbar/navbar'; @import '../components/page_manager/page_manager'; @import '../components/positionable/positionable'; @import '../components/rotation_handle/rotation_handle'; diff --git a/x-pack/plugins/canvas/storybook/decorators/router_decorator.tsx b/x-pack/plugins/canvas/storybook/decorators/router_decorator.tsx index 43b0da6473f23..db775b697d248 100644 --- a/x-pack/plugins/canvas/storybook/decorators/router_decorator.tsx +++ b/x-pack/plugins/canvas/storybook/decorators/router_decorator.tsx @@ -6,25 +6,31 @@ import React from 'react'; import PropTypes from 'prop-types'; +import { RouterContext } from '../../public/components/router'; -class RouterContext extends React.Component { +const context = { + router: { + getFullPath: () => 'path', + create: () => '', + }, + navigateTo: () => {}, +}; + +class RouterProvider extends React.Component { static childContextTypes = { router: PropTypes.object.isRequired, + navigateTo: PropTypes.func, }; getChildContext() { - return { - router: { - getFullPath: () => 'path', - create: () => '', - }, - }; + return context; } + render() { - return <>{this.props.children}; + return {this.props.children}; } } export function routerContextDecorator(story: Function) { - return {story()}; + return {story()}; }