Skip to content

Commit

Permalink
[Uptime] Fix full reloads while navigating to alert/ml (#73796)
Browse files Browse the repository at this point in the history
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
shahzad31 and elasticmachine authored Aug 10, 2020
1 parent cccf15a commit 244ed04
Show file tree
Hide file tree
Showing 16 changed files with 371 additions and 29 deletions.
15 changes: 10 additions & 5 deletions x-pack/plugins/uptime/public/apps/uptime_app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ import React, { useEffect } from 'react';
import { Provider as ReduxProvider } from 'react-redux';
import { BrowserRouter as Router } from 'react-router-dom';
import { I18nStart, ChromeBreadcrumb, CoreStart } from 'kibana/public';
import { KibanaContextProvider } from '../../../../../src/plugins/kibana_react/public';
import {
KibanaContextProvider,
RedirectAppLinks,
} from '../../../../../src/plugins/kibana_react/public';
import { ClientPluginsSetup, ClientPluginsStart } from './plugin';
import { UMUpdateBadge } from '../lib/lib';
import {
Expand Down Expand Up @@ -103,10 +106,12 @@ const Application = (props: UptimeAppProps) => {
<UptimeStartupPluginsContextProvider {...startPlugins}>
<UptimeAlertsContextProvider>
<EuiPage className="app-wrapper-panel " data-test-subj="uptimeApp">
<main>
<UptimeAlertsFlyoutWrapper />
<PageRouter />
</main>
<RedirectAppLinks application={core.application}>
<main>
<UptimeAlertsFlyoutWrapper />
<PageRouter />
</main>
</RedirectAppLinks>
</EuiPage>
</UptimeAlertsContextProvider>
</UptimeStartupPluginsContextProvider>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* 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 { letBrowserHandleEvent } from '../index';

describe('letBrowserHandleEvent', () => {
const event = {
defaultPrevented: false,
metaKey: false,
altKey: false,
ctrlKey: false,
shiftKey: false,
button: 0,
target: {
getAttribute: () => '_self',
},
} as any;

describe('the browser should handle the link when', () => {
it('default is prevented', () => {
expect(letBrowserHandleEvent({ ...event, defaultPrevented: true })).toBe(true);
});

it('is modified with metaKey', () => {
expect(letBrowserHandleEvent({ ...event, metaKey: true })).toBe(true);
});

it('is modified with altKey', () => {
expect(letBrowserHandleEvent({ ...event, altKey: true })).toBe(true);
});

it('is modified with ctrlKey', () => {
expect(letBrowserHandleEvent({ ...event, ctrlKey: true })).toBe(true);
});

it('is modified with shiftKey', () => {
expect(letBrowserHandleEvent({ ...event, shiftKey: true })).toBe(true);
});

it('it is not a left click event', () => {
expect(letBrowserHandleEvent({ ...event, button: 2 })).toBe(true);
});

it('the target is anything value other than _self', () => {
expect(
letBrowserHandleEvent({
...event,
target: targetValue('_blank'),
})
).toBe(true);
});
});

describe('the browser should NOT handle the link when', () => {
it('default is not prevented', () => {
expect(letBrowserHandleEvent({ ...event, defaultPrevented: false })).toBe(false);
});

it('is not modified', () => {
expect(
letBrowserHandleEvent({
...event,
metaKey: false,
altKey: false,
ctrlKey: false,
shiftKey: false,
})
).toBe(false);
});

it('it is a left click event', () => {
expect(letBrowserHandleEvent({ ...event, button: 0 })).toBe(false);
});

it('the target is a value of _self', () => {
expect(
letBrowserHandleEvent({
...event,
target: targetValue('_self'),
})
).toBe(false);
});

it('the target has no value', () => {
expect(
letBrowserHandleEvent({
...event,
target: targetValue(null),
})
).toBe(false);
});
});
});

const targetValue = (value: string | null) => {
return {
getAttribute: () => value,
};
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* 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 { shallow, mount } from 'enzyme';
import { EuiLink, EuiButton } from '@elastic/eui';

import '../../../../lib/__mocks__/react_router_history.mock';

import { ReactRouterEuiLink, ReactRouterEuiButton } from '../link_for_eui';
import { mockHistory } from '../../../../lib/__mocks__';

describe('EUI & React Router Component Helpers', () => {
beforeEach(() => {
jest.clearAllMocks();
});

it('renders', () => {
const wrapper = shallow(<ReactRouterEuiLink to="/" />);

expect(wrapper.find(EuiLink)).toHaveLength(1);
});

it('renders an EuiButton', () => {
const wrapper = shallow(<ReactRouterEuiButton to="/" />);

expect(wrapper.find(EuiButton)).toHaveLength(1);
});

it('passes down all ...rest props', () => {
const wrapper = shallow(<ReactRouterEuiLink to="/" data-test-subj="foo" external={true} />);
const link = wrapper.find(EuiLink);

expect(link.prop('external')).toEqual(true);
expect(link.prop('data-test-subj')).toEqual('foo');
});

it('renders with the correct href and onClick props', () => {
const wrapper = mount(<ReactRouterEuiLink to="/foo/bar" />);
const link = wrapper.find(EuiLink);

expect(link.prop('onClick')).toBeInstanceOf(Function);
expect(link.prop('href')).toEqual('/enterprise_search/foo/bar');
expect(mockHistory.createHref).toHaveBeenCalled();
});

describe('onClick', () => {
it('prevents default navigation and uses React Router history', () => {
const wrapper = mount(<ReactRouterEuiLink to="/bar/baz" />);

const simulatedEvent = {
button: 0,
target: { getAttribute: () => '_self' },
preventDefault: jest.fn(),
};
wrapper.find(EuiLink).simulate('click', simulatedEvent);

expect(simulatedEvent.preventDefault).toHaveBeenCalled();
expect(mockHistory.push).toHaveBeenCalled();
});

it('does not prevent default browser behavior on new tab/window clicks', () => {
const wrapper = mount(<ReactRouterEuiLink to="/bar/baz" />);

const simulatedEvent = {
shiftKey: true,
target: { getAttribute: () => '_blank' },
};
wrapper.find(EuiLink).simulate('click', simulatedEvent);

expect(mockHistory.push).not.toHaveBeenCalled();
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* 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.
*/

export { letBrowserHandleEvent } from './link_events';
export {
ReactRouterEuiLink,
ReactRouterEuiButton,
ReactRouterEuiButtonEmpty,
} from './link_for_eui';
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* 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 { MouseEvent } from 'react';

/**
* Helper functions for determining which events we should
* let browsers handle natively, e.g. new tabs/windows
*/

type THandleEvent = (event: MouseEvent) => boolean;

export const letBrowserHandleEvent: THandleEvent = (event) =>
event.defaultPrevented ||
isModifiedEvent(event) ||
!isLeftClickEvent(event) ||
isTargetBlank(event);

const isModifiedEvent: THandleEvent = (event) =>
!!(event.metaKey || event.altKey || event.ctrlKey || event.shiftKey);

const isLeftClickEvent: THandleEvent = (event) => event.button === 0;

const isTargetBlank: THandleEvent = (event) => {
const element = event.target as HTMLElement;
const target = element.getAttribute('target');
return !!target && target !== '_self';
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* 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 { useHistory } from 'react-router-dom';
import {
EuiLink,
EuiButton,
EuiButtonProps,
EuiButtonEmptyProps,
EuiLinkAnchorProps,
EuiButtonEmpty,
} from '@elastic/eui';

import { letBrowserHandleEvent } from './link_events';

/**
* Generates either an EuiLink or EuiButton with a React-Router-ified link
*
* Based off of EUI's recommendations for handling React Router:
* https://github.com/elastic/eui/blob/master/wiki/react-router.md#react-router-51
*/

interface IEuiReactRouterProps {
to: string;
}

export const ReactRouterHelperForEui: React.FC<IEuiReactRouterProps> = ({ to, children }) => {
const history = useHistory();

const onClick = (event: React.MouseEvent) => {
if (letBrowserHandleEvent(event)) return;

// Prevent regular link behavior, which causes a browser refresh.
event.preventDefault();

// Push the route to the history.
history.push(to);
};

// Generate the correct link href (with basename etc. accounted for)
const href = history.createHref({ pathname: to });

const reactRouterProps = { href, onClick };
return React.cloneElement(children as React.ReactElement, reactRouterProps);
};

type TEuiReactRouterLinkProps = EuiLinkAnchorProps & IEuiReactRouterProps;
type TEuiReactRouterButtonProps = EuiButtonProps & IEuiReactRouterProps;
type TEuiReactRouterButtonEmptyProps = EuiButtonEmptyProps & IEuiReactRouterProps;

export const ReactRouterEuiLink: React.FC<TEuiReactRouterLinkProps> = ({ to, ...rest }) => (
<ReactRouterHelperForEui to={to}>
<EuiLink {...rest} />
</ReactRouterHelperForEui>
);

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

export const ReactRouterEuiButtonEmpty: React.FC<TEuiReactRouterButtonEmptyProps> = ({
to,
...rest
}) => (
<ReactRouterHelperForEui to={to}>
<EuiButtonEmpty {...rest} />
</ReactRouterHelperForEui>
);

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,20 @@ export const ManageMLJobComponent = ({ hasMLJob, onEnableJob, onJobDelete }: Pro
const deleteAnomalyAlert = () =>
dispatch(deleteAlertAction.get({ alertId: anomalyAlert?.id as string }));

const showLoading = isMLJobCreating || isMLJobLoading;

const btnText = hasMLJob ? labels.ANOMALY_DETECTION : labels.ENABLE_ANOMALY_DETECTION;

const button = (
<EuiButton
data-test-subj={hasMLJob ? 'uptimeManageMLJobBtn' : 'uptimeEnableAnomalyBtn'}
onClick={hasMLJob ? () => setIsPopOverOpen(true) : onEnableJob}
disabled={hasMLJob && !canDeleteMLJob}
isLoading={isMLJobCreating || isMLJobLoading}
size="s"
aria-label={labels.ENABLE_MANAGE_JOB}
>
{hasMLJob ? labels.ANOMALY_DETECTION : labels.ENABLE_ANOMALY_DETECTION}
{showLoading ? '' : btnText}
</EuiButton>
);

Expand All @@ -79,7 +84,6 @@ export const ManageMLJobComponent = ({ hasMLJob, onEnableJob, onJobDelete }: Pro
monitorId,
dateRange: { from: dateRangeStart, to: dateRangeEnd },
}),
target: '_blank',
},
{
name: anomalyAlert ? labels.DISABLE_ANOMALY_ALERT : labels.ENABLE_ANOMALY_ALERT,
Expand Down
Loading

0 comments on commit 244ed04

Please sign in to comment.