Chore: Clean up old navigation (#66287)

* remove code outside of the topnav feature flag

* delete NavBar folder

* remove topnav toggle from backend

* restructure AppChrome folder

* fix utils mock

* fix applinks tests

* remove tests since they're covered in e2e

* fix 1 of the approotpage tests

* Fix another dashboardpage test

* remove reverse portalling + test for plugins using deprecated onNavChanged method

* kick drone

* handle correlations
This commit is contained in:
Ashley Harrison
2023-04-14 09:43:11 +01:00
committed by GitHub
parent 202afb9041
commit 4abe0249ba
108 changed files with 240 additions and 3266 deletions

View File

@@ -36,32 +36,10 @@ const getPluginSettingsMock = getPluginSettings as jest.Mock<
>;
class RootComponent extends Component<AppRootProps> {
static timesMounted = 0;
componentDidMount() {
RootComponent.timesMounted += 1;
const node: NavModelItem = {
text: 'My Great plugin',
children: [
{
text: 'A page',
url: '/apage',
id: 'a',
},
{
text: 'Another page',
url: '/anotherpage',
id: 'b',
},
],
};
this.props.onNavChanged({
main: node,
node,
});
}
static timesRendered = 0;
render() {
return <p>my great plugin</p>;
RootComponent.timesRendered += 1;
return <p>my great component</p>;
}
}
@@ -119,36 +97,9 @@ describe('AppRootPage', () => {
enabled: true,
});
it('should not mount plugin twice if nav is changed', async () => {
// reproduces https://github.com/grafana/grafana/pull/28105
getPluginSettingsMock.mockResolvedValue(pluginMeta);
const plugin = new AppPlugin();
plugin.meta = pluginMeta;
plugin.root = RootComponent;
importAppPluginMock.mockResolvedValue(plugin);
renderUnderRouter();
// check that plugin and nav links were rendered, and plugin is mounted only once
expect(await screen.findByText('my great plugin')).toBeVisible();
expect(await screen.findByLabelText('Tab A page')).toBeVisible();
expect(await screen.findByLabelText('Tab Another page')).toBeVisible();
expect(RootComponent.timesMounted).toEqual(1);
});
it('should not render component if not at plugin path', async () => {
getPluginSettingsMock.mockResolvedValue(pluginMeta);
class RootComponent extends Component<AppRootProps> {
static timesRendered = 0;
render() {
RootComponent.timesRendered += 1;
return <p>my great component</p>;
}
}
const plugin = new AppPlugin();
plugin.meta = pluginMeta;
plugin.root = RootComponent;
@@ -160,18 +111,18 @@ describe('AppRootPage', () => {
expect(await screen.findByText('my great component')).toBeVisible();
// renders the first time
expect(RootComponent.timesRendered).toEqual(2);
expect(RootComponent.timesRendered).toEqual(1);
await act(async () => {
locationService.push('/foo');
});
expect(RootComponent.timesRendered).toEqual(2);
expect(RootComponent.timesRendered).toEqual(1);
await act(async () => {
locationService.push('/a/my-awesome-plugin');
});
expect(RootComponent.timesRendered).toEqual(4);
expect(RootComponent.timesRendered).toEqual(2);
});
});

View File

@@ -1,7 +1,6 @@
// Libraries
import { AnyAction, createSlice, PayloadAction } from '@reduxjs/toolkit';
import React, { useCallback, useEffect, useMemo, useReducer } from 'react';
import { createHtmlPortalNode, InPortal, OutPortal } from 'react-reverse-portal';
import { useLocation, useRouteMatch } from 'react-router-dom';
import { AppEvents, AppPlugin, AppPluginMeta, NavModel, NavModelItem, PluginType } from '@grafana/data';
@@ -37,7 +36,6 @@ export function AppRootPage({ pluginId, pluginNavSection }: Props) {
const match = useRouteMatch();
const location = useLocation();
const [state, dispatch] = useReducer(stateSlice.reducer, initialState);
const portalNode = useMemo(() => createHtmlPortalNode(), []);
const currentUrl = config.appSubUrl + location.pathname + location.search;
const { plugin, loading, pluginNav } = state;
const navModel = buildPluginSectionNav(pluginNavSection, pluginNav, currentUrl);
@@ -75,23 +73,18 @@ export function AppRootPage({ pluginId, pluginNavSection }: Props) {
/>
);
if (config.featureToggles.topnav && !pluginNav) {
if (!pluginNav) {
return <PluginPageContext.Provider value={context}>{pluginRoot}</PluginPageContext.Provider>;
}
return (
<>
<InPortal node={portalNode}>{pluginRoot}</InPortal>
{navModel ? (
<Page navModel={navModel} pageNav={pluginNav?.node}>
<Page.Contents isLoading={loading}>
<OutPortal node={portalNode} />
</Page.Contents>
<Page.Contents isLoading={loading}>{pluginRoot}</Page.Contents>
</Page>
) : (
<Page>
<OutPortal node={portalNode} />
</Page>
<Page>{pluginRoot}</Page>
)}
</>
);
@@ -112,8 +105,7 @@ const stateSlice = createSlice({
...pluginNav,
node: {
...pluginNav.main,
// Because breadcumbs code is also used to set title when topnav should only set hideFromBreadcrumbs when topnav is enabled
hideFromBreadcrumbs: config.featureToggles.topnav,
hideFromBreadcrumbs: true,
},
};
}

View File

@@ -1,5 +1,4 @@
import { NavModelItem } from '@grafana/data';
import { config } from '@grafana/runtime';
import { HOME_NAV_ID } from 'app/core/reducers/navModel';
import { buildPluginSectionNav } from './utils';
@@ -50,41 +49,30 @@ describe('buildPluginSectionNav', () => {
app1.parentItem = appsSection;
it('Should return pluginNav if topnav is disabled', () => {
config.featureToggles.topnav = false;
const result = buildPluginSectionNav(appsSection, pluginNav, '/a/plugin1/page1');
expect(result).toBe(pluginNav);
});
it('Should return return section nav if topnav is enabled', () => {
config.featureToggles.topnav = true;
it('Should return return section nav', () => {
const result = buildPluginSectionNav(appsSection, pluginNav, '/a/plugin1/page1');
expect(result?.main.text).toBe('apps');
});
it('Should set active page', () => {
config.featureToggles.topnav = true;
const result = buildPluginSectionNav(appsSection, null, '/a/plugin1/page2');
expect(result?.main.children![0].children![1].active).toBe(true);
expect(result?.node.text).toBe('page2');
});
it('Should only set the most specific match as active (not the parents)', () => {
config.featureToggles.topnav = true;
const result = buildPluginSectionNav(appsSection, null, '/a/plugin1/page2');
expect(result?.main.children![0].children![1].active).toBe(true);
expect(result?.main.children![0].active).not.toBe(true); // Parent should not be active
});
it('Should set app section to active', () => {
config.featureToggles.topnav = true;
const result = buildPluginSectionNav(appsSection, null, '/a/plugin1');
expect(result?.main.children![0].active).toBe(true);
expect(result?.node.text).toBe('App1');
});
it('Should handle standalone page', () => {
config.featureToggles.topnav = true;
const result = buildPluginSectionNav(adminSection, pluginNav, '/a/app2/config');
expect(result?.main.text).toBe('Admin');
expect(result?.node.text).toBe('Standalone page');

View File

@@ -1,5 +1,4 @@
import { GrafanaPlugin, NavModel, NavModelItem, PanelPluginMeta, PluginType } from '@grafana/data';
import { config } from '@grafana/runtime';
import { importPanelPluginFromMeta } from './importPanelPlugin';
import { getPluginSettings } from './pluginSettings';
@@ -35,11 +34,6 @@ export function buildPluginSectionNav(
pluginNav: NavModel | null,
currentUrl: string
): NavModel | undefined {
// When topnav is disabled we only just show pluginNav like before
if (!config.featureToggles.topnav) {
return pluginNav ?? undefined;
}
// shallow clone as we set active flag
let copiedPluginNavSection = { ...pluginNavSection };
let activePage: NavModelItem | undefined;