Skip to content

[Lens] Make breadcrumbs look and feel like Visualize #44258

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.

Sign up for GitHub

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

Merged
chrisdavies merged 3 commits into from
Sep 4, 2019
Merged

[Lens] Make breadcrumbs look and feel like Visualize #44258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
24a98cd
Make Lens breadcrumbs match Visualize, so
chrisdavies Aug 28, 2019
1f12180
Test breadcrumb effect
chrisdavies Aug 30, 2019
c48fa23
Fix TypeScript errors
chrisdavies Sep 3, 2019
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
43 changes: 42 additions & 1 deletion x-pack/legacy/plugins/lens/public/app_plugin/app.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import React from 'react';
import { App } from './app';
import { EditorFrameInstance } from '../types';

import { Chrome } from 'ui/chrome';
import { toastNotifications } from 'ui/notify';
import { Storage } from 'ui/storage';
Expand Down Expand Up @@ -44,6 +43,8 @@ function makeDefaultArgs(): jest.Mocked<{
return ({
editorFrame: createMockFrame(),
chrome: {
addBasePath: (s: string) => `/testbasepath/${s}`,
breadcrumbs: { set: jest.fn() },
getUiSettingsClient() {
return {
get: jest.fn(type => {
Expand Down Expand Up @@ -114,6 +115,46 @@ describe('Lens App', () => {
`);
});

it('sets breadcrumbs when the document title changes', async () => {
const mockSet = jest.fn();
const defaultArgs = makeDefaultArgs();
const args = {
...defaultArgs,
chrome: ({
...defaultArgs.chrome,
addBasePath: jest.fn(s => `/testbasepath${s}`),
breadcrumbs: {
...defaultArgs.chrome.breadcrumbs,
set: mockSet,
},
} as unknown) as Chrome,
};

const instance = mount(<App {...args} />);

expect(mockSet).toHaveBeenCalledWith([
{ text: 'Visualize', href: '/testbasepath/app/kibana#/visualize' },
{ text: 'Create' },
]);

(args.docStorage.load as jest.Mock).mockResolvedValue({
id: '1234',
title: 'Daaaaaaadaumching!',
state: {
query: 'fake query',
datasourceMetaData: { filterableIndexPatterns: [{ id: '1', title: 'saved' }] },
},
});

instance.setProps({ docId: '1234' });
await waitForPromises();

expect(mockSet).toHaveBeenCalledWith([
{ text: 'Visualize', href: '/testbasepath/app/kibana#/visualize' },
{ text: 'Daaaaaaadaumching!' },
]);
});
Copy link
Copy Markdown
Contributor

@wylieconlon wylieconlon Aug 29, 2019

Choose a reason for hiding this comment

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

I don't see a test for the useEffect you added, which would be if the persisted title is changed, the breadcrumb should change.

Copy link
Copy Markdown
Contributor Author

@chrisdavies chrisdavies Aug 30, 2019

Choose a reason for hiding this comment

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

Yeah... It's true. It's clumsy to test, and I felt like I was just testing mocks, mostly, but it's probably worth something basic at least.


describe('persistence', () => {
it('does not load a document if there is no document id', () => {
const args = makeDefaultArgs();
Expand Down
17 changes: 17 additions & 0 deletions x-pack/legacy/plugins/lens/public/app_plugin/app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,23 @@ export function App({

const lastKnownDocRef = useRef<Document | undefined>(undefined);

// Sync Kibana breadcrumbs any time the saved document's title changes
useEffect(() => {
chrome.breadcrumbs.set([
{
href: chrome.addBasePath(`/app/kibana#/visualize`),
text: i18n.translate('xpack.lens.breadcrumbsTitle', {
defaultMessage: 'Visualize',
}),
},
{
text: state.persistedDoc
? state.persistedDoc.title
: i18n.translate('xpack.lens.breadcrumbsCreate', { defaultMessage: 'Create' }),
},
]);
}, [state.persistedDoc && state.persistedDoc.title]);

useEffect(() => {
if (docId && (!state.persistedDoc || state.persistedDoc.id !== docId)) {
setState({ ...state, isLoading: true });
Expand Down