Skip to content

Commit

Permalink
[Search] replace custom toasts with core notifications toasts (elasti…
Browse files Browse the repository at this point in the history
…c#176868)

## Summary

Updated the `enterprise_search` plugin to use the toasts from
`core.notifications` instead of rendering toasts in `enterprise_search`
directly.

The solution for this is a slight hack in that I added the
NotificationsStart as a value on the flash messages logic and reference
it in the existing helpers. Ideally I would have preferred to replace
the usages of the helpers with direct calls to the notifications
service, but that would have proved to be a MUCH larger refactor.

### Screenshots

![image](https://github.com/elastic/kibana/assets/1972968/244d10bf-cb36-40a8-9dcd-1d337348b07e)

![image](https://github.com/elastic/kibana/assets/1972968/86340bc7-19e9-40b4-8288-6e9f35ad9914)

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
  • Loading branch information
TattdCodeMonkey authored Feb 14, 2024
1 parent 2d1713d commit 0a55a39
Show file tree
Hide file tree
Showing 11 changed files with 99 additions and 192 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { ClientConfigType, InitialAppData, ProductAccess } from '../../common/ty
import { PluginsStart, ClientData, ESConfig } from '../plugin';

import { externalUrl } from './shared/enterprise_search_url';
import { mountFlashMessagesLogic, Toasts } from './shared/flash_messages';
import { mountFlashMessagesLogic } from './shared/flash_messages';
import { getCloudEnterpriseSearchHost } from './shared/get_cloud_enterprise_search_host/get_cloud_enterprise_search_host';
import { mountHttpLogic } from './shared/http';
import { mountKibanaLogic } from './shared/kibana';
Expand Down Expand Up @@ -66,7 +66,7 @@ export const renderApp = (
workplaceSearch,
} = data;
const { history } = params;
const { application, chrome, http, uiSettings } = core;
const { application, chrome, http, notifications, uiSettings } = core;
const { capabilities, navigateToUrl } = application;
const { charts, cloud, guidedOnboarding, lens, security, share, ml } = plugins;

Expand Down Expand Up @@ -138,7 +138,7 @@ export const renderApp = (
http,
readOnlyMode,
});
const unmountFlashMessagesLogic = mountFlashMessagesLogic();
const unmountFlashMessagesLogic = mountFlashMessagesLogic({ notifications });

ReactDOM.render(
<I18nProvider>
Expand All @@ -158,7 +158,6 @@ export const renderApp = (
searchOAuth={searchOAuth}
workplaceSearch={workplaceSearch}
/>
<Toasts />
</Router>
</Provider>
</CloudContext>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@
* 2.0.
*/

import { setMockValues, setMockActions } from '../../__mocks__/kea_logic';
import { setMockValues } from '../../__mocks__/kea_logic';

import React from 'react';

import { shallow } from 'enzyme';

import { EuiCallOut, EuiGlobalToastList } from '@elastic/eui';
import { EuiCallOut } from '@elastic/eui';

import { FlashMessages, Toasts } from './flash_messages';
import { FlashMessages } from './flash_messages';

describe('FlashMessages', () => {
it('renders an array of callouts', () => {
Expand Down Expand Up @@ -52,37 +52,3 @@ describe('FlashMessages', () => {
expect(wrapper.find('[data-test-subj="testing"]').text()).toContain('Some action');
});
});

describe('Toasts', () => {
const actions = { dismissToastMessage: jest.fn() };
beforeAll(() => setMockActions(actions));

it('renders an EUI toast list', () => {
const mockToasts = [
{ id: 'test', title: 'Hello world!!' },
{
color: 'success',
iconType: 'check',
title: 'Success!',
toastLifeTimeMs: 500,
id: 'successToastId',
},
{
color: 'danger',
iconType: 'error',
title: 'Oh no!',
text: <div data-test-subj="error">Something went wrong</div>,
id: 'errorToastId',
},
];
setMockValues({ toastMessages: mockToasts });

const wrapper = shallow(<Toasts />);
const euiToastList = wrapper.find(EuiGlobalToastList);

expect(euiToastList).toHaveLength(1);
expect(euiToastList.prop('toasts')).toEqual(mockToasts);
expect(euiToastList.prop('dismissToast')).toEqual(actions.dismissToastMessage);
expect(euiToastList.prop('toastLifeTimeMs')).toEqual(5000);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@

import React, { Fragment } from 'react';

import { useValues, useActions } from 'kea';
import { useValues } from 'kea';

import { EuiCallOut, EuiSpacer, EuiGlobalToastList } from '@elastic/eui';
import { EuiCallOut, EuiSpacer } from '@elastic/eui';

import { FLASH_MESSAGE_TYPES, DEFAULT_TOAST_TIMEOUT } from './constants';
import { FLASH_MESSAGE_TYPES } from './constants';
import { FlashMessagesLogic } from './flash_messages_logic';

export const FlashMessages: React.FC = ({ children }) => {
Expand All @@ -35,21 +35,3 @@ export const FlashMessages: React.FC = ({ children }) => {
</div>
);
};

/*
* NOTE: Toasts are rendered at the highest app level (@see public/applications/index.tsx)
* so that they don't rerender/reset their timers when navigating between pages,
* and also to prevent z-index issues with flyouts and modals
*/
export const Toasts: React.FC = () => {
const { toastMessages } = useValues(FlashMessagesLogic);
const { dismissToastMessage } = useActions(FlashMessagesLogic);

return (
<EuiGlobalToastList
toasts={toastMessages}
dismissToast={dismissToastMessage}
toastLifeTimeMs={DEFAULT_TOAST_TIMEOUT}
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,28 @@
* 2.0.
*/

import { LogicMounter } from '../../__mocks__/kea_logic';
import { mockKibanaValues } from '../../__mocks__/kea_logic/kibana_logic.mock';

import { resetContext } from 'kea';

import type { NotificationsStart } from '@kbn/core-notifications-browser';

const { history } = mockKibanaValues;

import { FlashMessagesLogic, mountFlashMessagesLogic } from './flash_messages_logic';
import { FlashMessagesLogic } from './flash_messages_logic';
import { IFlashMessage } from './types';

describe('FlashMessagesLogic', () => {
const { mount: mountFlashMessagesLogic, unmount: unmountFlashMessagesLogic } = new LogicMounter(
FlashMessagesLogic
);
const mount = () => {
resetContext({});
return mountFlashMessagesLogic();
mountFlashMessagesLogic(undefined, {
notifications: {} as unknown as NotificationsStart,
});
return unmountFlashMessagesLogic;
};

beforeEach(() => {
Expand All @@ -27,10 +36,10 @@ describe('FlashMessagesLogic', () => {
it('has default values', () => {
mount();
expect(FlashMessagesLogic.values).toEqual({
historyListener: expect.any(Function),
messages: [],
notifications: {},
queuedMessages: [],
toastMessages: [],
historyListener: expect.any(Function),
});
});

Expand Down Expand Up @@ -94,45 +103,6 @@ describe('FlashMessagesLogic', () => {
});
});

describe('toastMessages', () => {
beforeAll(() => {
mount();
});

describe('addToastMessage', () => {
it('appends a toast message to the current toasts array', () => {
FlashMessagesLogic.actions.addToastMessage({ id: 'hello' });
FlashMessagesLogic.actions.addToastMessage({ id: 'world' });
FlashMessagesLogic.actions.addToastMessage({ id: 'lorem ipsum' });

expect(FlashMessagesLogic.values.toastMessages).toEqual([
{ id: 'hello' },
{ id: 'world' },
{ id: 'lorem ipsum' },
]);
});
});

describe('dismissToastMessage', () => {
it('removes a specific toast ID from the current toasts array', () => {
FlashMessagesLogic.actions.dismissToastMessage({ id: 'world' });

expect(FlashMessagesLogic.values.toastMessages).toEqual([
{ id: 'hello' },
{ id: 'lorem ipsum' },
]);
});
});

describe('clearToastMessages', () => {
it('resets toast messages back to an empty array', () => {
FlashMessagesLogic.actions.clearToastMessages();

expect(FlashMessagesLogic.values.toastMessages).toEqual([]);
});
});
});

describe('history listener logic', () => {
describe('setHistoryListener', () => {
it('sets the historyListener value', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,75 +7,67 @@

import { kea, MakeLogicType } from 'kea';

import { EuiGlobalToastListToast as IToast } from '@elastic/eui';
import type { NotificationsStart } from '@kbn/core-notifications-browser';

import { KibanaLogic } from '../kibana';

import { IFlashMessage } from './types';

interface FlashMessagesValues {
historyListener: Function | null;
messages: IFlashMessage[];
notifications: NotificationsStart;
queuedMessages: IFlashMessage[];
toastMessages: IToast[];
historyListener: Function | null;
}
interface FlashMessagesActions {
setFlashMessages(messages: IFlashMessage | IFlashMessage[]): { messages: IFlashMessage[] };
clearFlashMessages(): void;
setQueuedMessages(messages: IFlashMessage | IFlashMessage[]): { messages: IFlashMessage[] };
clearQueuedMessages(): void;
addToastMessage(newToast: IToast): { newToast: IToast };
dismissToastMessage(removedToast: IToast): { removedToast: IToast };
clearToastMessages(): void;
setHistoryListener(historyListener: Function): { historyListener: Function };
}

interface FlashMessagesLogicProps {
notifications: NotificationsStart;
}

const convertToArray = (messages: IFlashMessage | IFlashMessage[]) =>
!Array.isArray(messages) ? [messages] : messages;

export const FlashMessagesLogic = kea<MakeLogicType<FlashMessagesValues, FlashMessagesActions>>({
path: ['enterprise_search', 'flash_messages_logic'],
export const FlashMessagesLogic = kea<
MakeLogicType<FlashMessagesValues, FlashMessagesActions, FlashMessagesLogicProps>
>({
actions: {
setFlashMessages: (messages) => ({ messages: convertToArray(messages) }),
clearFlashMessages: () => null,
setQueuedMessages: (messages) => ({ messages: convertToArray(messages) }),
clearQueuedMessages: () => null,
addToastMessage: (newToast) => ({ newToast }),
dismissToastMessage: (removedToast) => ({ removedToast }),
clearToastMessages: () => null,
setFlashMessages: (messages) => ({ messages: convertToArray(messages) }),
setHistoryListener: (historyListener) => ({ historyListener }),
setQueuedMessages: (messages) => ({ messages: convertToArray(messages) }),
},
reducers: {
path: ['enterprise_search', 'flash_messages_logic'],
reducers: ({ props }) => ({
historyListener: [
null,
{
setHistoryListener: (_, { historyListener }) => historyListener,
},
],
messages: [
[],
{
setFlashMessages: (_, { messages }) => messages,
clearFlashMessages: () => [],
setFlashMessages: (_, { messages }) => messages,
},
],
notifications: [props.notifications || {}, {}],
queuedMessages: [
[],
{
setQueuedMessages: (_, { messages }) => messages,
clearQueuedMessages: () => [],
setQueuedMessages: (_, { messages }) => messages,
},
],
toastMessages: [
[],
{
addToastMessage: (toasts, { newToast }) => [...toasts, newToast],
dismissToastMessage: (toasts, { removedToast }) =>
toasts.filter(({ id }) => id !== removedToast.id),
clearToastMessages: () => [],
},
],
historyListener: [
null,
{
setHistoryListener: (_, { historyListener }) => historyListener,
},
],
},
}),
events: ({ values, actions }) => ({
afterMount: () => {
// On React Router navigation, clear previous flash messages and load any queued messages
Expand All @@ -96,7 +88,8 @@ export const FlashMessagesLogic = kea<MakeLogicType<FlashMessagesValues, FlashMe
/**
* Mount/props helper
*/
export const mountFlashMessagesLogic = () => {
export const mountFlashMessagesLogic = (props: FlashMessagesLogicProps) => {
FlashMessagesLogic(props);
const unmount = FlashMessagesLogic.mount();
return unmount;
};
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ jest.mock('./set_message_helpers', () => ({
}));
import '../../__mocks__/kea_logic/kibana_logic.mock';

import type { NotificationsStart } from '@kbn/core-notifications-browser';

import { FlashMessagesLogic } from './flash_messages_logic';
import { flashAPIErrors, getErrorsFromHttpResponse, toastAPIErrors } from './handle_api_errors';
import { flashErrorToast } from './set_message_helpers';
Expand All @@ -28,6 +30,7 @@ describe('flashAPIErrors', () => {

beforeEach(() => {
jest.clearAllMocks();
FlashMessagesLogic({ notifications: {} as unknown as NotificationsStart });
FlashMessagesLogic.mount();
jest.spyOn(FlashMessagesLogic.actions, 'setFlashMessages');
jest.spyOn(FlashMessagesLogic.actions, 'setQueuedMessages');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

export { FlashMessages, Toasts } from './flash_messages';
export { FlashMessages } from './flash_messages';
export { FlashMessagesLogic, mountFlashMessagesLogic } from './flash_messages_logic';
export type { IFlashMessage } from './types';
export { flashAPIErrors, toastAPIErrors } from './handle_api_errors';
Expand Down
Loading

0 comments on commit 0a55a39

Please sign in to comment.