Skip to content

Commit

Permalink
[App Search] Engines Overview polish pass (#102778)
Browse files Browse the repository at this point in the history
* Split up engines vs. meta engines into separate panels

- per Davey's feedback from earlier UI passes

* DRY out manual header/spacing to reusable DataPanel component

+ update DataPanel icon typing to not error when passed a custom icon/svg

- kudos again to Davey for the component

* Typography tweaks

- Update DataPanel component to accept a custom titleSize (to maintain previous UI/sizing)

- Fix meta engines empty prompt title heading to follow heading levels + tweak sizing to not be larger than panel heading

* Set up new license CTA button for upcoming meta engines CTA

falls back to a documentation link! so fancy

* Update Enterprise Search Overview to use new license button

* Add new Meta Engines license upgrade CTA

- Reuse some copy from Meta Engines creation view
- Reuse DataPanel so visuals stay consistent + it looks similar to CTA on Enterprise Search Overview
- Update DataPanel to allow buttons to be responsive + conditionally load spacer between header & children

* Improve responsiveness of app when platinum license changes

Previously, routes/apps were going off the static data passed from the server which was only initialized once on page load. hasPlatinumLicense however changes dynamically and in real-time, removing the need for a hard page refresh.

I could have replaced all `canManageMetaEngine` flags with `isPlatinum && canManageEngines`, but I thought baking license checks into the main ability would be more scalable and potentially open the way to other license-based flags also being dynamic.

* [PR feedback] Typos in test names

Co-authored-by: Jason Stoltzfus <jastoltz24@gmail.com>

* Fix failing test

Missed updating the heading level

Co-authored-by: Jason Stoltzfus <jastoltz24@gmail.com>
  • Loading branch information
Constance and JasonStoltz committed Jun 21, 2021
1 parent 62eece5 commit 6dc9962
Show file tree
Hide file tree
Showing 21 changed files with 337 additions and 141 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@ export const mockLicensingValues = {
license: licensingMock.createLicense(),
hasPlatinumLicense: false,
hasGoldLicense: false,
isTrial: false,
canManageLicense: true,
};

jest.mock('../../shared/licensing', () => ({
...(jest.requireActual('../../shared/licensing') as object),
LicensingLogic: { values: mockLicensingValues },
}));
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
*/

import { DEFAULT_INITIAL_APP_DATA } from '../../../common/__mocks__';
import { LogicMounter } from '../__mocks__/kea_logic';
import { LogicMounter } from '../__mocks__/kea_logic/logic_mounter.test_helper';

jest.mock('../shared/licensing', () => ({
LicensingLogic: { selectors: { hasPlatinumLicense: () => false } },
}));

import { AppLogic } from './app_logic';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { kea, MakeLogicType } from 'kea';

import { InitialAppData } from '../../../common/types';

import { LicensingLogic } from '../shared/licensing';

import { ConfiguredLimits, Account, Role } from './types';

import { getRoleAbilities } from './utils/role';
Expand Down Expand Up @@ -43,8 +45,8 @@ export const AppLogic = kea<MakeLogicType<AppValues, AppActions, Required<Initia
}),
selectors: {
myRole: [
(selectors) => [selectors.account],
({ role }) => (role ? getRoleAbilities(role) : {}),
(selectors) => [selectors.account, LicensingLogic.selectors.hasPlatinumLicense],
({ role }, hasPlatinumLicense) => (role ? getRoleAbilities(role, hasPlatinumLicense) : {}),
],
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import React from 'react';

import { shallow } from 'enzyme';

import { EuiIcon, EuiButton } from '@elastic/eui';
import { EuiIcon, EuiButton, EuiTitle, EuiFlexGroup, EuiSpacer } from '@elastic/eui';

import { LoadingOverlay } from '../../../shared/loading';

Expand All @@ -27,6 +27,16 @@ describe('DataPanel', () => {
expect(wrapper.find('[data-test-subj="children"]').text()).toEqual('Look at this graph');
});

it('conditionally renders a spacer between the header and children', () => {
const wrapper = shallow(<DataPanel title={<h1>Test</h1>} />);

expect(wrapper.find(EuiSpacer)).toHaveLength(0);

wrapper.setProps({ children: 'hello world' });

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

describe('components', () => {
it('renders with an icon', () => {
const wrapper = shallow(<DataPanel title={<h1>The Smoke Monster</h1>} iconType="eye" />);
Expand Down Expand Up @@ -70,6 +80,26 @@ describe('DataPanel', () => {
});

describe('props', () => {
it('passes titleSize to the title', () => {
const wrapper = shallow(<DataPanel title={<h2>Test</h2>} />);

expect(wrapper.find(EuiTitle).prop('size')).toEqual('xs'); // Default

wrapper.setProps({ titleSize: 's' });

expect(wrapper.find(EuiTitle).prop('size')).toEqual('s');
});

it('passes responsive to the header flex group', () => {
const wrapper = shallow(<DataPanel title={<h1>Test</h1>} />);

expect(wrapper.find(EuiFlexGroup).first().prop('responsive')).toEqual(false);

wrapper.setProps({ responsive: true });

expect(wrapper.find(EuiFlexGroup).first().prop('responsive')).toEqual(true);
});

it('renders panel color based on filled flag', () => {
const wrapper = shallow(<DataPanel title={<h1>Test</h1>} />);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@ import {
EuiFlexGroup,
EuiFlexItem,
EuiIcon,
EuiIconProps,
EuiPanel,
EuiSpacer,
EuiText,
EuiTitle,
EuiTitleProps,
} from '@elastic/eui';

import { LoadingOverlay } from '../../../shared/loading';
Expand All @@ -25,9 +27,11 @@ import './data_panel.scss';

interface Props {
title: React.ReactElement; // e.g., h2 tag
subtitle?: string;
iconType?: string;
titleSize?: EuiTitleProps['size'];
subtitle?: React.ReactNode;
iconType?: EuiIconProps['type'];
action?: React.ReactNode;
responsive?: boolean;
filled?: boolean;
hasBorder?: boolean;
isLoading?: boolean;
Expand All @@ -36,9 +40,11 @@ interface Props {

export const DataPanel: React.FC<Props> = ({
title,
titleSize = 'xs',
subtitle,
iconType,
action,
responsive = false,
filled,
hasBorder,
isLoading,
Expand All @@ -59,7 +65,7 @@ export const DataPanel: React.FC<Props> = ({
hasShadow={false}
aria-busy={isLoading}
>
<EuiFlexGroup justifyContent="spaceBetween" alignItems="center" responsive={false}>
<EuiFlexGroup justifyContent="spaceBetween" alignItems="center" responsive={responsive}>
<EuiFlexItem>
<EuiFlexGroup gutterSize="s" alignItems="center" responsive={false}>
{iconType && (
Expand All @@ -68,7 +74,7 @@ export const DataPanel: React.FC<Props> = ({
</EuiFlexItem>
)}
<EuiFlexItem>
<EuiTitle size="xs">{title}</EuiTitle>
<EuiTitle size={titleSize}>{title}</EuiTitle>
</EuiFlexItem>
</EuiFlexGroup>
{subtitle && (
Expand All @@ -79,8 +85,12 @@ export const DataPanel: React.FC<Props> = ({
</EuiFlexItem>
{action && <EuiFlexItem grow={false}>{action}</EuiFlexItem>}
</EuiFlexGroup>
<EuiSpacer />
{children}
{children && (
<>
<EuiSpacer />
{children}
</>
)}
{isLoading && <LoadingOverlay />}
</EuiPanel>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('EmptyMetaEnginesState', () => {
.find(EuiEmptyPrompt)
.dive();

expect(wrapper.find('h2').text()).toEqual('Create your first meta engine');
expect(wrapper.find('h3').text()).toEqual('Create your first meta engine');
expect(wrapper.find(EuiButton).prop('href')).toEqual(
expect.stringContaining('/meta-engines-guide.html')
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@ import { DOCS_PREFIX } from '../../../routes';
export const EmptyMetaEnginesState: React.FC = () => (
<EuiEmptyPrompt
title={
<h2>
<h3>
{i18n.translate('xpack.enterpriseSearch.appSearch.engines.metaEngines.emptyPromptTitle', {
defaultMessage: 'Create your first meta engine',
})}
</h2>
</h3>
}
titleSize="s"
body={
<p>
{i18n.translate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,17 @@
* 2.0.
*/

import React from 'react';

import { EuiLink } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';

import { DOCS_PREFIX } from '../../routes';
import {
META_ENGINE_CREATION_FORM_META_ENGINE_DESCRIPTION,
META_ENGINE_CREATION_FORM_DOCUMENTATION_LINK,
} from '../meta_engine_creation/constants';

export const ENGINES_TITLE = i18n.translate('xpack.enterpriseSearch.appSearch.engines.title', {
defaultMessage: 'Engines',
Expand All @@ -21,6 +31,24 @@ export const META_ENGINES_TITLE = i18n.translate(
{ defaultMessage: 'Meta Engines' }
);

export const META_ENGINES_DESCRIPTION = (
<>
{META_ENGINE_CREATION_FORM_META_ENGINE_DESCRIPTION}
<br />
<FormattedMessage
id="xpack.enterpriseSearch.appSearch.metaEngines.upgradeDescription"
defaultMessage="{readDocumentationLink} for more information or upgrade to a Platinum license to get started."
values={{
readDocumentationLink: (
<EuiLink href={`${DOCS_PREFIX}/meta-engines-guide.html`} target="_blank">
{META_ENGINE_CREATION_FORM_DOCUMENTATION_LINK}
</EuiLink>
),
}}
/>
</>
);

export const SOURCE_ENGINES_TITLE = i18n.translate(
'xpack.enterpriseSearch.appSearch.enginesOverview.metaEnginesTable.sourceEngines.title',
{ defaultMessage: 'Source Engines' }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('EnginesOverview', () => {
metaEnginesLoading: false,
hasPlatinumLicense: false,
// AppLogic
myRole: { canManageEngines: false },
myRole: { canManageEngines: false, canManageMetaEngines: false },
// MetaEnginesTableLogic
expandedSourceEngines: {},
conflictingEnginesSets: {},
Expand Down Expand Up @@ -85,17 +85,25 @@ describe('EnginesOverview', () => {
expect(actions.loadEngines).toHaveBeenCalled();
});

describe('when the user can manage/create engines', () => {
it('renders a create engine button which takes users to the create engine page', () => {
describe('engine creation', () => {
it('renders a create engine action when the users can create engines', () => {
setMockValues({
...valuesWithEngines,
myRole: { canManageEngines: true },
});
const wrapper = shallow(<EnginesOverview />);

expect(
wrapper.find('[data-test-subj="appSearchEnginesEngineCreationButton"]').prop('to')
).toEqual('/engine_creation');
expect(wrapper.find('[data-test-subj="appSearchEngines"]').prop('action')).toBeTruthy();
});

it('does not render a create engine action if the user cannot create engines', () => {
setMockValues({
...valuesWithEngines,
myRole: { canManageEngines: false },
});
const wrapper = shallow(<EnginesOverview />);

expect(wrapper.find('[data-test-subj="appSearchEngines"]').prop('action')).toBeFalsy();
});
});

Expand All @@ -111,19 +119,41 @@ describe('EnginesOverview', () => {
expect(actions.loadMetaEngines).toHaveBeenCalled();
});

describe('when the user can manage/create engines', () => {
it('renders a create engine button which takes users to the create meta engine page', () => {
describe('meta engine creation', () => {
it('renders a create meta engine action when the user can create meta engines', () => {
setMockValues({
...valuesWithEngines,
hasPlatinumLicense: true,
myRole: { canManageEngines: true },
myRole: { canManageMetaEngines: true },
});
const wrapper = shallow(<EnginesOverview />);

expect(
wrapper.find('[data-test-subj="appSearchEnginesMetaEngineCreationButton"]').prop('to')
).toEqual('/meta_engine_creation');
expect(wrapper.find('[data-test-subj="appSearchMetaEngines"]').prop('action')).toBeTruthy();
});

it('does not render a create meta engine action if user cannot create meta engines', () => {
setMockValues({
...valuesWithEngines,
hasPlatinumLicense: true,
myRole: { canManageMetaEngines: false },
});
const wrapper = shallow(<EnginesOverview />);

expect(wrapper.find('[data-test-subj="appSearchMetaEngines"]').prop('action')).toBeFalsy();
});
});
});

describe('when an account does not have a platinum license', () => {
it('renders a license call to action in place of the meta engines table', () => {
setMockValues({
...valuesWithEngines,
hasPlatinumLicense: false,
});
const wrapper = shallow(<EnginesOverview />);

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

Expand Down
Loading

0 comments on commit 6dc9962

Please sign in to comment.