Skip to content

Commit

Permalink
Chore: Remove topnav references (#5092)
Browse files Browse the repository at this point in the history
# What this PR does

- removes references to `topnav`
  - `topnav` was default enabled in grafana v9.5
  - we intend to remove the toggle soon™️ 

## Which issue(s) this PR closes

Related to [issue link here]

<!--
*Note*: If you want the issue to be auto-closed once the PR is merged,
change "Related to" to "Closes" in the line above.
If you have more than one GitHub issue that this PR closes, be sure to
preface
each issue link with a [closing
keyword](https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/using-keywords-in-issues-and-pull-requests#linking-a-pull-request-to-an-issue).
This ensures that the issue(s) are auto-closed once the PR has been
merged.
-->

## Checklist

- [ ] Unit, integration, and e2e (if applicable) tests updated
- [x] Documentation added (or `pr:no public docs` PR label added if not
required)
- [x] Added the relevant release notes label (see labels prefixed w/
`release:`). These labels dictate how your PR will
    show up in the autogenerated release notes.
  • Loading branch information
ashharrison90 authored Sep 30, 2024
1 parent 2852618 commit 671a537
Show file tree
Hide file tree
Showing 14 changed files with 64 additions and 159 deletions.
2 changes: 1 addition & 1 deletion dev/helm-local.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ grafana:
- name: DATABASE_PASSWORD
value: oncallpassword
env:
GF_FEATURE_TOGGLES_ENABLE: topnav,externalServiceAccounts
GF_FEATURE_TOGGLES_ENABLE: externalServiceAccounts
GF_SECURITY_ADMIN_PASSWORD: oncall
GF_SECURITY_ADMIN_USER: oncall
GF_PLUGINS_ALLOW_LOADING_UNSIGNED_PLUGINS: grafana-oncall-app
Expand Down
4 changes: 1 addition & 3 deletions grafana-plugin/jest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ import { TextEncoder, TextDecoder } from 'util';
jest.mock('@grafana/runtime', () => ({
__esModule: true,
config: {
featureToggles: {
topNav: false,
},
featureToggles: {},
bootData: {
user: {
timezone: 'UTC',
Expand Down
7 changes: 1 addition & 6 deletions grafana-plugin/src/PluginPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@ import { Header } from 'navbar/Header/Header';

import { RenderConditionally } from 'components/RenderConditionally/RenderConditionally';
import { pages } from 'pages/pages';
import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers';

interface AppPluginPageProps extends PluginPageProps {
page?: string;
}

export const PluginPage = (isTopNavbar() ? RealPlugin : PluginPageFallback) as React.ComponentType<AppPluginPageProps>;
export const PluginPage = RealPlugin as React.ComponentType<AppPluginPageProps>;

function RealPlugin(props: AppPluginPageProps): React.ReactNode {
const { page } = props;
Expand All @@ -33,7 +32,3 @@ function RealPlugin(props: AppPluginPageProps): React.ReactNode {
</RealPluginPage>
);
}

export function PluginPageFallback(props: PluginPageProps): React.ReactNode {
return props.children;
}
11 changes: 1 addition & 10 deletions grafana-plugin/src/containers/Alerts/Alerts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import { getSlackMessage } from 'containers/DefaultPageLayout/DefaultPageLayout.
import { SlackError } from 'containers/DefaultPageLayout/DefaultPageLayout.types';
import { getIfChatOpsConnected } from 'containers/DefaultPageLayout/helper';
import { UserHelper } from 'models/user/user.helpers';
import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers';
import { AppFeature } from 'state/features';
import { useStore } from 'state/useStore';

Expand Down Expand Up @@ -72,7 +71,7 @@ export const Alerts = observer(() => {
return null;
}
return (
<div className={cx(styles.alertsContainer, { [styles.alertsContainerLegacy]: !isTopNavbar() })}>
<div className={styles.alertsContainer}>
{showSlackInstallAlert && (
<Alert
className={styles.alert}
Expand Down Expand Up @@ -192,13 +191,5 @@ const getStyles = (theme: GrafanaTheme2) => {
instructionsLink: css`
color: ${theme.colors.primary.text};
`,

alertsContainerLegacy: css`
paddingtop: '10px';
@media (max-width: 768px) {
padding-top: 50px;
}
`,
};
};
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import React, { FC, ReactElement } from 'react';
import React, { FC } from 'react';

import { css, cx } from '@emotion/css';
import { css } from '@emotion/css';
import { AppRootProps, NavModelItem } from '@grafana/data';
import { useStyles2 } from '@grafana/ui';
import { PluginPage } from 'PluginPage';
import { observer } from 'mobx-react';

import { Alerts } from 'containers/Alerts/Alerts';
import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers';

interface DefaultPageLayoutProps extends AppRootProps {
children?: any;
Expand All @@ -19,39 +17,11 @@ export const DefaultPageLayout: FC<DefaultPageLayoutProps> = observer((props) =>
const { children, page, pageNav } = props;
const styles = useStyles2(getStyles);

if (isTopNavbar()) {
return renderTopNavbar();
}

return renderLegacyNavbar();

function renderTopNavbar(): ReactElement {
return (
<PluginPage page={page} pageNav={pageNav as any}>
<div className={styles.root}>{children}</div>
</PluginPage>
);
}

function renderLegacyNavbar(): ReactElement {
return (
<PluginPage page={page}>
<div
className={cx(
'page-container',
css`
height: 100%;
`
)}
>
<div className={cx(styles.root)}>
<Alerts />
{children}
</div>
</div>
</PluginPage>
);
}
return (
<PluginPage page={page} pageNav={pageNav as any}>
<div className={styles.root}>{children}</div>
</PluginPage>
);
});

const getStyles = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ import { rootStore } from 'state/rootStore';

import { MobileAppConnection } from './MobileAppConnection';

jest.mock('plugin/GrafanaPluginRootPage.helpers', () => ({
isTopNavbar: () => false,
}));

jest.mock('helpers/authorization/authorization', () => ({
...jest.requireActual('helpers/authorization/authorization'),
isUserActionAllowed: jest.fn().mockReturnValue(true),
Expand Down
27 changes: 9 additions & 18 deletions grafana-plugin/src/containers/RotationForm/RotationForm.helpers.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import { Dayjs, ManipulateType } from 'dayjs';
import { GRAFANA_HEADER_HEIGHT, GRAFANA_LEGACY_SIDEBAR_WIDTH } from 'helpers/consts';
import { GRAFANA_HEADER_HEIGHT } from 'helpers/consts';
import { DraggableData } from 'react-draggable';

import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers';

import { RepeatEveryPeriod } from './RotationForm.types';

export const getRepeatShiftsEveryOptions = (repeatEveryPeriod: number) => {
Expand Down Expand Up @@ -195,20 +193,13 @@ export function getDraggableModalCoordinatesOnInit(
const baseReferenceElRect = body.getBoundingClientRect();
const { innerHeight } = window;

const { right, bottom } = baseReferenceElRect;
const { right } = baseReferenceElRect;

return isTopNavbar()
? {
// values are adjusted by any padding/margin differences
left: -data.node.offsetLeft + 12,
right: right - (data.node.offsetLeft + data.node.offsetWidth) - 12,
top: -offsetTop + GRAFANA_HEADER_HEIGHT + 12,
bottom: innerHeight - data.node.offsetHeight - offsetTop - 12,
}
: {
left: -data.node.offsetLeft + 12 + GRAFANA_LEGACY_SIDEBAR_WIDTH,
right: right - (data.node.offsetLeft + data.node.offsetWidth) - 12,
top: -offsetTop + 12,
bottom: bottom - data.node.offsetHeight - offsetTop - 12,
};
return {
// values are adjusted by any padding/margin differences
left: -data.node.offsetLeft + 12,
right: right - (data.node.offsetLeft + data.node.offsetWidth) - 12,
top: -offsetTop + GRAFANA_HEADER_HEIGHT + 12,
bottom: innerHeight - data.node.offsetHeight - offsetTop - 12,
};
}
3 changes: 1 addition & 2 deletions grafana-plugin/src/navbar/Header/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { observer } from 'mobx-react';
import gitHubStarSVG from 'assets/img/github_star.svg';
import logo from 'assets/img/logo.svg';
import { Alerts } from 'containers/Alerts/Alerts';
import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers';

import { getHeaderStyles } from './Header.styles';

Expand All @@ -18,7 +17,7 @@ export const Header = observer(() => {
return (
<>
<div>
<div className={cx('page-header__inner', { [styles.headerTopNavbar]: isTopNavbar() })}>
<div className={cx('page-header__inner', styles.headerTopNavbar)}>
<div className={styles.navbarLeft}>
<span className={cx('page-header__logo', styles.logoContainer)}>
<img className={styles.pageHeaderImage} src={logo} alt="Grafana OnCall" />
Expand Down
4 changes: 1 addition & 3 deletions grafana-plugin/src/navbar/LegacyNavHeading.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers';

interface LegacyNavHeadingProps {
children: JSX.Element;
show?: boolean;
}

export const LegacyNavHeading = function (props: LegacyNavHeadingProps): JSX.Element {
const { show = !isTopNavbar(), children } = props;
const { show = false, children } = props;
return show ? children : null;
};
9 changes: 4 additions & 5 deletions grafana-plugin/src/pages/pages.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { UserActions, UserAction, isUserActionAllowed } from 'helpers/authorizat
import { PLUGIN_ROOT } from 'helpers/consts';
import { matchPath } from 'react-router-dom-v5-compat';

import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers';
import { AppFeature } from 'state/features';
import { RootBaseStore } from 'state/rootBaseStore/RootBaseStore';

Expand Down Expand Up @@ -109,7 +108,7 @@ export const pages: { [id: string]: PageDefinition } = [
text: 'ChatOps',
path: getPath('chat-ops'),
hideFromBreadcrumbs: true,
hideFromTabs: isTopNavbar(),
hideFromTabs: true,
action: UserActions.ChatOpsRead,
},
{
Expand All @@ -126,7 +125,7 @@ export const pages: { [id: string]: PageDefinition } = [
text: 'Env Variables',
hideFromTabsFn: (store: RootBaseStore) => {
const hasLiveSettings = store.hasFeature(AppFeature.LiveSettings);
return isTopNavbar() || !hasLiveSettings;
return !hasLiveSettings;
},
path: getPath('live-settings'),
action: UserActions.OtherSettingsRead,
Expand All @@ -137,7 +136,7 @@ export const pages: { [id: string]: PageDefinition } = [
text: 'Cloud',
hideFromTabsFn: (store: RootBaseStore) => {
const hasCloudFeature = store.hasFeature(AppFeature.CloudConnection);
return isTopNavbar() || !hasCloudFeature;
return !hasCloudFeature;
},
path: getPath('cloud'),
action: UserActions.OtherSettingsWrite,
Expand All @@ -161,7 +160,7 @@ export const pages: { [id: string]: PageDefinition } = [
...current,
getPageNav: (pageTitle: string) =>
({
text: isTopNavbar() ? '' : current.text,
text: '',
parentItem: current.getParentItem ? current.getParentItem(pageTitle) : undefined,
hideFromBreadcrumbs: current.hideFromBreadcrumbs,
hideFromTabs: current.hideFromTabs,
Expand Down
79 changes: 37 additions & 42 deletions grafana-plugin/src/pages/settings/SettingsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { observer } from 'mobx-react';

import { ChatOpsPage } from 'pages/settings/tabs/ChatOps/ChatOps';
import { MainSettings } from 'pages/settings/tabs/MainSettings/MainSettings';
import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers';
import { AppFeature } from 'state/features';
import { WithStoreProps } from 'state/types';
import { withMobXProviderContext } from 'state/withStore';
Expand Down Expand Up @@ -54,52 +53,48 @@ class Settings extends React.Component<SettingsPageProps, SettingsPageState> {
const showCloudPage = hasCloudPage && isUserActionAllowed(UserActions.OtherSettingsWrite);
const showLiveSettings = hasLiveSettings && isUserActionAllowed(UserActions.OtherSettingsRead);

if (isTopNavbar()) {
return (
<>
<TabsBar>
return (
<>
<TabsBar>
<Tab
key={SettingsPageTab.MainSettings.key}
onChangeTab={() => onTabChange(SettingsPageTab.MainSettings.key)}
active={activeTab === SettingsPageTab.MainSettings.key}
label={SettingsPageTab.MainSettings.value}
/>
<Tab
key={SettingsPageTab.ChatOps.key}
onChangeTab={() => onTabChange(SettingsPageTab.ChatOps.key)}
active={activeTab === SettingsPageTab.ChatOps.key}
label={SettingsPageTab.ChatOps.value}
/>
<Tab
key={SettingsPageTab.TeamsSettings.key}
onChangeTab={() => onTabChange(SettingsPageTab.TeamsSettings.key)}
active={activeTab === SettingsPageTab.TeamsSettings.key}
label={SettingsPageTab.TeamsSettings.value}
/>
{showLiveSettings && (
<Tab
key={SettingsPageTab.MainSettings.key}
onChangeTab={() => onTabChange(SettingsPageTab.MainSettings.key)}
active={activeTab === SettingsPageTab.MainSettings.key}
label={SettingsPageTab.MainSettings.value}
key={SettingsPageTab.EnvVariables.key}
onChangeTab={() => onTabChange(SettingsPageTab.EnvVariables.key)}
active={activeTab === SettingsPageTab.EnvVariables.key}
label={SettingsPageTab.EnvVariables.value}
/>
)}
{showCloudPage && (
<Tab
key={SettingsPageTab.ChatOps.key}
onChangeTab={() => onTabChange(SettingsPageTab.ChatOps.key)}
active={activeTab === SettingsPageTab.ChatOps.key}
label={SettingsPageTab.ChatOps.value}
key={SettingsPageTab.Cloud.key}
onChangeTab={() => onTabChange(SettingsPageTab.Cloud.key)}
active={activeTab === SettingsPageTab.Cloud.key}
label={SettingsPageTab.Cloud.value}
/>
<Tab
key={SettingsPageTab.TeamsSettings.key}
onChangeTab={() => onTabChange(SettingsPageTab.TeamsSettings.key)}
active={activeTab === SettingsPageTab.TeamsSettings.key}
label={SettingsPageTab.TeamsSettings.value}
/>
{showLiveSettings && (
<Tab
key={SettingsPageTab.EnvVariables.key}
onChangeTab={() => onTabChange(SettingsPageTab.EnvVariables.key)}
active={activeTab === SettingsPageTab.EnvVariables.key}
label={SettingsPageTab.EnvVariables.value}
/>
)}
{showCloudPage && (
<Tab
key={SettingsPageTab.Cloud.key}
onChangeTab={() => onTabChange(SettingsPageTab.Cloud.key)}
active={activeTab === SettingsPageTab.Cloud.key}
label={SettingsPageTab.Cloud.value}
/>
)}
</TabsBar>

<TabsContent activeTab={activeTab} />
</>
);
}
)}
</TabsBar>

return <MainSettings />;
<TabsContent activeTab={activeTab} />
</>
);
}

getMatchingPageNav() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import { LegacyNavHeading } from 'navbar/LegacyNavHeading';
import { Text } from 'components/Text/Text';
import { ApiTokenSettings } from 'containers/ApiTokenSettings/ApiTokenSettings';
import { WithPermissionControlTooltip } from 'containers/WithPermissionControl/WithPermissionControlTooltip';
import { TeamsSettings } from 'pages/settings/tabs/TeamsSettings/TeamsSettings';
import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers';
import { useStore } from 'state/useStore';

export const MainSettings = observer(() => {
Expand Down Expand Up @@ -50,14 +48,6 @@ export const MainSettings = observer(() => {
</WithPermissionControlTooltip>
</Field>
</div>
{!isTopNavbar() && (
<div style={{ marginBottom: '20px' }}>
<Text.Title level={3} className={styles.title}>
Teams and Access Settings
</Text.Title>
<TeamsSettings />
</div>
)}
<Text.Title level={3} className={styles.title}>
API URL
</Text.Title>
Expand Down
5 changes: 0 additions & 5 deletions grafana-plugin/src/plugin/GrafanaPluginRootPage.helpers.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
import { config } from '@grafana/runtime';

export function isTopNavbar(): boolean {
return !!config.featureToggles.topnav;
}

export function getQueryParams(): any {
const searchParams = new URLSearchParams(window.location.search);
Expand Down
Loading

0 comments on commit 671a537

Please sign in to comment.