Skip to content

Commit

Permalink
Fix tabs navigation on the settings page (#4289)
Browse files Browse the repository at this point in the history
# What this PR does

Fix tabs navigation on the settings page

## Which issue(s) this PR closes

Related to https://github.com/grafana/oncall-private/issues/2563

## 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.

---------

Co-authored-by: Dominik Broj <[email protected]>
  • Loading branch information
Maxim Mordasov and brojd authored Apr 30, 2024
1 parent b5d900d commit 2f1ea7d
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 12 deletions.
12 changes: 12 additions & 0 deletions grafana-plugin/e2e-tests/settings/tabs.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { test, expect } from '../fixtures';
import { goToOnCallPage } from '../utils/navigation';

test(`tab query param is used to show proper page tab`, async ({ adminRolePage }) => {
const { page } = adminRolePage;
goToOnCallPage(page, `settings`, { tab: 'ChatOps' });

const tab = page.getByRole('tab', { name: 'Chat Ops' });
const isSelected = await tab.getAttribute('aria-selected');

expect(isSelected).toBe('true');
});
7 changes: 5 additions & 2 deletions grafana-plugin/e2e-tests/utils/navigation.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { KeyValue } from '@grafana/data';
import type { Page } from '@playwright/test';
import qs from 'query-string';

import { BASE_URL } from './constants';

Expand All @@ -17,8 +19,9 @@ const _goToPage = async (page: Page, url = '') => page.goto(`${BASE_URL}${url}`)

export const goToGrafanaPage = async (page: Page, url = '') => _goToPage(page, url);

export const goToOnCallPage = async (page: Page, onCallPage: OnCallPage) => {
await _goToPage(page, `/a/grafana-oncall-app/${onCallPage}`);
export const goToOnCallPage = async (page: Page, onCallPage: OnCallPage, queryParams?: KeyValue) => {
const queryParamsString = queryParams ? `?${qs.stringify(queryParams)}` : '';
await _goToPage(page, `/a/grafana-oncall-app/${onCallPage}${queryParamsString}`);
await page.waitForLoadState('networkidle');
await page.waitForTimeout(1000);
};
21 changes: 14 additions & 7 deletions grafana-plugin/src/pages/settings/SettingsPage.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';

import { AppRootProps } from '@grafana/data';
import { Tab, TabsBar } from '@grafana/ui';
import cn from 'classnames/bind';
import { observer } from 'mobx-react';
Expand All @@ -8,8 +9,9 @@ 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 { RootBaseStore } from 'state/rootBaseStore/RootBaseStore';
import { WithStoreProps } from 'state/types';
import { withMobXProviderContext } from 'state/withStore';
import { LocationHelper } from 'utils/LocationHelper';
import { isUserActionAllowed, UserActions } from 'utils/authorization/authorization';

import { SettingsPageTab } from './SettingsPage.types';
Expand All @@ -21,18 +23,22 @@ import styles from './SettingsPage.module.css';

const cx = cn.bind(styles);

interface SettingsPageProps {
store: RootBaseStore;
}
interface SettingsPageProps extends AppRootProps, WithStoreProps {}
interface SettingsPageState {
activeTab: string;
}

@observer
class Settings extends React.Component<SettingsPageProps, SettingsPageState> {
state: SettingsPageState = {
activeTab: SettingsPageTab.MainSettings.key, // should read from route instead
};
constructor(props: SettingsPageProps) {
super(props);

const tab = LocationHelper.getQueryParam('tab');

this.state = {
activeTab: tab || SettingsPageTab.MainSettings.key,
};
}

render() {
return <div className={cx('root')}>{this.renderContent()}</div>;
Expand All @@ -44,6 +50,7 @@ class Settings extends React.Component<SettingsPageProps, SettingsPageState> {

const onTabChange = (tab: string) => {
this.setState({ activeTab: tab });
LocationHelper.update({ tab }, 'partial');
};

const hasLiveSettings = store.hasFeature(AppFeature.LiveSettings);
Expand Down
6 changes: 3 additions & 3 deletions grafana-plugin/src/pages/settings/tabs/ChatOps/ChatOps.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ export class _ChatOpsPage extends React.Component<ChatOpsProps, ChatOpsState> {
};

componentDidMount() {
const { query } = this.props; // eslint-disable-line
const tab = LocationHelper.getQueryParam('chatOpsTab');

this.handleChatopsTabChange(query?.tab || ChatOpsTab.Slack);
this.handleChatopsTabChange(tab || ChatOpsTab.Slack);
}

componentWillUnmount() {
Expand Down Expand Up @@ -94,7 +94,7 @@ export class _ChatOpsPage extends React.Component<ChatOpsProps, ChatOpsState> {

handleChatopsTabChange(tab: ChatOpsTab) {
this.setState({ activeTab: tab });
LocationHelper.update({ tab: tab }, 'partial');
LocationHelper.update({ chatOpsTab: tab }, 'partial');
}
}

Expand Down

0 comments on commit 2f1ea7d

Please sign in to comment.