Skip to content

Commit

Permalink
[Workplace Search] Convert Sources pages to new page template (+ pers…
Browse files Browse the repository at this point in the history
…onal dashboard) (elastic#102592)

* Refactor PersonalDashboardLayout to more closely match new page template

- Remove references to enterpriseSearchLayout CSS (which will be removed in an upcoming PR)

- Prefer to lean more heavily on default EuiPage props/CSS/etc.

- Handle conditional sidebar logic in this layout rather than passing it in as a prop

- Update props & DRY concerns to more closely match WorkplaceSearchPageTemplate
  - e.g. isLoading & pageChrome (mostly for document titles)
  - make FlashMessage and readOnlyMode work OOTB w/o props)

* Convert Source subnav to EuiSideNav format

+ update PrivateSourcesSidebar to use EuiSIdeNav

* Update routers

- removing wrapping layouts, flash messages, chrome/telemetry

* Refactor SourceRouter into shared layout component

- Remove license callout, page header, and page chrome/telemetry

- NOTE: The early page isLoading behavior (lines 51-) is required to prevent a flash of a completely empty page (instead of preserving the layout/side nav while loading). We cannot let the page fall through to the route because some routes are conditionally rendered based on isCustomSource.

- FWIW: App Search has a similar isLoading early return with its Engine sub nav, and also a similar AnalyticsLayout for DRYing out repeated concerns/UI elements within Analytics subroutes.

* Convert all single source views to new page template

- Mostly removing isLoading tests
- NOTE: Schema page could *possibly* use the new isEmptyState/emptyState page template props, but would need some layout reshuffling

* Convert Add Source pages to conditional page templates

- Opted to give these pages their own conditional layout logic - this could possibly be DRY'd out

- There is possibly extra cleanup here on this file that could have been done (e.g. empty state, titles, etc.) in light of the new templates - but I didn't want to spend extra time here and went with creating as few diffs as possible

* Convert separate Organization Sources & Private Sources views to new page templates

+ fix Link to EuiButtonTo on Organization Sources view

* Update Account Settings with personal layout + write tests

+ add related KibanaLogic branch coverage

* [UX feedback] Do not render page headers while loading on Overview & Sources pages

* [PR feedback] Breadcrumb errors/fallbacks

* [Proposal] Update schema errors routing to better work with nav/breadcrumbs

- `exact` is required to make the parent schemas/ not gobble schema/{errorId}

- added bonus breadcrumb for nicer schema navigation UX

- No tests need to update AFAICT

* Ignore Typescript error on soon-to-come EUI prop
# Conflicts:
#	x-pack/plugins/enterprise_search/public/applications/workplace_search/views/content_sources/components/source_settings.tsx
  • Loading branch information
Constance authored and cee-chen committed Jun 22, 2021
1 parent 7f7ba0b commit 5d3b193
Show file tree
Hide file tree
Showing 42 changed files with 881 additions and 552 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ describe('KibanaLogic', () => {
expect(KibanaLogic.values.config).toEqual({});
});

it('gracefully handles disabled security', () => {
mountKibanaLogic({ ...mockKibanaValues, security: undefined } as any);

expect(KibanaLogic.values.security).toEqual({});
});

it('gracefully handles non-cloud installs', () => {
mountKibanaLogic({ ...mockKibanaValues, cloud: undefined } as any);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ jest.mock('../../../shared/layout', () => ({
...jest.requireActual('../../../shared/layout'),
generateNavLink: jest.fn(({ to }) => ({ href: to })),
}));
jest.mock('../../views/content_sources/components/source_sub_nav', () => ({
useSourceSubNav: () => [],
}));
jest.mock('../../views/groups/components/group_sub_nav', () => ({
useGroupSubNav: () => [],
}));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
GROUPS_PATH,
ORG_SETTINGS_PATH,
} from '../../routes';
import { useSourceSubNav } from '../../views/content_sources/components/source_sub_nav';
import { useGroupSubNav } from '../../views/groups/components/group_sub_nav';
import { useSettingsSubNav } from '../../views/settings/components/settings_sub_nav';

Expand All @@ -33,7 +34,7 @@ export const useWorkplaceSearchNav = () => {
id: 'sources',
name: NAV.SOURCES,
...generateNavLink({ to: SOURCES_PATH }),
items: [], // TODO: Source subnav
items: useSourceSubNav(),
},
{
id: 'groups',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,20 @@
*/

.personalDashboardLayout {
$sideBarWidth: $euiSize * 30;
$consoleHeaderHeight: 48px; // NOTE: Keep an eye on this for changes
$pageHeight: calc(100vh - #{$consoleHeaderHeight});
&__sideBar {
padding: $euiSizeXL $euiSizeXXL $euiSizeXXL;

left: $sideBarWidth;
width: calc(100% - #{$sideBarWidth});
min-height: $pageHeight;
@include euiBreakpoint('m', 'l') {
min-width: $euiSize * 20;
}
@include euiBreakpoint('xl') {
min-width: $euiSize * 30;
}
}

&__sideBar {
padding: 32px 40px 40px;
width: $sideBarWidth;
margin-left: -$sideBarWidth;
height: $pageHeight;
&__body {
position: relative;
width: 100%;
height: 100%;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,37 +5,102 @@
* 2.0.
*/

import { setMockValues } from '../../../../__mocks__/kea_logic';
import { mockUseRouteMatch } from '../../../../__mocks__/react_router';

import React from 'react';

import { shallow } from 'enzyme';

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

import { AccountHeader } from '..';
import { FlashMessages } from '../../../../shared/flash_messages';
import { SetWorkplaceSearchChrome } from '../../../../shared/kibana_chrome';
import { Loading } from '../../../../shared/loading';

import { AccountHeader, AccountSettingsSidebar, PrivateSourcesSidebar } from '../index';

import { PersonalDashboardLayout } from './personal_dashboard_layout';

describe('PersonalDashboardLayout', () => {
const children = <p data-test-subj="TestChildren">test</p>;
const sidebar = <p data-test-subj="TestSidebar">test</p>;

beforeEach(() => {
jest.clearAllMocks();
setMockValues({ readOnlyMode: false });
});

it('renders', () => {
const wrapper = shallow(
<PersonalDashboardLayout sidebar={sidebar}>{children}</PersonalDashboardLayout>
);
const wrapper = shallow(<PersonalDashboardLayout>{children}</PersonalDashboardLayout>);

expect(wrapper.find('[data-test-subj="TestChildren"]')).toHaveLength(1);
expect(wrapper.find('[data-test-subj="TestSidebar"]')).toHaveLength(1);
expect(wrapper.find('.personalDashboardLayout')).toHaveLength(1);
expect(wrapper.find(AccountHeader)).toHaveLength(1);
expect(wrapper.find(FlashMessages)).toHaveLength(1);
});

it('renders callout when in read-only mode', () => {
describe('renders sidebar content based on the route', () => {
it('renders the private sources sidebar on the private sources path', () => {
(mockUseRouteMatch as jest.Mock).mockImplementation((path: string) => path === '/p/sources');
const wrapper = shallow(<PersonalDashboardLayout>{children}</PersonalDashboardLayout>);

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

it('renders the account settings sidebar on the account settings path', () => {
(mockUseRouteMatch as jest.Mock).mockImplementation((path: string) => path === '/p/settings');
const wrapper = shallow(<PersonalDashboardLayout>{children}</PersonalDashboardLayout>);

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

it('does not render a sidebar if not on a valid personal dashboard path', () => {
(mockUseRouteMatch as jest.Mock).mockImplementation((path: string) => path === '/test');
const wrapper = shallow(<PersonalDashboardLayout>{children}</PersonalDashboardLayout>);

expect(wrapper.find(AccountSettingsSidebar)).toHaveLength(0);
expect(wrapper.find(PrivateSourcesSidebar)).toHaveLength(0);
});
});

describe('loading state', () => {
it('renders a loading icon in place of children', () => {
const wrapper = shallow(
<PersonalDashboardLayout isLoading>{children}</PersonalDashboardLayout>
);

expect(wrapper.find(Loading)).toHaveLength(1);
expect(wrapper.find('[data-test-subj="TestChildren"]')).toHaveLength(0);
});

it('renders children & does not render a loading icon when the page is done loading', () => {
const wrapper = shallow(
<PersonalDashboardLayout isLoading={false}>{children}</PersonalDashboardLayout>
);

expect(wrapper.find(Loading)).toHaveLength(0);
expect(wrapper.find('[data-test-subj="TestChildren"]')).toHaveLength(1);
});
});

it('sets WS page chrome (primarily document title)', () => {
const wrapper = shallow(
<PersonalDashboardLayout sidebar={sidebar} readOnlyMode>
<PersonalDashboardLayout pageChrome={['Sources', 'Add source', 'Gmail']}>
{children}
</PersonalDashboardLayout>
);

expect(wrapper.find(SetWorkplaceSearchChrome).prop('trail')).toEqual([
'Sources',
'Add source',
'Gmail',
]);
});

it('renders callout when in read-only mode', () => {
setMockValues({ readOnlyMode: true });
const wrapper = shallow(<PersonalDashboardLayout>{children}</PersonalDashboardLayout>);

expect(wrapper.find(EuiCallOut)).toHaveLength(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,44 +6,67 @@
*/

import React from 'react';
import { useRouteMatch } from 'react-router-dom';

import { EuiPage, EuiPageSideBar, EuiPageBody, EuiCallOut } from '@elastic/eui';
import { useValues } from 'kea';

import { AccountHeader } from '..';
import {
EuiPage,
EuiPageSideBar,
EuiPageBody,
EuiPageContentBody,
EuiCallOut,
EuiSpacer,
} from '@elastic/eui';

import { FlashMessages } from '../../../../shared/flash_messages';
import { HttpLogic } from '../../../../shared/http';
import { SetWorkplaceSearchChrome } from '../../../../shared/kibana_chrome';
import { BreadcrumbTrail } from '../../../../shared/kibana_chrome/generate_breadcrumbs';
import { Loading } from '../../../../shared/loading';

import { PERSONAL_SOURCES_PATH, PERSONAL_SETTINGS_PATH } from '../../../routes';
import { PRIVATE_DASHBOARD_READ_ONLY_MODE_WARNING } from '../../../views/content_sources/constants';
import { AccountHeader, AccountSettingsSidebar, PrivateSourcesSidebar } from '../index';

import './personal_dashboard_layout.scss';

interface LayoutProps {
restrictWidth?: boolean;
readOnlyMode?: boolean;
sidebar: React.ReactNode;
isLoading?: boolean;
pageChrome?: BreadcrumbTrail;
}

export const PersonalDashboardLayout: React.FC<LayoutProps> = ({
children,
restrictWidth,
readOnlyMode,
sidebar,
isLoading,
pageChrome,
}) => {
const { readOnlyMode } = useValues(HttpLogic);

return (
<>
{pageChrome && <SetWorkplaceSearchChrome trail={pageChrome} />}
<AccountHeader />
<EuiPage className="enterpriseSearchLayout personalDashboardLayout">
<EuiPageSideBar className="enterpriseSearchLayout__sideBar personalDashboardLayout__sideBar">
{sidebar}
<EuiPage className="personalDashboardLayout" paddingSize="none">
<EuiPageSideBar className="personalDashboardLayout__sideBar" sticky>
{useRouteMatch(PERSONAL_SOURCES_PATH) && <PrivateSourcesSidebar />}
{useRouteMatch(PERSONAL_SETTINGS_PATH) && <AccountSettingsSidebar />}
</EuiPageSideBar>
<EuiPageBody className="enterpriseSearchLayout__body" restrictWidth={restrictWidth}>
{readOnlyMode && (
<EuiCallOut
className="enterpriseSearchLayout__readOnlyMode"
color="warning"
iconType="lock"
title={PRIVATE_DASHBOARD_READ_ONLY_MODE_WARNING}
/>
)}
{children}
<EuiPageBody component="main" panelled>
<EuiPageContentBody className="personalDashboardLayout__body" restrictWidth>
{readOnlyMode && (
<>
<EuiCallOut
color="warning"
iconType="lock"
title={PRIVATE_DASHBOARD_READ_ONLY_MODE_WARNING}
/>
<EuiSpacer />
</>
)}
<FlashMessages />
{isLoading ? <Loading /> : children}
</EuiPageContentBody>
</EuiPageBody>
</EuiPage>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,22 @@

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

jest.mock('../../../views/content_sources/components/source_sub_nav', () => ({
useSourceSubNav: () => [],
}));

import React from 'react';

import { shallow } from 'enzyme';

import { EuiSideNav } from '@elastic/eui';

import {
PRIVATE_CAN_CREATE_PAGE_TITLE,
PRIVATE_VIEW_ONLY_PAGE_TITLE,
PRIVATE_VIEW_ONLY_PAGE_DESCRIPTION,
PRIVATE_CAN_CREATE_PAGE_DESCRIPTION,
} from '../../../constants';
import { SourceSubNav } from '../../../views/content_sources/components/source_sub_nav';

import { ViewContentHeader } from '../../shared/view_content_header';

Expand All @@ -26,6 +31,7 @@ import { PrivateSourcesSidebar } from './private_sources_sidebar';
describe('PrivateSourcesSidebar', () => {
const mockValues = {
account: { canCreatePersonalSources: true },
contentSource: {},
};

beforeEach(() => {
Expand All @@ -36,25 +42,42 @@ describe('PrivateSourcesSidebar', () => {
const wrapper = shallow(<PrivateSourcesSidebar />);

expect(wrapper.find(ViewContentHeader)).toHaveLength(1);
expect(wrapper.find(SourceSubNav)).toHaveLength(1);
});

it('uses correct title and description when private sources are enabled', () => {
const wrapper = shallow(<PrivateSourcesSidebar />);
describe('header text', () => {
it('uses correct title and description when private sources are enabled', () => {
const wrapper = shallow(<PrivateSourcesSidebar />);

expect(wrapper.find(ViewContentHeader).prop('title')).toEqual(PRIVATE_CAN_CREATE_PAGE_TITLE);
expect(wrapper.find(ViewContentHeader).prop('description')).toEqual(
PRIVATE_CAN_CREATE_PAGE_DESCRIPTION
);
});

expect(wrapper.find(ViewContentHeader).prop('title')).toEqual(PRIVATE_CAN_CREATE_PAGE_TITLE);
expect(wrapper.find(ViewContentHeader).prop('description')).toEqual(
PRIVATE_CAN_CREATE_PAGE_DESCRIPTION
);
it('uses correct title and description when private sources are disabled', () => {
setMockValues({ ...mockValues, account: { canCreatePersonalSources: false } });
const wrapper = shallow(<PrivateSourcesSidebar />);

expect(wrapper.find(ViewContentHeader).prop('title')).toEqual(PRIVATE_VIEW_ONLY_PAGE_TITLE);
expect(wrapper.find(ViewContentHeader).prop('description')).toEqual(
PRIVATE_VIEW_ONLY_PAGE_DESCRIPTION
);
});
});

it('uses correct title and description when private sources are disabled', () => {
setMockValues({ account: { canCreatePersonalSources: false } });
const wrapper = shallow(<PrivateSourcesSidebar />);
describe('sub nav', () => {
it('renders a side nav when viewing a single source', () => {
setMockValues({ ...mockValues, contentSource: { id: '1', name: 'test source' } });
const wrapper = shallow(<PrivateSourcesSidebar />);

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

it('does not render a side nav if not on a source page', () => {
setMockValues({ ...mockValues, contentSource: {} });
const wrapper = shallow(<PrivateSourcesSidebar />);

expect(wrapper.find(ViewContentHeader).prop('title')).toEqual(PRIVATE_VIEW_ONLY_PAGE_TITLE);
expect(wrapper.find(ViewContentHeader).prop('description')).toEqual(
PRIVATE_VIEW_ONLY_PAGE_DESCRIPTION
);
expect(wrapper.find(EuiSideNav)).toHaveLength(0);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,17 @@ import React from 'react';

import { useValues } from 'kea';

import { EuiSideNav } from '@elastic/eui';

import { AppLogic } from '../../../app_logic';
import {
PRIVATE_CAN_CREATE_PAGE_TITLE,
PRIVATE_VIEW_ONLY_PAGE_TITLE,
PRIVATE_VIEW_ONLY_PAGE_DESCRIPTION,
PRIVATE_CAN_CREATE_PAGE_DESCRIPTION,
} from '../../../constants';
import { SourceSubNav } from '../../../views/content_sources/components/source_sub_nav';
import { useSourceSubNav } from '../../../views/content_sources/components/source_sub_nav';
import { SourceLogic } from '../../../views/content_sources/source_logic';
import { ViewContentHeader } from '../../shared/view_content_header';

export const PrivateSourcesSidebar = () => {
Expand All @@ -31,10 +34,17 @@ export const PrivateSourcesSidebar = () => {
? PRIVATE_CAN_CREATE_PAGE_DESCRIPTION
: PRIVATE_VIEW_ONLY_PAGE_DESCRIPTION;

const {
contentSource: { id = '', name = '' },
} = useValues(SourceLogic);

const navItems = [{ id, name, items: useSourceSubNav() }];

return (
<>
<ViewContentHeader title={PAGE_TITLE} description={PAGE_DESCRIPTION} />
<SourceSubNav />
{/* @ts-expect-error: TODO, uncomment this once EUI 34.x lands in Kibana & `mobileBreakpoints` is a valid prop */}
{id && <EuiSideNav items={navItems} mobileBreakpoints={[]} />}
</>
);
};
Loading

0 comments on commit 5d3b193

Please sign in to comment.