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

[navigation]feat: add home icon in left bottom #7802

Merged
merged 9 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from all 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/7802.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feat:
- Add home icon in left bottom ([#7802](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7802))

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
.bottom-container {
padding: 0 $euiSize;
display: flex;
-ms-overflow-style: -ms-autohiding-scrollbar;

&.bottom-container-collapsed {
flex-direction: column;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,7 @@ export function CollapsibleNavGroupEnabled({
<div
className={classNames({
'bottom-container': true,
'eui-xScroll': true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

How will this look on browser-OS combos that show a scrollbar?

Copy link
Member Author

Choose a reason for hiding this comment

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

I get your point, I thought the class will help to hide the scrollbar, but if not, let me find if there is any pattern code I can follow in OSD.

Copy link
Member

Choose a reason for hiding this comment

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

Seem behavior of adding the utility css class follows the OS settings. Was the intention to hide the scrollbar but allow user to scroll?

Copy link
Member Author

@SuZhou-Joe SuZhou-Joe Aug 23, 2024

Choose a reason for hiding this comment

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

I added -ms-overflow-style: -ms-autohiding-scrollbar; to the bottom container to make the behavior of scrollbar consistent. Do you think that's good enough?

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with it, UX need to figure out how to fit those overflow icons, I think we can optimize this afterwards.

'bottom-container-collapsed': !isNavOpen,
'bottom-container-expanded': isNavOpen,
})}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import React from 'react';
import { fireEvent, render } from '@testing-library/react';
import { HomeIcon } from './home_icon';
import { coreMock } from '../../../../../core/public/mocks';

describe('<HomeIcon />', () => {
it('should call chrome.navGroup.setCurrentNavGroup and application.navigateToApp methods from core service when click', () => {
const coreStartMock = coreMock.createStart();
const { container } = render(<HomeIcon core={coreStartMock} appId="foo" />);
const component = container.children[0];
fireEvent.click(component);
expect(coreStartMock.application.navigateToApp).toBeCalledWith('foo');
});
});
20 changes: 20 additions & 0 deletions src/plugins/home/public/application/components/home_icon.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import React from 'react';
import { EuiButtonIcon } from '@elastic/eui';
import { CoreStart } from 'opensearch-dashboards/public';

export function HomeIcon({ core, appId }: { core: CoreStart; appId: string }) {
return (
<EuiButtonIcon
aria-label="go-to-home"
SuZhou-Joe marked this conversation as resolved.
Show resolved Hide resolved
iconType="home"
onClick={() => {
core.application.navigateToApp(appId);
}}
/>
);
}
26 changes: 16 additions & 10 deletions src/plugins/home/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
} from 'opensearch-dashboards/public';
import { i18n } from '@osd/i18n';
import { first } from 'rxjs/operators';
import React from 'react';

import { Branding } from 'src/core/types';
import {
Expand Down Expand Up @@ -70,6 +71,8 @@
ContentManagementPluginStart,
} from '../../content_management/public';
import { initHome, setupHome } from './application/home_render';
import { toMountPoint } from '../../opensearch_dashboards_react/public';
import { HomeIcon } from './application/components/home_icon';

export interface HomePluginStartDependencies {
data: DataPublicPluginStart;
Expand Down Expand Up @@ -151,9 +154,7 @@
core.application.register({
id: PLUGIN_ID,
title: 'Home',
navLinkStatus: core.chrome.navGroup.getNavGroupEnabled()
? undefined
: AppNavLinkStatus.hidden,
navLinkStatus: AppNavLinkStatus.hidden,
mount: async (params: AppMountParameters) => {
const [coreStart] = await core.getStartServices();
setCommonService();
Expand All @@ -166,13 +167,6 @@
workspaceAvailability: WorkspaceAvailability.outsideWorkspace,
});

core.chrome.navGroup.addNavLinksToGroup(DEFAULT_NAV_GROUPS.all, [
{
id: PLUGIN_ID,
title: 'Home',
},
]);

// Register import sample data as a standalone app so that it is available inside workspace.
core.application.register({
id: IMPORT_SAMPLE_DATA_APP_ID,
Expand Down Expand Up @@ -255,6 +249,18 @@
});
}

if (core.chrome.navGroup.getNavGroupEnabled()) {
core.chrome.navControls.registerLeftBottom({

Check warning on line 253 in src/plugins/home/public/plugin.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/home/public/plugin.ts#L253

Added line #L253 was not covered by tests
order: 0,
mount: toMountPoint(
React.createElement(HomeIcon, {
core,
appId: PLUGIN_ID,
})
),
});
}

return {
featureCatalogue: this.featuresCatalogueRegistry,
getSavedHomepageLoader: () => this.sectionTypeService.getSavedHomepageLoader(),
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/saved_objects_management/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ export class SavedObjectsManagementPlugin
core.chrome.navGroup.addNavLinksToGroup(DEFAULT_NAV_GROUPS.settingsAndSetup, [
{
id: APP_ID,
order: 300,
order: 400,
},
]);

Expand Down
13 changes: 11 additions & 2 deletions src/plugins/visualize/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,39 +228,48 @@ export class VisualizePlugin
},
});

const titleInLeftNav = i18n.translate('visualize.leftNav.visualizeTitle', {
defaultMessage: 'Visualizations',
});

core.chrome.navGroup.addNavLinksToGroup(DEFAULT_NAV_GROUPS.observability, [
{
id: visualizeAppId,
category: DEFAULT_APP_CATEGORIES.visualizeAndReport,
order: 200,
title: titleInLeftNav,
},
]);
core.chrome.navGroup.addNavLinksToGroup(DEFAULT_NAV_GROUPS['security-analytics'], [
{
id: visualizeAppId,
category: DEFAULT_APP_CATEGORIES.visualizeAndReport,
order: 200,
title: titleInLeftNav,
},
]);
core.chrome.navGroup.addNavLinksToGroup(DEFAULT_NAV_GROUPS.essentials, [
{
id: visualizeAppId,
category: DEFAULT_APP_CATEGORIES.visualizeAndReport,
order: 200,
category: undefined,
order: 400,
title: titleInLeftNav,
},
]);
core.chrome.navGroup.addNavLinksToGroup(DEFAULT_NAV_GROUPS.search, [
{
id: visualizeAppId,
category: DEFAULT_APP_CATEGORIES.analyzeSearch,
order: 400,
title: titleInLeftNav,
},
]);
core.chrome.navGroup.addNavLinksToGroup(DEFAULT_NAV_GROUPS.all, [
{
id: visualizeAppId,
category: undefined,
order: 400,
title: titleInLeftNav,
},
]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export const WorkspaceMenu = ({ coreStart, registeredUseCases$ }: Props) => {
};

const currentWorkspaceButton = currentWorkspace ? (
<EuiButtonEmpty onClick={openPopover} data-test-subj="current-workspace-button">
<EuiButtonEmpty onClick={openPopover} data-test-subj="current-workspace-button" flush="both">
<EuiAvatar
size="s"
type="space"
Expand Down
18 changes: 3 additions & 15 deletions src/plugins/workspace/public/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,14 +170,14 @@ describe('Workspace plugin', () => {
expect.arrayContaining([
{
id: 'workspace_list',
order: 150,
title: 'Workspace settings',
order: 350,
title: 'Workspaces',
},
])
);
});

it('#setup should register workspace detail with a visible application and register to all nav group', async () => {
it('#setup should register workspace detail', async () => {
const setupMock = coreMock.createSetup();
setupMock.chrome.navGroup.getNavGroupEnabled.mockReturnValue(true);
const workspacePlugin = new WorkspacePlugin();
Expand All @@ -186,20 +186,8 @@ describe('Workspace plugin', () => {
expect(setupMock.application.register).toHaveBeenCalledWith(
expect.objectContaining({
id: 'workspace_detail',
navLinkStatus: AppNavLinkStatus.hidden,
})
);

expect(setupMock.chrome.navGroup.addNavLinksToGroup).toHaveBeenCalledWith(
DEFAULT_NAV_GROUPS.all,
expect.arrayContaining([
{
id: 'workspace_detail',
title: 'Overview',
order: 100,
},
])
);
});

it('#setup should register workspace initial with a visible application', async () => {
Expand Down
29 changes: 3 additions & 26 deletions src/plugins/workspace/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,19 +111,7 @@ export class WorkspacePlugin
this.registeredUseCases$,
]).subscribe(([currentWorkspace, registeredUseCases]) => {
if (currentWorkspace) {
const isAllUseCase =
getFirstUseCaseOfFeatureConfigs(currentWorkspace.features || []) === ALL_USE_CASE_ID;
this.appUpdater$.next((app) => {
// When in all workspace, the home should be replaced by workspace detail page
if (app.id === 'home' && isAllUseCase) {
return { navLinkStatus: AppNavLinkStatus.hidden };
}

// show the overview page in all use case
if (app.id === WORKSPACE_DETAIL_APP_ID && isAllUseCase) {
return { navLinkStatus: AppNavLinkStatus.visible };
}

if (isAppAccessibleInWorkspace(app, currentWorkspace, registeredUseCases)) {
return;
}
Expand Down Expand Up @@ -354,7 +342,6 @@ export class WorkspacePlugin
title: i18n.translate('workspace.settings.workspaceDetail', {
defaultMessage: 'Workspace Detail',
}),
navLinkStatus: AppNavLinkStatus.hidden,
async mount(params: AppMountParameters) {
const { renderDetailApp } = await import('./application');
return mountWorkspaceApp(params, renderDetailApp);
Expand Down Expand Up @@ -397,16 +384,6 @@ export class WorkspacePlugin
workspaceAvailability: WorkspaceAvailability.outsideWorkspace,
});

core.chrome.navGroup.addNavLinksToGroup(DEFAULT_NAV_GROUPS.all, [
{
id: WORKSPACE_DETAIL_APP_ID,
order: 100,
title: i18n.translate('workspace.nav.workspaceDetail.title', {
defaultMessage: 'Overview',
}),
},
]);

/**
* register workspace column into saved objects table
*/
Expand All @@ -418,9 +395,9 @@ export class WorkspacePlugin
core.chrome.navGroup.addNavLinksToGroup(DEFAULT_NAV_GROUPS.settingsAndSetup, [
{
id: WORKSPACE_LIST_APP_ID,
order: 150,
title: i18n.translate('workspace.settings.workspaceSettings', {
defaultMessage: 'Workspace settings',
order: 350,
title: i18n.translate('workspace.settings.workspaces', {
defaultMessage: 'Workspaces',
}),
},
]);
Expand Down
53 changes: 52 additions & 1 deletion src/plugins/workspace/public/services/use_case_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,17 @@

import { BehaviorSubject } from 'rxjs';
import { first } from 'rxjs/operators';
import { chromeServiceMock } from '../../../../core/public/mocks';
import { chromeServiceMock, coreMock } from '../../../../core/public/mocks';
import {
ALL_USE_CASE_ID,
DEFAULT_APP_CATEGORIES,
DEFAULT_NAV_GROUPS,
NavGroupItemInMap,
NavGroupType,
} from '../../../../core/public';
import { UseCaseService } from './use_case_service';
import { waitFor } from '@testing-library/dom';
import { WORKSPACE_DETAIL_APP_ID } from '../../common/constants';

const mockNavGroupsMap = {
system: {
Expand Down Expand Up @@ -61,6 +64,54 @@ const setupUseCaseStart = (options?: { navGroupEnabled?: boolean }) => {
};

describe('UseCaseService', () => {
describe('#setup', () => {
it('should add manage workspace category to current use case', async () => {
const useCaseService = new UseCaseService();
const coreSetup = coreMock.createSetup();
const navGroupMap$ = new BehaviorSubject<Record<string, NavGroupItemInMap>>({});
const coreStartMock = coreMock.createStart();
coreSetup.getStartServices.mockResolvedValue([coreStartMock, {}, {}]);
coreStartMock.chrome.navGroup.getNavGroupsMap$.mockReturnValue(navGroupMap$);
useCaseService.setup(coreSetup);
const navGroupInfo = {
...DEFAULT_NAV_GROUPS.all,
navLinks: [],
};
navGroupMap$.next({
[ALL_USE_CASE_ID]: navGroupInfo,
});
coreSetup.workspaces.currentWorkspace$.next({
id: ALL_USE_CASE_ID,
name: ALL_USE_CASE_ID,
features: [`use-case-${ALL_USE_CASE_ID}`],
});
await waitFor(() => {
expect(coreSetup.chrome.navGroup.addNavLinksToGroup).toBeCalledWith(navGroupInfo, [
{
id: 'dataSources_core',
category: DEFAULT_APP_CATEGORIES.manageWorkspace,
order: 100,
},
{
id: 'indexPatterns',
category: DEFAULT_APP_CATEGORIES.manageWorkspace,
order: 200,
},
{
id: 'objects',
category: DEFAULT_APP_CATEGORIES.manageWorkspace,
order: 300,
},
{
id: WORKSPACE_DETAIL_APP_ID,
category: DEFAULT_APP_CATEGORIES.manageWorkspace,
order: 400,
title: 'Workspace settings',
},
]);
});
});
});
describe('#start', () => {
it('should return built in use cases when nav group disabled', async () => {
const { useCaseStart } = setupUseCaseStart({
Expand Down
Loading
Loading