From e63d2b2efe25d8a0b9ef2baa471733bf1a4fa29e Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Tue, 11 Aug 2020 15:13:11 -0700 Subject: [PATCH] [Feedback] Update mobile UX to close menu on link click/navigation - This requires updating our EUI/React Router components to accept and run custom onClick events - Also requires adding a new ReactContext to pass down closeNavigation, but that's not too onerous thanks to useContext --- .../shared/layout/layout.test.tsx | 14 +++++++++- .../applications/shared/layout/layout.tsx | 8 +++++- .../applications/shared/layout/side_nav.tsx | 16 +++++++++--- .../react_router_helpers/eui_link.test.tsx | 9 +++++++ .../shared/react_router_helpers/eui_link.tsx | 26 +++++++++++++------ 5 files changed, 60 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/layout/layout.test.tsx b/x-pack/plugins/enterprise_search/public/applications/shared/layout/layout.test.tsx index ad182d186191e0..4053f2f4bb613e 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/layout/layout.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/shared/layout/layout.test.tsx @@ -8,7 +8,7 @@ import React from 'react'; import { shallow } from 'enzyme'; import { EuiPageSideBar, EuiButton } from '@elastic/eui'; -import { Layout } from './'; +import { Layout, INavContext } from './layout'; describe('Layout', () => { it('renders', () => { @@ -36,6 +36,18 @@ describe('Layout', () => { expect(wrapper.find(EuiButton).prop('iconType')).toEqual('arrowDown'); }); + it('passes down NavContext to navigation links', () => { + const wrapper = shallow(} />); + + const toggle = wrapper.find('[data-test-subj="enterpriseSearchNavToggle"]'); + toggle.simulate('click'); + expect(wrapper.find(EuiPageSideBar).prop('className')).toContain('--isOpen'); + + const context = (wrapper.find('ContextProvider').prop('value') as unknown) as INavContext; + context.closeNavigation(); + expect(wrapper.find(EuiPageSideBar).prop('className')).not.toContain('--isOpen'); + }); + it('renders children', () => { const wrapper = shallow( diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/layout/layout.tsx b/x-pack/plugins/enterprise_search/public/applications/shared/layout/layout.tsx index ece1a90aa1f1ff..b4497140b65b7e 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/layout/layout.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/shared/layout/layout.tsx @@ -16,9 +16,15 @@ interface ILayoutProps { navigation: React.ReactNode; } +export interface INavContext { + closeNavigation(): void; +} +export const NavContext = React.createContext({}); + export const Layout: React.FC = ({ children, navigation }) => { const [isNavOpen, setIsNavOpen] = useState(false); const toggleNavigation = () => setIsNavOpen(!isNavOpen); + const closeNavigation = () => setIsNavOpen(false); const navClasses = classNames('enterpriseSearchLayout__sideBar', { 'enterpriseSearchLayout__sideBar--isOpen': isNavOpen, // eslint-disable-line @typescript-eslint/naming-convention @@ -46,7 +52,7 @@ export const Layout: React.FC = ({ children, navigation }) => { })} - {navigation} + {navigation} {children} diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/layout/side_nav.tsx b/x-pack/plugins/enterprise_search/public/applications/shared/layout/side_nav.tsx index 013036c139ad1e..5969fa7806a446 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/layout/side_nav.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/shared/layout/side_nav.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React from 'react'; +import React, { useContext } from 'react'; import { useLocation } from 'react-router-dom'; import classNames from 'classnames'; @@ -14,6 +14,8 @@ import { EuiLink } from '../react_router_helpers'; import { ENTERPRISE_SEARCH_PLUGIN } from '../../../../common/constants'; +import { NavContext, INavContext } from './layout'; + import './side_nav.scss'; /** @@ -73,6 +75,8 @@ export const SideNavLink: React.FC = ({ isRoot, ...rest }) => { + const { closeNavigation } = useContext(NavContext) as INavContext; + const { pathname } = useLocation(); const currentPath = pathname.endsWith('/') ? pathname.slice(0, -1) : pathname; const isActive = currentPath === to || (isRoot && currentPath === ''); @@ -84,11 +88,17 @@ export const SideNavLink: React.FC = ({ return (
  • {isExternal ? ( - + {children} ) : ( - + {children} )} diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_link.test.tsx b/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_link.test.tsx index 7d4c068b211555..76ee8293f2c8b9 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_link.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_link.test.tsx @@ -73,5 +73,14 @@ describe('EUI & React Router Component Helpers', () => { expect(mockHistory.push).not.toHaveBeenCalled(); }); + + it('calls inherited onClick actions in addition to default navigation', () => { + const customOnClick = jest.fn(); // Can be anything from telemetry to a state reset + const wrapper = mount(); + + wrapper.find(EuiLink).simulate('click', { shiftKey: true }); + + expect(customOnClick).toHaveBeenCalled(); + }); }); }); diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_link.tsx b/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_link.tsx index f486e432bae76a..b53b2f2b3b650a 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_link.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_link.tsx @@ -19,13 +19,15 @@ import { letBrowserHandleEvent } from './link_events'; interface IEuiReactRouterProps { to: string; + onClick?(): void; } -export const EuiReactRouterHelper: React.FC = ({ to, children }) => { +export const EuiReactRouterHelper: React.FC = ({ to, onClick, children }) => { const history = useHistory(); - const onClick = (event: React.MouseEvent) => { - if (letBrowserHandleEvent(event)) return; + const reactRouterLinkClick = (event: React.MouseEvent) => { + if (onClick) onClick(); // Run any passed click events (e.g. telemetry) + if (letBrowserHandleEvent(event)) return; // Return early if the link behavior shouldn't be handled by React Router // Prevent regular link behavior, which causes a browser refresh. event.preventDefault(); @@ -37,21 +39,29 @@ export const EuiReactRouterHelper: React.FC = ({ to, child // Generate the correct link href (with basename etc. accounted for) const href = history.createHref({ pathname: to }); - const reactRouterProps = { href, onClick }; + const reactRouterProps = { href, onClick: reactRouterLinkClick }; return React.cloneElement(children as React.ReactElement, reactRouterProps); }; type TEuiReactRouterLinkProps = EuiLinkAnchorProps & IEuiReactRouterProps; type TEuiReactRouterButtonProps = EuiButtonProps & IEuiReactRouterProps; -export const EuiReactRouterLink: React.FC = ({ to, ...rest }) => ( - +export const EuiReactRouterLink: React.FC = ({ + to, + onClick, + ...rest +}) => ( + ); -export const EuiReactRouterButton: React.FC = ({ to, ...rest }) => ( - +export const EuiReactRouterButton: React.FC = ({ + to, + onClick, + ...rest +}) => ( + );