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 8 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
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
11 changes: 10 additions & 1 deletion src/plugins/workspace/public/services/use_case_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import { combineLatest, Observable, Subscription } from 'rxjs';
import { distinctUntilChanged, map } from 'rxjs/operators';
import { i18n } from '@osd/i18n';

import {
ChromeStart,
Expand All @@ -15,7 +16,7 @@ import {
DEFAULT_NAV_GROUPS,
ALL_USE_CASE_ID,
} from '../../../../core/public';
import { WORKSPACE_USE_CASES } from '../../common/constants';
import { WORKSPACE_DETAIL_APP_ID, WORKSPACE_USE_CASES } from '../../common/constants';
import {
convertNavGroupToWorkspaceUseCase,
getFirstUseCaseOfFeatureConfigs,
Expand Down Expand Up @@ -78,6 +79,14 @@ export class UseCaseService {
category: DEFAULT_APP_CATEGORIES.manageWorkspace,
order: 300,
},
{
id: WORKSPACE_DETAIL_APP_ID,
category: DEFAULT_APP_CATEGORIES.manageWorkspace,
order: 400,
title: i18n.translate('workspace.settings.workspaceSettings', {
defaultMessage: 'Workspace settings',
}),
},
]);
}
});
Expand Down
Loading