Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Uptime] Fix full reloads while navigating to alert/ml #73796

Merged
merged 16 commits into from
Aug 10, 2020
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 }) => {
shahzad31 marked this conversation as resolved.
Show resolved Hide resolved
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