Skip to content

Commit

Permalink
Fix SpacesContext wrapper
Browse files Browse the repository at this point in the history 
The wrapper would recreate the underlying context object each time
it is re-rendered. That was not a problem for the Saved Objects
Management page, which only rendered it once -- but it turned out
to be a problem for the Machine Learning Jobs management page which
re-renders all of its children multiple times.
  • Loading branch information
@jportner
jportner committed Feb 12, 2021
1 parent 9d77ce8 commit d28bbda
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 128 deletions.
47 changes: 20 additions & 27 deletions ...a_frame_analytics/pages/analytics_management/components/analytics_list/analytics_list.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,9 @@ import { filterAnalytics } from '../../../../common/search_bar_filters';
import { AnalyticsEmptyPrompt } from './empty_prompt';
import { useTableSettings } from './use_table_settings';
import { RefreshAnalyticsListButton } from '../refresh_analytics_list_button';
import { PLUGIN_ID } from '../../../../../../../common/constants/app';
import { ListingPageUrlState } from '../../../../../../../common/types/common';
import { JobsAwaitingNodeWarning } from '../../../../../components/jobs_awaiting_node_warning';

const EmptyFunctionComponent: React.FC = ({ children }) => <>{children}</>;

const filters: EuiSearchBarProps['filters'] = [
{
type: 'field_value_selection',
Expand Down Expand Up @@ -265,8 +262,6 @@ export const DataFrameAnalyticsList: FC<Props> = ({
filters,
};

const ContextWrapper = spacesApi?.ui.components.SpacesContext || EmptyFunctionComponent;

return (
<div data-test-subj="mlAnalyticsJobList">
{modals}
Expand All @@ -290,28 +285,26 @@ export const DataFrameAnalyticsList: FC<Props> = ({
</EuiFlexGroup>
<EuiSpacer size="m" />
<div data-test-subj="mlAnalyticsTableContainer">
<ContextWrapper feature={PLUGIN_ID}>
<EuiInMemoryTable<DataFrameAnalyticsListRow>
allowNeutralSort={false}
columns={columns}
hasActions={false}
isExpandable={true}
itemIdToExpandedRowMap={itemIdToExpandedRowMap}
isSelectable={false}
items={analytics}
itemId={DataFrameAnalyticsListColumn.id}
loading={isLoading}
onTableChange={onTableChange}
pagination={pagination}
sorting={sorting}
search={search}
data-test-subj={isLoading ? 'mlAnalyticsTable loading' : 'mlAnalyticsTable loaded'}
rowProps={(item) => ({
'data-test-subj': `mlAnalyticsTableRow row-${item.id}`,
})}
error={searchError}
/>
</ContextWrapper>
<EuiInMemoryTable<DataFrameAnalyticsListRow>
allowNeutralSort={false}
columns={columns}
hasActions={false}
isExpandable={true}
itemIdToExpandedRowMap={itemIdToExpandedRowMap}
isSelectable={false}
items={analytics}
itemId={DataFrameAnalyticsListColumn.id}
loading={isLoading}
onTableChange={onTableChange}
pagination={pagination}
sorting={sorting}
search={search}
data-test-subj={isLoading ? 'mlAnalyticsTable loading' : 'mlAnalyticsTable loaded'}
rowProps={(item) => ({
'data-test-subj': `mlAnalyticsTableRow row-${item.id}`,
})}
error={searchError}
/>
</div>

{isSourceIndexModalVisible === true && (
Expand Down
60 changes: 27 additions & 33 deletions x-pack/plugins/ml/public/application/jobs/jobs_list/components/jobs_list/jobs_list.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { ResultLinks, actionsMenuContent } from '../job_actions';
import { JobDescription } from './job_description';
import { JobIcon } from '../../../../components/job_message_icon';
import { JobSpacesList } from '../../../../components/job_spaces_list';
import { PLUGIN_ID } from '../../../../../../common/constants/app';
import { TIME_FORMAT } from '../../../../../../common/constants/time_format';

import { EuiBasicTable, EuiButtonIcon, EuiScreenReaderOnly } from '@elastic/eui';
Expand All @@ -26,8 +25,6 @@ import { AnomalyDetectionJobIdLink } from './job_id_link';

const PAGE_SIZE_OPTIONS = [10, 25, 50];

const EmptyFunctionComponent = ({ children }) => <>{children}</>;

// 'isManagementTable' bool prop to determine when to configure table for use in Kibana management page
export class JobsList extends Component {
constructor(props) {
Expand Down Expand Up @@ -331,38 +328,35 @@ export class JobsList extends Component {
};

const selectedJobsClass = this.props.selectedJobsCount ? 'jobs-selected' : '';
const ContextWrapper = spacesApi?.ui.components.SpacesContext || EmptyFunctionComponent;

return (
<ContextWrapper feature={PLUGIN_ID}>
<EuiBasicTable
data-test-subj={loading ? 'mlJobListTable loading' : 'mlJobListTable loaded'}
loading={loading === true}
noItemsMessage={
loading
? i18n.translate('xpack.ml.jobsList.loadingJobsLabel', {
defaultMessage: 'Loading jobs…',
})
: i18n.translate('xpack.ml.jobsList.noJobsFoundLabel', {
defaultMessage: 'No jobs found',
})
}
itemId="id"
className={`jobs-list-table ${selectedJobsClass}`}
items={pageOfItems}
columns={columns}
pagination={pagination}
onChange={this.onTableChange}
selection={isManagementTable ? undefined : selectionControls}
itemIdToExpandedRowMap={this.state.itemIdToExpandedRowMap}
isExpandable={true}
sorting={sorting}
hasActions={true}
rowProps={(item) => ({
'data-test-subj': `mlJobListRow row-${item.id}`,
})}
/>
</ContextWrapper>
<EuiBasicTable
data-test-subj={loading ? 'mlJobListTable loading' : 'mlJobListTable loaded'}
loading={loading === true}
noItemsMessage={
loading
? i18n.translate('xpack.ml.jobsList.loadingJobsLabel', {
defaultMessage: 'Loading jobs…',
})
: i18n.translate('xpack.ml.jobsList.noJobsFoundLabel', {
defaultMessage: 'No jobs found',
})
}
itemId="id"
className={`jobs-list-table ${selectedJobsClass}`}
items={pageOfItems}
columns={columns}
pagination={pagination}
onChange={this.onTableChange}
selection={isManagementTable ? undefined : selectionControls}
itemIdToExpandedRowMap={this.state.itemIdToExpandedRowMap}
isExpandable={true}
sorting={sorting}
hasActions={true}
rowProps={(item) => ({
'data-test-subj': `mlJobListRow row-${item.id}`,
})}
/>
);
}
}
Expand Down
123 changes: 65 additions & 58 deletions ...s/ml/public/application/management/jobs_list/components/jobs_list_page/jobs_list_page.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
EuiTabbedContentTab,
} from '@elastic/eui';

import { PLUGIN_ID } from '../../../../../../common/constants/app';
import { ManagementAppMountParams } from '../../../../../../../../../src/plugins/management/public/';

import { checkGetManagementMlJobsResolver } from '../../../../capabilities/check_capabilities';
Expand Down Expand Up @@ -66,6 +67,8 @@ function usePageState<T extends ListingPageUrlState>(
return [pageState, updateState];
}

const EmptyFunctionComponent: React.FC = ({ children }) => <>{children}</>;

function useTabs(isMlEnabledInSpace: boolean, spacesApi: SpacesPluginStart | undefined): Tab[] {
const [adPageState, updateAdPageState] = usePageState(getDefaultAnomalyDetectionJobsListState());
const [dfaPageState, updateDfaPageState] = usePageState(getDefaultDFAListState());
Expand Down Expand Up @@ -182,70 +185,74 @@ export const JobsListPage: FC<{
return <AccessDeniedPage />;
}

const ContextWrapper = spacesApi?.ui.components.SpacesContext || EmptyFunctionComponent;

return (
<RedirectAppLinks application={coreStart.application}>
<I18nContext>
<KibanaContextProvider
services={{ ...coreStart, share, mlServices: getMlGlobalServices(coreStart.http) }}
>
<Router history={history}>
<EuiPageContent
id="kibanaManagementMLSection"
data-test-subj="mlPageStackManagementJobsList"
>
<EuiTitle size="l">
<EuiFlexGroup alignItems="center" justifyContent="spaceBetween">
<EuiFlexItem grow={false}>
<h1>
{i18n.translate('xpack.ml.management.jobsList.jobsListTitle', {
defaultMessage: 'Machine Learning Jobs',
})}
</h1>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButtonEmpty
target="_blank"
iconType="help"
iconSide="left"
color="primary"
href={
currentTabId === 'anomaly_detection_jobs'
? anomalyDetectionJobsUrl
: dataFrameAnalyticsUrl
}
>
{currentTabId === 'anomaly_detection_jobs'
? anomalyDetectionDocsLabel
: analyticsDocsLabel}
</EuiButtonEmpty>
</EuiFlexItem>
</EuiFlexGroup>
</EuiTitle>
<EuiSpacer size="s" />
<EuiTitle size="s">
<EuiText color="subdued">
{i18n.translate('xpack.ml.management.jobsList.jobsListTagline', {
defaultMessage: 'View machine learning analytics and anomaly detection jobs.',
})}
</EuiText>
</EuiTitle>
<EuiSpacer size="l" />
<EuiPageContentBody>
{spacesEnabled && (
<>
<EuiButtonEmpty onClick={() => setShowSyncFlyout(true)}>
{i18n.translate('xpack.ml.management.jobsList.syncFlyoutButton', {
defaultMessage: 'Synchronize saved objects',
})}
</EuiButtonEmpty>
{showSyncFlyout && <JobSpacesSyncFlyout onClose={onCloseSyncFlyout} />}
<EuiSpacer size="s" />
</>
)}
{renderTabs()}
</EuiPageContentBody>
</EuiPageContent>
</Router>
<ContextWrapper feature={PLUGIN_ID}>
<Router history={history}>
<EuiPageContent
id="kibanaManagementMLSection"
data-test-subj="mlPageStackManagementJobsList"
>
<EuiTitle size="l">
<EuiFlexGroup alignItems="center" justifyContent="spaceBetween">
<EuiFlexItem grow={false}>
<h1>
{i18n.translate('xpack.ml.management.jobsList.jobsListTitle', {
defaultMessage: 'Machine Learning Jobs',
})}
</h1>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButtonEmpty
target="_blank"
iconType="help"
iconSide="left"
color="primary"
href={
currentTabId === 'anomaly_detection_jobs'
? anomalyDetectionJobsUrl
: dataFrameAnalyticsUrl
}
>
{currentTabId === 'anomaly_detection_jobs'
? anomalyDetectionDocsLabel
: analyticsDocsLabel}
</EuiButtonEmpty>
</EuiFlexItem>
</EuiFlexGroup>
</EuiTitle>
<EuiSpacer size="s" />
<EuiTitle size="s">
<EuiText color="subdued">
{i18n.translate('xpack.ml.management.jobsList.jobsListTagline', {
defaultMessage: 'View machine learning analytics and anomaly detection jobs.',
})}
</EuiText>
</EuiTitle>
<EuiSpacer size="l" />
<EuiPageContentBody>
{spacesEnabled && (
<>
<EuiButtonEmpty onClick={() => setShowSyncFlyout(true)}>
{i18n.translate('xpack.ml.management.jobsList.syncFlyoutButton', {
defaultMessage: 'Synchronize saved objects',
})}
</EuiButtonEmpty>
{showSyncFlyout && <JobSpacesSyncFlyout onClose={onCloseSyncFlyout} />}
<EuiSpacer size="s" />
</>
)}
{renderTabs()}
</EuiPageContentBody>
</EuiPageContent>
</Router>
</ContextWrapper>
</KibanaContextProvider>
</I18nContext>
</RedirectAppLinks>
Expand Down
30 changes: 20 additions & 10 deletions x-pack/plugins/spaces/public/spaces_context/wrapper.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,30 @@
*/

import React, { useState, useEffect, PropsWithChildren, useMemo } from 'react';
import { StartServicesAccessor, CoreStart } from 'src/core/public';
import {
StartServicesAccessor,
DocLinksStart,
ApplicationStart,
NotificationsStart,
} from 'src/core/public';
import type { SpacesContextProps } from '../../../../../src/plugins/spaces_oss/public';
import { createSpacesReactContext } from './context';
import { PluginsStart } from '../plugin';
import { SpacesManager } from '../spaces_manager';
import { ShareToSpacesData, ShareToSpaceTarget } from '../types';
import { SpacesReactContext } from './types';

interface InternalProps {
spacesManager: SpacesManager;
getStartServices: StartServicesAccessor<PluginsStart>;
}

interface Services {
application: ApplicationStart;
docLinks: DocLinksStart;
notifications: NotificationsStart;
}

async function getShareToSpacesData(
spacesManager: SpacesManager,
feature?: string
Expand Down Expand Up @@ -47,26 +59,24 @@ async function getShareToSpacesData(
const SpacesContextWrapper = (props: PropsWithChildren<InternalProps & SpacesContextProps>) => {
const { spacesManager, getStartServices, feature, children } = props;

const [coreStart, setCoreStart] = useState<CoreStart>();
const [context, setContext] = useState<SpacesReactContext<Services> | undefined>();
const shareToSpacesDataPromise = useMemo(() => getShareToSpacesData(spacesManager, feature), [
spacesManager,
feature,
]);

useEffect(() => {
getStartServices().then(([coreStartValue]) => {
setCoreStart(coreStartValue);
getStartServices().then(([coreStart]) => {
const { application, docLinks, notifications } = coreStart;
const services = { application, docLinks, notifications };
setContext(createSpacesReactContext(services, spacesManager, shareToSpacesDataPromise));
});
}, [getStartServices]);
}, [getStartServices, shareToSpacesDataPromise, spacesManager]);

if (!coreStart) {
if (!context) {
return null;
}

const { application, docLinks, notifications } = coreStart;
const services = { application, docLinks, notifications };
const context = createSpacesReactContext(services, spacesManager, shareToSpacesDataPromise);

return <context.Provider>{children}</context.Provider>;
};

Expand Down

0 comments on commit d28bbda

Please sign in to comment.