Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[App Search] Convert Engine subnav and Engine Overview pages to new page template #102679

Merged
merged 6 commits into from
Jun 21, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,15 @@
*/

.appSearchNavEngineLabel {
padding-top: $euiSizeS;
margin-left: $euiSizeS;
padding-top: $euiSizeXS;
padding-bottom: $euiSizeS;

.euiText {
font-weight: $euiFontWeightMedium;
}
.euiBadge {
margin-top: $euiSizeXS;
}
}

.appSearchNavIcons {
// EUI override
&.euiFlexItem {
flex-grow: 0;
flex-direction: row;
}
.appSearchNavIcon {
order: 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀 👀 👀

Seeing order rules in CSS always raises the hairs on the back of my neck. I trust there's a good reason for this, but could you leave a small comment in the code explaining what that reason is?

Copy link
Member Author

@cee-chen cee-chen Jun 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch on this Byron! Kibana renders its icons to the left of text instead of to the right:

But we want the icons to the right, to match our previous UI:

If it had been more annoying than a simple CSS/flex re-order to do this I might have left them alone, but I figured since there's an easy way to quickly replicate our old nav icon UI/UX I might as well do so.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, if EUI or Davey pushes back on this / wants it removed, it's definitely easy enough to do so in the future!

Copy link
Member Author

@cee-chen cee-chen Jun 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, just saw the request was for a code comment 🤦‍♀️ hope this looks good!

Suggested change
order: 1;
// EuiSideNav renders icons to the left of the nav link by default, but we use icons
// as warning or error indicators & prefer to render them on the right side of the nav
order: 1;

}
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,14 @@
*/

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

jest.mock('../../../shared/layout', () => ({
...jest.requireActual('../../../shared/layout'), // TODO: Remove once side nav components are gone
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Layout and SideNav components will get deleted in one final PR (likely shared with WS and AS).

generateNavLink: jest.fn(({ to }) => ({ href: to })),
}));

import React from 'react';

import { shallow } from 'enzyme';
Expand All @@ -16,7 +22,305 @@ import { EuiBadge, EuiIcon } from '@elastic/eui';

import { rerender } from '../../../test_helpers';

import { EngineNav } from './engine_nav';
import { useEngineNav, EngineNav } from './engine_nav';

describe('useEngineNav', () => {
const values = { ...mockEngineValues, myRole: {}, dataLoading: false };
Comment on lines +27 to +28
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mostly copies the EngineNav test suite down below, it just converts the assertions from expecting react components to expecting EuiSideNavItem objects. (except for portions of the nav that render raw jsx)


beforeEach(() => {
setMockValues(values);
mockUseRouteMatch.mockReturnValue(true);
});

describe('returns empty', () => {
it('does not return engine nav items if not on an engine route', () => {
mockUseRouteMatch.mockReturnValueOnce(false);
expect(useEngineNav()).toBeUndefined();
});

it('does not return engine nav items if data is still loading', () => {
setMockValues({ ...values, dataLoading: true });
expect(useEngineNav()).toBeUndefined();
});

it('does not return engine nav items if engine data is missing', () => {
setMockValues({ ...values, engineName: '' });
expect(useEngineNav()).toBeUndefined();
});
});

describe('returns an array of EUI side nav items', () => {
const BASE_NAV = [
{
id: 'engineName',
name: 'some-engine',
renderItem: expect.any(Function),
'data-test-subj': 'EngineLabel',
},
{
id: 'overview',
name: 'Overview',
href: '/engines/some-engine',
'data-test-subj': 'EngineOverviewLink',
},
];

it('always returns an engine label and overview link', () => {
expect(useEngineNav()).toEqual(BASE_NAV);
});

describe('engine label', () => {
const renderEngineLabel = (engineNav: any) => {
return shallow(engineNav[0].renderItem() as any);
};

it('renders the capitalized engine name', () => {
const wrapper = renderEngineLabel(useEngineNav());
const name = wrapper.find('.eui-textTruncate');

expect(name.text()).toEqual('SOME-ENGINE');
expect(wrapper.find(EuiBadge)).toHaveLength(0);
});

it('renders a sample engine badge for the sample engine', () => {
setMockValues({ ...values, isSampleEngine: true });
const wrapper = renderEngineLabel(useEngineNav());

expect(wrapper.find(EuiBadge).prop('children')).toEqual('SAMPLE ENGINE');
});

it('renders a meta engine badge for meta engines', () => {
setMockValues({ ...values, isMetaEngine: true });
const wrapper = renderEngineLabel(useEngineNav());

expect(wrapper.find(EuiBadge).prop('children')).toEqual('META ENGINE');
});
});

it('returns an analytics nav item', () => {
setMockValues({ ...values, myRole: { canViewEngineAnalytics: true } });
expect(useEngineNav()).toEqual([
...BASE_NAV,
{
id: 'analytics',
name: 'Analytics',
href: '/engines/some-engine/analytics',
'data-test-subj': 'EngineAnalyticsLink',
},
]);
});

it('returns a documents nav item', () => {
setMockValues({ ...values, myRole: { canViewEngineDocuments: true } });
expect(useEngineNav()).toEqual([
...BASE_NAV,
{
id: 'documents',
name: 'Documents',
href: '/engines/some-engine/documents',
'data-test-subj': 'EngineDocumentsLink',
},
]);
});

it('returns a schema nav item', () => {
setMockValues({ ...values, myRole: { canViewEngineSchema: true } });
expect(useEngineNav()).toEqual([
...BASE_NAV,
{
id: 'schema',
name: 'Schema',
href: '/engines/some-engine/schema',
'data-test-subj': 'EngineSchemaLink',
icon: expect.anything(),
},
]);
});

describe('schema nav icons', () => {
const myRole = { canViewEngineSchema: true };

const renderIcons = (engineNav: any) => {
return shallow(<div>{engineNav[2].icon}</div>);
};

it('renders schema errors alert icon', () => {
setMockValues({ ...values, myRole, hasSchemaErrors: true });
const wrapper = renderIcons(useEngineNav());

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

it('renders unconfirmed schema fields info icon', () => {
setMockValues({ ...values, myRole, hasUnconfirmedSchemaFields: true });
const wrapper = renderIcons(useEngineNav());

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

it('renders schema conflicts alert icon', () => {
setMockValues({ ...values, myRole, hasSchemaConflicts: true });
const wrapper = renderIcons(useEngineNav());

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

describe('crawler', () => {
const myRole = { canViewEngineCrawler: true };

it('returns a crawler nav item', () => {
setMockValues({ ...values, myRole });
expect(useEngineNav()).toEqual([
...BASE_NAV,
{
id: 'crawler',
name: 'Web Crawler',
href: '/engines/some-engine/crawler',
'data-test-subj': 'EngineCrawlerLink',
},
]);
});

it('does not return a crawler nav item for meta engines', () => {
setMockValues({ ...values, myRole, isMetaEngine: true });
expect(useEngineNav()).toEqual(BASE_NAV);
});
});

describe('meta engine source engines', () => {
const myRole = { canViewMetaEngineSourceEngines: true };

it('returns a source engines nav item', () => {
setMockValues({ ...values, myRole, isMetaEngine: true });
expect(useEngineNav()).toEqual([
...BASE_NAV,
{
id: 'sourceEngines',
name: 'Engines',
href: '/engines/some-engine/engines',
'data-test-subj': 'MetaEngineEnginesLink',
},
]);
});

it('does not return a source engines nav item for non-meta engines', () => {
setMockValues({ ...values, myRole, isMetaEngine: false });
expect(useEngineNav()).toEqual(BASE_NAV);
});
});

it('returns a relevance tuning nav item', () => {
setMockValues({ ...values, myRole: { canManageEngineRelevanceTuning: true } });
expect(useEngineNav()).toEqual([
...BASE_NAV,
{
id: 'relevanceTuning',
name: 'Relevance Tuning',
href: '/engines/some-engine/relevance_tuning',
'data-test-subj': 'EngineRelevanceTuningLink',
icon: expect.anything(),
},
]);
});

describe('relevance tuning nav icons', () => {
const myRole = { canManageEngineRelevanceTuning: true };

const renderIcons = (engineNav: any) => {
return shallow(<div>{engineNav[2].icon}</div>);
};

it('renders unconfirmed schema fields info icon', () => {
setMockValues({ ...values, myRole, engine: { unsearchedUnconfirmedFields: true } });
const wrapper = renderIcons(useEngineNav());
expect(
wrapper.find('[data-test-subj="EngineNavRelevanceTuningUnsearchedFields"]')
).toHaveLength(1);
});

it('renders schema conflicts alert icon', () => {
setMockValues({ ...values, myRole, engine: { invalidBoosts: true } });
const wrapper = renderIcons(useEngineNav());
expect(
wrapper.find('[data-test-subj="EngineNavRelevanceTuningInvalidBoosts"]')
).toHaveLength(1);
});

it('can render multiple icons', () => {
const engine = { invalidBoosts: true, unsearchedUnconfirmedFields: true };
setMockValues({ ...values, myRole, engine });
const wrapper = renderIcons(useEngineNav());
expect(wrapper.find(EuiIcon)).toHaveLength(2);
});
});

it('returns a synonyms nav item', () => {
setMockValues({ ...values, myRole: { canManageEngineSynonyms: true } });
expect(useEngineNav()).toEqual([
...BASE_NAV,
{
id: 'synonyms',
name: 'Synonyms',
href: '/engines/some-engine/synonyms',
'data-test-subj': 'EngineSynonymsLink',
},
]);
});

it('returns a curations nav item', () => {
setMockValues({ ...values, myRole: { canManageEngineCurations: true } });
expect(useEngineNav()).toEqual([
...BASE_NAV,
{
id: 'curations',
name: 'Curations',
href: '/engines/some-engine/curations',
'data-test-subj': 'EngineCurationsLink',
},
]);
});

it('returns a results settings nav item', () => {
setMockValues({ ...values, myRole: { canManageEngineResultSettings: true } });
expect(useEngineNav()).toEqual([
...BASE_NAV,
{
id: 'resultSettings',
name: 'Result Settings',
href: '/engines/some-engine/result_settings',
'data-test-subj': 'EngineResultSettingsLink',
},
]);
});

it('returns a Search UI nav item', () => {
setMockValues({ ...values, myRole: { canManageEngineSearchUi: true } });
expect(useEngineNav()).toEqual([
...BASE_NAV,
{
id: 'searchUI',
name: 'Search UI',
href: '/engines/some-engine/search_ui',
'data-test-subj': 'EngineSearchUILink',
},
]);
});

it('returns an API logs nav item', () => {
setMockValues({ ...values, myRole: { canViewEngineApiLogs: true } });
expect(useEngineNav()).toEqual([
...BASE_NAV,
{
id: 'apiLogs',
name: 'API Logs',
href: '/engines/some-engine/api_logs',
'data-test-subj': 'EngineAPILogsLink',
},
]);
});
});
});

describe('EngineNav', () => {
const values = { ...mockEngineValues, myRole: {}, dataLoading: false };
Expand Down
Loading