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

[Workspace]Dismiss get started for search/essential/analytics overview page #8874

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
2 changes: 2 additions & 0 deletions changelogs/fragments/8874.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feat:
- [Workspace] support dismiss get started for search overview page ([#8874](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8874))
1 change: 1 addition & 0 deletions src/plugins/content_management/public/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const createStartContract = (): jest.Mocked<ContentManagementPluginStart> => {
registerContentProvider: jest.fn(),
renderPage: jest.fn(),
updatePageSection: jest.fn(),
getPage: jest.fn(),
};
};

Expand Down
1 change: 1 addition & 0 deletions src/plugins/home/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,4 @@ export const USE_NEW_HOME_PAGE = 'home:useNewHomePage';
* The id is used in src/plugins/workspace/public/plugin.ts and please change that accordingly if you change the id here.
*/
export const IMPORT_SAMPLE_DATA_APP_ID = 'import_sample_data';
export const SEARCH_WORKSPACE_DISMISS_GET_STARTED = 'searchWorkspace:dismissGetStarted';
8 changes: 6 additions & 2 deletions src/plugins/home/public/application/application.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,15 @@ export const renderImportSampleDataApp = async (
export const renderSearchUseCaseOverviewApp = async (
element: HTMLElement,
coreStart: CoreStart,
contentManagementStart: ContentManagementPluginStart
contentManagementStart: ContentManagementPluginStart,
navigation: NavigationPublicPluginStart
) => {
render(
<OpenSearchDashboardsContextProvider services={{ ...coreStart }}>
<SearchUseCaseOverviewApp contentManagement={contentManagementStart} />
<SearchUseCaseOverviewApp
contentManagement={contentManagementStart}
navigation={navigation}
/>
</OpenSearchDashboardsContextProvider>,
element
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { render } from '@testing-library/react';
import { act, fireEvent, render } from '@testing-library/react';
import React from 'react';
import { coreMock } from '../../../../../../core/public/mocks';
import { OpenSearchDashboardsContextProvider } from '../../../../../opensearch_dashboards_react/public';
Expand All @@ -12,6 +12,9 @@ import { SearchUseCaseOverviewApp } from './search_use_case_app';
import { ContentManagementPluginStart } from '../../../../../content_management/public';
import { ALL_USE_CASE_ID, SEARCH_USE_CASE_ID } from '../../../../../../core/public';
import { BehaviorSubject } from 'rxjs';
import { navigationPluginMock } from '../../../../../navigation/public/mocks';
import { NavigationPublicPluginStart } from '../../../../../navigation/public';
import { TopNavControlsProps } from '../../../../../navigation/public/top_nav_menu';

describe('<SearchUseCaseOverviewApp />', () => {
const renderPageMock = jest.fn();
Expand All @@ -20,6 +23,15 @@ describe('<SearchUseCaseOverviewApp />', () => {
...contentManagementPluginMocks.createStartContract(),
renderPage: renderPageMock,
};
const navigationMock = navigationPluginMock.createStartContract();
const FunctionComponent = (props: TopNavControlsProps) => (
<div>
{props.controls?.map((control, idx) => (
<div key={idx}>{control.renderComponent}</div>
))}
</div>
);
navigationMock.ui.HeaderControl = FunctionComponent;
const core = coreMock.createStart();
const currentNavGroupMock = new BehaviorSubject({ id: 'Search' });
const coreStartMocks = {
Expand All @@ -35,11 +47,12 @@ describe('<SearchUseCaseOverviewApp />', () => {

function renderSearchUseCaseOverviewApp(
contentManagement: ContentManagementPluginStart,
navigation: NavigationPublicPluginStart,
services = { ...coreStartMocks }
) {
return (
<OpenSearchDashboardsContextProvider services={services}>
<SearchUseCaseOverviewApp contentManagement={contentManagement} />
<SearchUseCaseOverviewApp contentManagement={contentManagement} navigation={navigation} />
</OpenSearchDashboardsContextProvider>
);
}
Expand All @@ -50,10 +63,41 @@ describe('<SearchUseCaseOverviewApp />', () => {

it('render page normally', () => {
currentNavGroupMock.next({ id: SEARCH_USE_CASE_ID });
const { container } = render(renderSearchUseCaseOverviewApp(mock, coreStartMocks));
const { container } = render(
renderSearchUseCaseOverviewApp(mock, navigationMock, coreStartMocks)
);

expect(container).toMatchInlineSnapshot(`
<div>
<div>
<div>
<span
class="euiToolTipAnchor"
>
<div
class="euiPopover euiPopover--anchorDownCenter"
>
<div
class="euiPopover__anchor"
>
<button
aria-label="Page settings"
class="euiButtonIcon euiButtonIcon--primary euiButtonIcon--small"
data-test-subj="search-overview-setting-button"
type="button"
>
<span
aria-hidden="true"
class="euiButtonIcon__icon"
color="inherit"
data-euiicon-type="gear"
/>
</button>
</div>
</div>
</span>
</div>
</div>
dummy page
</div>
`);
Expand All @@ -64,10 +108,48 @@ describe('<SearchUseCaseOverviewApp />', () => {

it('set page title correctly in all use case', () => {
currentNavGroupMock.next({ id: ALL_USE_CASE_ID });
render(renderSearchUseCaseOverviewApp(mock, coreStartMocks));
render(renderSearchUseCaseOverviewApp(mock, navigationMock, coreStartMocks));

expect(coreStartMocks.chrome.setBreadcrumbs).toHaveBeenCalledWith([
{ text: 'Search Overview' },
]);
});

it('user able to dismiss get started', () => {
const { getByTestId, queryByText } = render(
renderSearchUseCaseOverviewApp(mock, navigationMock, coreStartMocks)
);

expect(getByTestId('search-overview-setting-button')).toBeInTheDocument();
act(() => {
fireEvent.click(getByTestId('search-overview-setting-button'));
});
expect(queryByText('Hide Get started with Search')).toBeInTheDocument();

act(() => {
fireEvent.click(queryByText('Hide Get started with Search')!);
});
expect(coreStartMocks.uiSettings.set).toBeCalledWith('searchWorkspace:dismissGetStarted', true);
});

it('user able to enable get started', () => {
coreStartMocks.uiSettings.get.mockReturnValueOnce(true);
const { getByTestId, queryByText } = render(
renderSearchUseCaseOverviewApp(mock, navigationMock, coreStartMocks)
);

expect(getByTestId('search-overview-setting-button')).toBeInTheDocument();
act(() => {
fireEvent.click(getByTestId('search-overview-setting-button'));
});
expect(queryByText('Show Get started with Search')).toBeInTheDocument();

act(() => {
fireEvent.click(queryByText('Show Get started with Search')!);
});
expect(coreStartMocks.uiSettings.set).toBeCalledWith(
'searchWorkspace:dismissGetStarted',
false
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,53 @@
* SPDX-License-Identifier: Apache-2.0
*/

import React, { useEffect } from 'react';
import React, { useEffect, useState } from 'react';
import { useObservable } from 'react-use';
import { CoreStart } from 'opensearch-dashboards/public';
import { EuiBreadcrumb } from '@elastic/eui';
import {
EuiBreadcrumb,
EuiButtonIcon,
EuiContextMenu,
EuiIcon,
EuiPopover,
EuiToolTip,
} from '@elastic/eui';
import { I18nProvider } from '@osd/i18n/react';
import { i18n } from '@osd/i18n';
import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_react/public';
import {
ContentManagementPluginStart,
SEARCH_OVERVIEW_PAGE_ID,
SECTIONS,
} from '../../../../../content_management/public';
import { SEARCH_USE_CASE_ID } from '../../../../../../core/public';
import { NavigationPublicPluginStart } from '../../../../../navigation/public';
import { getStartedSection } from './search_use_case_setup';
import { SEARCH_WORKSPACE_DISMISS_GET_STARTED } from '../../../../common/constants';

interface Props {
contentManagement: ContentManagementPluginStart;
navigation: NavigationPublicPluginStart;
}

export const SearchUseCaseOverviewApp = ({ contentManagement }: Props) => {
export const SearchUseCaseOverviewApp = ({ contentManagement, navigation }: Props) => {
const {
services: { chrome },
services: { chrome, application, uiSettings },
} = useOpenSearchDashboards<CoreStart>();
const [isPopoverOpen, setIsPopoverOpen] = useState(false);
const [isGetStartedDismissed, setIsGetStartedDismissed] = useState<boolean>(
!!uiSettings.get(SEARCH_WORKSPACE_DISMISS_GET_STARTED)
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I remember uiSettings has a get$ method that can return an observable, maybe we can use that and there is no need to have a state here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the state isGetStartedDismissed used for multiple place and we need a variable for

 const contextMenuItems = [
    {
      name: isGetStartedDismissed ? show : hide,
      icon: <EuiIcon type={isGetStartedDismissed ? 'eye' : 'eyeClosed'} />,
      onClick: async () => {
        await dismissGetStartCards(!isGetStartedDismissed);
        closePopover();
      },
    },
  ];

Copy link
Member

Choose a reason for hiding this comment

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

const isGetStartedDismissed = useObservable<boolean>(!!uiSettings.get$(SEARCH_WORKSPACE_DISMISS_GET_STARTED), may work as well and setIsGetStartedDismissed seems useless.

);

const togglePopover = () => setIsPopoverOpen(!isPopoverOpen);
const closePopover = () => setIsPopoverOpen(false);

const currentNavGroup = useObservable(chrome.navGroup.getCurrentNavGroup$());
const isSearchUseCase = currentNavGroup?.id === SEARCH_USE_CASE_ID;

const HeaderControl = navigation.ui.HeaderControl;
const page = contentManagement.getPage(SEARCH_OVERVIEW_PAGE_ID);

useEffect(() => {
const title = i18n.translate('home.searchOverview.title', { defaultMessage: 'Overview' });
const titleWithUseCase = i18n.translate('home.searchOverview.titleWithUseCase', {
Expand All @@ -48,8 +70,83 @@ export const SearchUseCaseOverviewApp = ({ contentManagement }: Props) => {
chrome.setBreadcrumbs(breadcrumbs);
}, [chrome, isSearchUseCase]);

const dismissGetStartCards = async (state: boolean) => {
uiSettings.set(SEARCH_WORKSPACE_DISMISS_GET_STARTED, state);
setIsGetStartedDismissed(state);
};

useEffect(() => {
if (isGetStartedDismissed) {
page?.removeSection(SECTIONS.GET_STARTED);
} else {
page?.createSection(getStartedSection);
}
}, [isGetStartedDismissed, page]);

const hide = i18n.translate('home.searchOverview.getStartedCard.setting.hide.label', {
defaultMessage: 'Hide Get started with Search',
});

const show = i18n.translate('home.searchOverview.getStartedCard.setting.show.label', {
defaultMessage: 'Show Get started with Search',
});
const contextMenuItems = [
{
name: isGetStartedDismissed ? show : hide,
icon: <EuiIcon type={isGetStartedDismissed ? 'eye' : 'eyeClosed'} />,
onClick: async () => {
await dismissGetStartCards(!isGetStartedDismissed);
closePopover();
},
},
];

const settingToolTip = i18n.translate('home.searchOverview.getStartedCard.setting.tooltip', {
defaultMessage: 'Page settings',
});

const pageHeaderButton = () => {
const popOver = (
<EuiPopover
button={
<EuiButtonIcon
iconType="gear"
aria-label={settingToolTip}
color="primary"
onClick={togglePopover}
display="base"
data-test-subj="search-overview-setting-button"
size="s"
/>
}
isOpen={isPopoverOpen}
closePopover={closePopover}
panelPaddingSize="none"
>
<EuiContextMenu
size="s"
initialPanelId={0}
panels={[
{
id: 0,
items: contextMenuItems,
},
]}
/>
</EuiPopover>
);
return isPopoverOpen ? popOver : <EuiToolTip content={settingToolTip}>{popOver}</EuiToolTip>;
};

const TopNavControls = [
{
renderComponent: pageHeaderButton(),
},
];

return (
<I18nProvider>
<HeaderControl setMountPoint={application.setAppRightControls} controls={TopNavControls} />
{contentManagement ? contentManagement.renderPage(SEARCH_OVERVIEW_PAGE_ID) : null}
</I18nProvider>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,26 @@ import {
SEARCH_OVERVIEW_PAGE_ID,
SECTIONS,
SEARCH_OVERVIEW_CONTENT_AREAS,
Section,
} from '../../../../../content_management/public';

const DISCOVER_APP_ID = 'discover';

export const getStartedSection: Section = {
id: SECTIONS.GET_STARTED,
order: 1000,
title: i18n.translate('home.searchOverview.setupSection.title', {
defaultMessage: 'Set up search',
}),
kind: 'card',
};

export const setupSearchUseCase = (contentManagement: ContentManagementPluginSetup) => {
contentManagement.registerPage({
id: SEARCH_OVERVIEW_PAGE_ID,
title: 'Overview',
sections: [
{
id: SECTIONS.GET_STARTED,
order: 1000,
title: i18n.translate('home.searchOverview.setupSection.title', {
defaultMessage: 'Set up search',
}),
kind: 'card',
},
getStartedSection,
{
id: SECTIONS.DIFFERENT_SEARCH_TYPES,
order: 2000,
Expand Down
5 changes: 3 additions & 2 deletions src/plugins/home/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,15 +196,16 @@ export class HomePublicPlugin
mount: async (params: AppMountParameters) => {
const [
coreStart,
{ contentManagement: contentManagementStart },
{ contentManagement: contentManagementStart, navigation },
] = await core.getStartServices();
setCommonService();

const { renderSearchUseCaseOverviewApp } = await import('./application');
return await renderSearchUseCaseOverviewApp(
params.element,
coreStart,
contentManagementStart
contentManagementStart,
navigation
);
},
});
Expand Down
6 changes: 6 additions & 0 deletions src/plugins/home/server/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import { registryForTutorialsMock, registryForSampleDataMock } from './plugin.test.mocks';
import { HomeServerPlugin } from './plugin';
import { coreMock, httpServiceMock } from '../../../core/server/mocks';
import { SEARCH_WORKSPACE_DISMISS_GET_STARTED } from '../common/constants';

describe('HomeServerPlugin', () => {
beforeEach(() => {
Expand Down Expand Up @@ -80,6 +81,11 @@ describe('HomeServerPlugin', () => {
expect.any(Function)
);
});

test('register ui settings', () => {
new HomeServerPlugin(initContext).setup(mockCoreSetup, {});
expect(mockCoreSetup.uiSettings.register).toHaveBeenCalledTimes(2);
});
});

describe('start', () => {
Expand Down
Loading
Loading