Skip to content

Commit

Permalink
[Feedback] Update mobile UX to close menu on link click/navigation
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
cee-chen committed Aug 11, 2020
1 parent 9f371ad commit e63d2b2
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -36,6 +36,18 @@ describe('Layout', () => {
expect(wrapper.find(EuiButton).prop('iconType')).toEqual('arrowDown');
});

it('passes down NavContext to navigation links', () => {
const wrapper = shallow(<Layout navigation={<nav />} />);

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(
<Layout navigation={null}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,15 @@ interface ILayoutProps {
navigation: React.ReactNode;
}

export interface INavContext {
closeNavigation(): void;
}
export const NavContext = React.createContext({});

export const Layout: React.FC<ILayoutProps> = ({ 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
Expand Down Expand Up @@ -46,7 +52,7 @@ export const Layout: React.FC<ILayoutProps> = ({ children, navigation }) => {
})}
</EuiButton>
</div>
{navigation}
<NavContext.Provider value={{ closeNavigation }}>{navigation}</NavContext.Provider>
</EuiPageSideBar>
<EuiPageBody className="enterpriseSearchLayout__body">{children}</EuiPageBody>
</EuiPage>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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';

/**
Expand Down Expand Up @@ -73,6 +75,8 @@ export const SideNavLink: React.FC<ISideNavLinkProps> = ({
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 === '');
Expand All @@ -84,11 +88,17 @@ export const SideNavLink: React.FC<ISideNavLinkProps> = ({
return (
<li>
{isExternal ? (
<EuiLinkExternal {...rest} className={classes} href={to} target="_blank">
<EuiLinkExternal
{...rest}
className={classes}
href={to}
target="_blank"
onClick={closeNavigation}
>
{children}
</EuiLinkExternal>
) : (
<EuiLink {...rest} className={classes} to={to}>
<EuiLink {...rest} className={classes} to={to} onClick={closeNavigation}>
{children}
</EuiLink>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(<EuiReactRouterLink to="/narnia" onClick={customOnClick} />);

wrapper.find(EuiLink).simulate('click', { shiftKey: true });

expect(customOnClick).toHaveBeenCalled();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@ import { letBrowserHandleEvent } from './link_events';

interface IEuiReactRouterProps {
to: string;
onClick?(): void;
}

export const EuiReactRouterHelper: React.FC<IEuiReactRouterProps> = ({ to, children }) => {
export const EuiReactRouterHelper: React.FC<IEuiReactRouterProps> = ({ 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();
Expand All @@ -37,21 +39,29 @@ export const EuiReactRouterHelper: React.FC<IEuiReactRouterProps> = ({ 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<TEuiReactRouterLinkProps> = ({ to, ...rest }) => (
<EuiReactRouterHelper to={to}>
export const EuiReactRouterLink: React.FC<TEuiReactRouterLinkProps> = ({
to,
onClick,
...rest
}) => (
<EuiReactRouterHelper to={to} onClick={onClick}>
<EuiLink {...rest} />
</EuiReactRouterHelper>
);

export const EuiReactRouterButton: React.FC<TEuiReactRouterButtonProps> = ({ to, ...rest }) => (
<EuiReactRouterHelper to={to}>
export const EuiReactRouterButton: React.FC<TEuiReactRouterButtonProps> = ({
to,
onClick,
...rest
}) => (
<EuiReactRouterHelper to={to} onClick={onClick}>
<EuiButton {...rest} />
</EuiReactRouterHelper>
);

0 comments on commit e63d2b2

Please sign in to comment.