-
Notifications
You must be signed in to change notification settings - Fork 916
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
feat: Add rightNavigationButton
component in chrome service for applications to register and add dev tool to top right navigation
#6553
Changes from 14 commits
6669c07
6d6ece1
25c8bc4
0faebef
c962c9e
943d0fe
2723569
e737c46
e1663a5
3966338
30c0ee5
c5ff561
2d1a8ee
64051a6
57881d7
052a1fd
99b0fde
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
---|---|---|
@@ -0,0 +1,45 @@ | ||
/* | ||
* Copyright OpenSearch Contributors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
import React from 'react'; | ||
import { fireEvent, render } from '@testing-library/react'; | ||
import { RightNavigationButton } from './right_navigation_button'; | ||
import { applicationServiceMock, httpServiceMock } from '../../../../../core/public/mocks'; | ||
|
||
const mockProps = { | ||
application: applicationServiceMock.createStartContract(), | ||
http: httpServiceMock.createStartContract(), | ||
appId: 'app_id', | ||
iconType: 'mock_icon', | ||
title: 'title', | ||
}; | ||
|
||
describe('Right navigation button', () => { | ||
it('should render base element normally', () => { | ||
const { baseElement } = render(<RightNavigationButton {...mockProps} />); | ||
expect(baseElement).toMatchSnapshot(); | ||
}); | ||
|
||
it('should call application getUrlForApp and navigateToUrl after clicked', () => { | ||
const navigateToUrl = jest.fn(); | ||
const getUrlForApp = jest.fn(); | ||
const props = { | ||
...mockProps, | ||
application: { | ||
...applicationServiceMock.createStartContract(), | ||
getUrlForApp, | ||
navigateToUrl, | ||
}, | ||
}; | ||
const { getByTestId } = render(<RightNavigationButton {...props} />); | ||
const icon = getByTestId('rightNavigationButton'); | ||
fireEvent.click(icon); | ||
expect(getUrlForApp).toHaveBeenCalledWith('app_id', { | ||
path: '/', | ||
absolute: false, | ||
}); | ||
expect(navigateToUrl).toHaveBeenCalled(); | ||
raintygao marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
/* | ||
* Copyright OpenSearch Contributors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
import { EuiHeaderSectionItemButton, EuiIcon } from '@elastic/eui'; | ||
import React, { useMemo } from 'react'; | ||
import { CoreStart } from '../../..'; | ||
|
||
import { isModifiedOrPrevented } from './nav_link'; | ||
export interface RightNavigationButtonProps { | ||
application: CoreStart['application']; | ||
http: CoreStart['http']; | ||
appId: string; | ||
iconType: string; | ||
title: string; | ||
} | ||
|
||
export const RightNavigationButton = ({ | ||
application, | ||
http, | ||
appId, | ||
iconType, | ||
title, | ||
}: RightNavigationButtonProps) => { | ||
const targetUrl = useMemo(() => { | ||
const appUrl = application.getUrlForApp(appId, { | ||
path: '/', | ||
absolute: false, | ||
}); | ||
// Remove prefix in Url including workspace and other prefix | ||
return http.basePath.prepend(http.basePath.remove(appUrl), { | ||
withoutClientBasePath: true, | ||
}); | ||
}, [application, http.basePath, appId]); | ||
|
||
const navigateToApp = (event: React.MouseEvent<HTMLButtonElement, MouseEvent>) => { | ||
/* Use href and onClick to support "open in new tab" and SPA navigation in the same link */ | ||
if ( | ||
event.button === 0 && // ignore everything but left clicks | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: consider to extract to a function for better readability const isLeftClickEvent = (event) => event.button === 0; There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems your guys approvals were dismissed because of commits to address comments. Could you guys help add 2.x label and approve again? |
||
!isModifiedOrPrevented(event) | ||
) { | ||
event.preventDefault(); | ||
application.navigateToUrl(targetUrl); | ||
} | ||
return; | ||
}; | ||
|
||
return ( | ||
<EuiHeaderSectionItemButton | ||
data-test-subj="rightNavigationButton" | ||
aria-label={title} | ||
onClick={navigateToApp} | ||
href={targetUrl} | ||
> | ||
<EuiIcon type={iconType} size="m" title={title} color="text" /> | ||
</EuiHeaderSectionItemButton> | ||
); | ||
}; |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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 |
---|---|---|
|
@@ -28,19 +28,26 @@ | |
* under the License. | ||
*/ | ||
|
||
import React from 'react'; | ||
import { BehaviorSubject } from 'rxjs'; | ||
import { Plugin, CoreSetup, AppMountParameters } from 'src/core/public'; | ||
import { Plugin, CoreSetup, AppMountParameters, CoreStart } from 'src/core/public'; | ||
import { AppUpdater } from 'opensearch-dashboards/public'; | ||
import { i18n } from '@osd/i18n'; | ||
import { sortBy } from 'lodash'; | ||
import { DataSourceManagementPluginSetup } from 'src/plugins/data_source_management/public'; | ||
import { DataSourcePluginSetup } from 'src/plugins/data_source/public'; | ||
import { AppNavLinkStatus, DEFAULT_APP_CATEGORIES } from '../../../core/public'; | ||
import { | ||
AppNavLinkStatus, | ||
DEFAULT_APP_CATEGORIES, | ||
RightNavigationOrder, | ||
RightNavigationButton, | ||
} from '../../../core/public'; | ||
import { UrlForwardingSetup } from '../../url_forwarding/public'; | ||
import { CreateDevToolArgs, DevToolApp, createDevToolApp } from './dev_tool'; | ||
|
||
import './index.scss'; | ||
import { ManagementOverViewPluginSetup } from '../../management_overview/public'; | ||
import { toMountPoint } from '../../opensearch_dashboards_react/public'; | ||
|
||
export interface DevToolsSetupDependencies { | ||
urlForwarding: UrlForwardingSetup; | ||
|
@@ -75,12 +82,14 @@ export class DevToolsPlugin implements Plugin<DevToolsSetup> { | |
defaultMessage: 'Dev Tools', | ||
}); | ||
|
||
private id = 'dev_tools'; | ||
|
||
public setup(coreSetup: CoreSetup, deps: DevToolsSetupDependencies) { | ||
const { application: applicationSetup, getStartServices } = coreSetup; | ||
const { urlForwarding, managementOverview } = deps; | ||
|
||
applicationSetup.register({ | ||
id: 'dev_tools', | ||
id: this.id, | ||
title: this.title, | ||
updater$: this.appStateUpdater, | ||
icon: '/ui/logos/opensearch_mark.svg', | ||
|
@@ -99,7 +108,7 @@ export class DevToolsPlugin implements Plugin<DevToolsSetup> { | |
}); | ||
|
||
managementOverview?.register({ | ||
id: 'dev_tools', | ||
id: this.id, | ||
title: this.title, | ||
description: i18n.translate('devTools.devToolsDescription', { | ||
defaultMessage: | ||
|
@@ -125,10 +134,22 @@ export class DevToolsPlugin implements Plugin<DevToolsSetup> { | |
}; | ||
} | ||
|
||
public start() { | ||
public start(core: CoreStart) { | ||
if (this.getSortedDevTools().length === 0) { | ||
this.appStateUpdater.next(() => ({ navLinkStatus: AppNavLinkStatus.hidden })); | ||
} | ||
core.chrome.navControls.registerRight({ | ||
order: RightNavigationOrder.DevTool, | ||
mount: toMountPoint( | ||
React.createElement(RightNavigationButton, { | ||
appId: this.id, | ||
iconType: 'consoleApp', | ||
title: this.title, | ||
application: core.application, | ||
http: core.http, | ||
Comment on lines
+148
to
+149
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: perhaps |
||
}) | ||
), | ||
}); | ||
} | ||
|
||
public stop() {} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: the value could be 10, 20 and leave some gaps between them so that we can add more between settings and devtool without change the value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. Updated.