Skip to content

Commit

Permalink
[ResponseOps] Remove usage of deprecated React rendering utilities (#…
Browse files Browse the repository at this point in the history
…180098)

## Summary

Partially addresses elastic/kibana-team#805

Follows #180003

These changes come up from searching in the code and finding where
certain kinds of deprecated AppEx-SharedUX modules are imported.
**Reviewers: Please interact with critical paths through the UI
components touched in this PR, ESPECIALLY in terms of testing dark mode
and i18n.**

* Started with focusing on code owned by ResponseOps.
* Stack Management changes trickled into this PR because ResponseOps
needs the `theme` field from `ManagementAppMountParams`
* Security changes trickled into this PR, in its test code, because of
the Stack Management changes.

### Checklist

Delete any items that are not applicable to this PR.

- [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
- [ ] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
tsullivan and kibanamachine authored Apr 9, 2024
1 parent 3839523 commit 09228a3
Show file tree
Hide file tree
Showing 37 changed files with 217 additions and 280 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ import './management_app.scss';
import React, { useState, useEffect, useCallback } from 'react';
import { BehaviorSubject } from 'rxjs';

import { I18nProvider } from '@kbn/i18n-react';
import { i18n } from '@kbn/i18n';
import { AppMountParameters, ChromeBreadcrumb, ScopedHistory } from '@kbn/core/public';
import { CoreStart } from '@kbn/core/public';
import { RedirectAppLinks } from '@kbn/shared-ux-link-redirect-app';

import { reactRouterNavigate, KibanaThemeProvider } from '@kbn/kibana-react-plugin/public';
import { reactRouterNavigate } from '@kbn/kibana-react-plugin/public';
import { KibanaRenderContextProvider } from '@kbn/react-kibana-context-render';
import { KibanaPageTemplate, KibanaPageTemplateProps } from '@kbn/shared-ux-page-kibana-template';
import useObservable from 'react-use/lib/useObservable';
import { AppContextProvider } from './management_context';
Expand All @@ -32,7 +32,6 @@ import { SectionsServiceStart, NavigationCardsSubject } from '../../types';
interface ManagementAppProps {
appBasePath: string;
history: AppMountParameters['history'];
theme$: AppMountParameters['theme$'];
dependencies: ManagementAppDependencies;
}

Expand All @@ -45,13 +44,8 @@ export interface ManagementAppDependencies {
cardsNavigationConfig$: BehaviorSubject<NavigationCardsSubject>;
}

export const ManagementApp = ({
dependencies,
history,
theme$,
appBasePath,
}: ManagementAppProps) => {
const { setBreadcrumbs, isSidebarEnabled$, cardsNavigationConfig$ } = dependencies;
export const ManagementApp = ({ dependencies, history, appBasePath }: ManagementAppProps) => {
const { coreStart, setBreadcrumbs, isSidebarEnabled$, cardsNavigationConfig$ } = dependencies;
const [selectedId, setSelectedId] = useState<string>('');
const [sections, setSections] = useState<ManagementSection[]>();
const isSidebarEnabled = useObservable(isSidebarEnabled$);
Expand Down Expand Up @@ -114,30 +108,28 @@ export const ManagementApp = ({
};

return (
<RedirectAppLinks coreStart={dependencies.coreStart}>
<I18nProvider>
<KibanaRenderContextProvider i18n={coreStart.i18n} theme={coreStart.theme}>
<RedirectAppLinks coreStart={dependencies.coreStart}>
<AppContextProvider value={contextDependencies}>
<KibanaThemeProvider theme$={theme$}>
<KibanaPageTemplate
restrictWidth={false}
solutionNav={solution}
// @ts-expect-error Techincally `paddingSize` isn't supported but it is passed through,
// this is a stop-gap for Stack managmement specifically until page components can be converted to template components
mainProps={{ paddingSize: 'l' }}
panelled
>
<ManagementRouter
history={history}
theme$={theme$}
setBreadcrumbs={setBreadcrumbsScoped}
onAppMounted={onAppMounted}
sections={sections}
analytics={dependencies.coreStart.analytics}
/>
</KibanaPageTemplate>
</KibanaThemeProvider>
<KibanaPageTemplate
restrictWidth={false}
solutionNav={solution}
// @ts-expect-error Techincally `paddingSize` isn't supported but it is passed through,
// this is a stop-gap for Stack managmement specifically until page components can be converted to template components
mainProps={{ paddingSize: 'l' }}
panelled
>
<ManagementRouter
history={history}
theme={coreStart.theme}
setBreadcrumbs={setBreadcrumbsScoped}
onAppMounted={onAppMounted}
sections={sections}
analytics={coreStart.analytics}
/>
</KibanaPageTemplate>
</AppContextProvider>
</I18nProvider>
</RedirectAppLinks>
</RedirectAppLinks>
</KibanaRenderContextProvider>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
AppMountParameters,
ChromeBreadcrumb,
ScopedHistory,
ThemeServiceStart,
} from '@kbn/core/public';
import { KibanaErrorBoundary, KibanaErrorBoundaryProvider } from '@kbn/shared-ux-error-boundary';
import { ManagementAppWrapper } from '../management_app_wrapper';
Expand All @@ -22,7 +23,7 @@ import { ManagementSection } from '../../utils';

interface ManagementRouterProps {
history: AppMountParameters['history'];
theme$: AppMountParameters['theme$'];
theme: ThemeServiceStart;
setBreadcrumbs: (crumbs?: ChromeBreadcrumb[], appHistory?: ScopedHistory) => void;
onAppMounted: (id: string) => void;
sections: ManagementSection[];
Expand All @@ -35,7 +36,7 @@ export const ManagementRouter = memo(
setBreadcrumbs,
onAppMounted,
sections,
theme$,
theme,
analytics,
}: ManagementRouterProps) => {
return (
Expand All @@ -55,7 +56,7 @@ export const ManagementRouter = memo(
setBreadcrumbs={setBreadcrumbs}
onAppMounted={onAppMounted}
history={history}
theme$={theme$}
theme={theme}
/>
)}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@

import React, { createRef, Component } from 'react';

import { ChromeBreadcrumb, AppMountParameters, ScopedHistory } from '@kbn/core/public';
import {
ChromeBreadcrumb,
AppMountParameters,
ScopedHistory,
ThemeServiceStart,
} from '@kbn/core/public';
import classNames from 'classnames';
import { APP_WRAPPER_CLASS } from '@kbn/core/public';
import { ThrowIfError } from '@kbn/shared-ux-error-boundary';
Expand All @@ -20,7 +25,7 @@ interface ManagementSectionWrapperProps {
setBreadcrumbs: (crumbs?: ChromeBreadcrumb[], history?: ScopedHistory) => void;
onAppMounted: (id: string) => void;
history: AppMountParameters['history'];
theme$: AppMountParameters['theme$'];
theme: ThemeServiceStart;
}

interface ManagementSectionWrapperState {
Expand All @@ -40,15 +45,19 @@ export class ManagementAppWrapper extends Component<
}

componentDidMount() {
const { setBreadcrumbs, app, onAppMounted, history, theme$ } = this.props;
const { setBreadcrumbs, app, onAppMounted, history, theme } = this.props;
const { mount, basePath } = app;
const appHistory = history.createSubHistory(app.basePath);

// TODO: Remove this: it provides a deprecated field still needed in ManagementAppMountParams
const { theme$ } = theme;

const mountResult = mount({
basePath,
setBreadcrumbs: (crumbs: ChromeBreadcrumb[]) => setBreadcrumbs(crumbs, appHistory),
element: this.mountElementRef.current!,
history: appHistory,
theme,
theme$,
});

Expand Down
4 changes: 3 additions & 1 deletion src/plugins/management/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import { Observable } from 'rxjs';
import { ScopedHistory, Capabilities } from '@kbn/core/public';
import { ScopedHistory, Capabilities, ThemeServiceStart } from '@kbn/core/public';
import type { LocatorPublic } from '@kbn/share-plugin/common';
import { ChromeBreadcrumb, CoreTheme } from '@kbn/core/public';
import type { CardsNavigationComponentProps } from '@kbn/management-cards-navigation';
Expand Down Expand Up @@ -70,6 +70,8 @@ export interface ManagementAppMountParams {
element: HTMLElement; // element the section should render into
setBreadcrumbs: (crumbs: ChromeBreadcrumb[]) => void;
history: ScopedHistory;
theme: ThemeServiceStart;
/** @deprecated - use `theme` **/
theme$: Observable<CoreTheme>;
}

Expand Down
1 change: 1 addition & 0 deletions src/plugins/management/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"@kbn/serverless",
"@kbn/shared-ux-error-boundary",
"@kbn/deeplinks-management",
"@kbn/react-kibana-context-render",
],
"exclude": [
"target/**/*"
Expand Down
32 changes: 14 additions & 18 deletions x-pack/plugins/alerting/public/application/maintenance_windows.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,18 @@

import React, { Suspense } from 'react';
import ReactDOM from 'react-dom';
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { Router, Routes, Route } from '@kbn/shared-ux-router';

import { EuiLoadingSpinner } from '@elastic/eui';
import { CoreStart } from '@kbn/core/public';
import { EuiThemeProvider } from '@kbn/kibana-react-plugin/common';
import { KibanaContextProvider, KibanaThemeProvider } from '@kbn/kibana-react-plugin/public';
import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public';
import { Storage } from '@kbn/kibana-utils-plugin/public';
import { ManagementAppMountParams } from '@kbn/management-plugin/public';
import { EuiLoadingSpinner } from '@elastic/eui';
import { AlertingPluginStart } from '../plugin';
import { KibanaRenderContextProvider } from '@kbn/react-kibana-context-render';
import { Route, Router, Routes } from '@kbn/shared-ux-router';
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { MAINTENANCE_WINDOW_PATHS } from '../../common';
import { useLicense } from '../hooks/use_license';
import { AlertingPluginStart } from '../plugin';

const MaintenanceWindowsLazy: React.FC = React.lazy(() => import('../pages/maintenance_windows'));
const MaintenanceWindowsCreateLazy: React.FC = React.lazy(
Expand Down Expand Up @@ -76,14 +77,13 @@ export const renderApp = ({
mountParams: ManagementAppMountParams;
kibanaVersion: string;
}) => {
const { element, history, theme$ } = mountParams;
const i18nCore = core.i18n;
const isDarkMode = core.theme.getTheme().darkMode;
const { element, history } = mountParams;
const { i18n, theme } = core;

const queryClient = new QueryClient();

ReactDOM.render(
<KibanaThemeProvider theme$={theme$}>
<KibanaRenderContextProvider i18n={i18n} theme={theme}>
<KibanaContextProvider
services={{
...core,
Expand All @@ -93,16 +93,12 @@ export const renderApp = ({
}}
>
<Router history={history}>
<EuiThemeProvider darkMode={isDarkMode}>
<i18nCore.Context>
<QueryClientProvider client={queryClient}>
<App />
</QueryClientProvider>
</i18nCore.Context>
</EuiThemeProvider>
<QueryClientProvider client={queryClient}>
<App />
</QueryClientProvider>
</Router>
</KibanaContextProvider>
</KibanaThemeProvider>,
</KibanaRenderContextProvider>,
element
);
return () => {
Expand Down
10 changes: 4 additions & 6 deletions x-pack/plugins/alerting/public/lib/test_utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@
*/

import React from 'react';
import { of, BehaviorSubject } from 'rxjs';
import { BehaviorSubject } from 'rxjs';
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { I18nProvider } from '@kbn/i18n-react';
import { KibanaContextProvider, KibanaThemeProvider } from '@kbn/kibana-react-plugin/public';
import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public';
import { KibanaThemeProvider } from '@kbn/react-kibana-context-theme';
import { render as reactRender, RenderOptions, RenderResult } from '@testing-library/react';
import { Capabilities, CoreStart } from '@kbn/core/public';
import { coreMock } from '@kbn/core/public/mocks';
import { euiDarkVars } from '@kbn/ui-theme';
import type { ILicense } from '@kbn/licensing-plugin/public';
import { licensingMock } from '@kbn/licensing-plugin/public/mocks';

Expand All @@ -40,8 +40,6 @@ export const createAppMockRenderer = ({
capabilities,
license,
}: AppMockRendererArgs = {}): AppMockRenderer => {
const theme$ = of({ eui: euiDarkVars, darkMode: true });

const licensingPluginMock = licensingMock.createStart();

const queryClient = new QueryClient({
Expand Down Expand Up @@ -83,7 +81,7 @@ export const createAppMockRenderer = ({
};
const AppWrapper: React.FC<{ children: React.ReactElement }> = React.memo(({ children }) => (
<I18nProvider>
<KibanaThemeProvider theme$={theme$}>
<KibanaThemeProvider theme={core.theme}>
<KibanaContextProvider services={services}>
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
</KibanaContextProvider>
Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugins/alerting/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@
"@kbn/core-ui-settings-server-mocks",
"@kbn/core-test-helpers-kbn-server",
"@kbn/core-http-router-server-internal",
"@kbn/core-execution-context-server-mocks"
"@kbn/core-execution-context-server-mocks",
"@kbn/react-kibana-context-render",
"@kbn/react-kibana-context-theme"
],
"exclude": [
"target/**/*"
Expand Down
12 changes: 7 additions & 5 deletions x-pack/plugins/cases/public/common/use_cases_toast.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type { ErrorToastOptions } from '@kbn/core/public';
import { EuiButtonEmpty, EuiText, logicalCSS, useEuiTheme } from '@elastic/eui';
import React, { useMemo } from 'react';
import { css } from '@emotion/react';
import { toMountPoint } from '@kbn/kibana-react-plugin/public';
import { toMountPoint } from '@kbn/react-kibana-mount';
import { isValidOwner } from '../../common/utils/owner';
import type { CaseUI } from '../../common';
import { AttachmentType } from '../../common/types/domain';
Expand Down Expand Up @@ -106,7 +106,8 @@ const getErrorMessage = (error: Error | ServerError): string => {

export const useCasesToast = () => {
const { appId } = useApplication();
const { getUrlForApp, navigateToUrl } = useKibana().services.application;
const { application, i18n, theme } = useKibana().services;
const { getUrlForApp, navigateToUrl } = application;

const toasts = useToasts();

Expand Down Expand Up @@ -147,12 +148,13 @@ export const useCasesToast = () => {
return toasts.addSuccess({
color: 'success',
iconType: 'check',
title: toMountPoint(<TruncatedText text={renderTitle} />),
title: toMountPoint(<TruncatedText text={renderTitle} />, { i18n, theme }),
text: toMountPoint(
<CaseToastSuccessContent
content={renderContent}
onViewCaseClick={url != null ? onViewCaseClick : undefined}
/>
/>,
{ i18n, theme }
),
});
},
Expand All @@ -175,7 +177,7 @@ export const useCasesToast = () => {
});
},
}),
[appId, getUrlForApp, navigateToUrl, toasts]
[i18n, theme, appId, getUrlForApp, navigateToUrl, toasts]
);
};

Expand Down
46 changes: 0 additions & 46 deletions x-pack/plugins/cases/public/common/use_is_dark_theme.test.tsx

This file was deleted.

Loading

0 comments on commit 09228a3

Please sign in to comment.