Skip to content

Commit 09fe320

Browse files
PMM-14508: QAN light mode support (#1732)
* QAN light mode support * delete commit * PMM-14508: improve areas of the user interface that may be less intuitive for users * PMM-14508: Remove unnecessary key prop, fix TypeScript types, and replace hardcoded colors - Remove key={theme.type} from Filters and CheckboxGroup components (no longer needed for theme switching) - Replace all 'any' types with proper TypeScript interfaces (FilterGroup, FilterItem, Filters) - Create Filters.types.ts with type definitions - Replace hardcoded colors with theme variables: - #3d3d3d → theme.colors.border2 (divider) - rgb(50, 179, 227) → theme.colors.linkExternal (counter) - rgba(40, 40, 40) → theme.colors.border2 (border) - #c6c6c6 → theme.colors.textWeak (icon fill) These changes address code review feedback and improve theme reactivity without force remounting components. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * PMM-14508: Remove key prop from main QAN component and improve theme type safety - Remove key={grafanaTheme.type} from QueryAnalytics.tsx (theme updates work correctly without force remount) - Update getAntdTheme parameter type to accept GrafanaTheme | undefined for better type safety - Improve JSDoc to clarify that grafanaTheme may be undefined during initialization These changes address additional code review feedback from matejkubinec. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * PMM-14508: Revert removal of key prop - required for Ant Design theme updates The key={grafanaTheme.type} on the main QAN component div is necessary to force remount when theme changes. Without it, Ant Design components (Table, Select, Checkbox) do not pick up the new theme from ConfigProvider and render with incorrect colors until page refresh (e.g., dark mode table in light theme). The key prop was correctly removed from child components (Filters, CheckboxGroup) as their styles update reactively via useTheme() hook. Only the top-level component wrapping ConfigProvider needs the key prop. Added detailed comment explaining why force remount is required for future maintainers. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * PMM-14508: change theme * PMM-14508: resolve test failed --------- Co-authored-by: Claude <[email protected]>
1 parent 29051bb commit 09fe320

File tree

14 files changed

+294
-47
lines changed

14 files changed

+294
-47
lines changed

pmm-app/src/pmm-qan/panel/QueryAnalytics.tsx

Lines changed: 60 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,27 @@
11
import React, {
2-
FC, useCallback, useContext, useEffect, useRef, useState,
2+
FC,
3+
useCallback,
4+
useContext,
5+
useEffect,
6+
useLayoutEffect,
7+
useRef,
8+
useState,
9+
useMemo,
310
} from 'react';
411
import SplitPane from 'react-split-pane';
512
import { Button, useTheme } from '@grafana/ui';
13+
import type { GrafanaTheme } from '@grafana/data';
614
import { cx } from '@emotion/css';
715
import { showSuccessNotification, showWarningNotification } from 'shared/components/helpers';
816
import { ConfigProvider } from 'antd';
9-
import { antdTheme } from 'shared/core/theme';
17+
import { getAntdTheme } from 'shared/core/theme';
18+
import { applyPmmCssVariables } from 'shared/components/helpers/getPmmTheme';
1019
import { QueryAnalyticsProvider, UrlParametersProvider } from './provider/provider';
1120
import {
12-
Details, Filters, ManageColumns, Overview,
21+
Details,
22+
Filters,
23+
ManageColumns,
24+
Overview,
1325
} from './components';
1426
import 'shared/styles.scss';
1527
import 'shared/style.less';
@@ -18,15 +30,21 @@ import { getStyles } from './QueryAnalytics.styles';
1830
import { Messages } from './QueryAnalytics.messages';
1931
import { buildShareLink, toUnixTimestamp } from './QueryAnalytics.tools';
2032

21-
const QueryAnalyticsPanel: FC = () => {
22-
const theme = useTheme();
23-
const styles = getStyles(theme);
33+
// Panel now receives theme from root.
34+
interface QueryAnalyticsPanelProps {
35+
grafanaTheme: GrafanaTheme;
36+
}
37+
38+
const QueryAnalyticsPanel: FC<QueryAnalyticsPanelProps> = ({ grafanaTheme }) => {
39+
const styles = getStyles(grafanaTheme);
2440

2541
const {
2642
panelState: { querySelected, from, to },
2743
} = useContext(QueryAnalyticsProvider);
44+
2845
const queryAnalyticsWrapper = useRef<HTMLDivElement>(null);
2946
const [, setReload] = useState<object>({});
47+
3048
const copyLinkToClipboard = useCallback(() => {
3149
const link = buildShareLink(toUnixTimestamp(from), toUnixTimestamp(to));
3250

@@ -50,7 +68,15 @@ const QueryAnalyticsPanel: FC = () => {
5068
}, [querySelected]);
5169

5270
return (
53-
<div className="query-analytics-grid" id="antd" ref={queryAnalyticsWrapper}>
71+
// Force remount when theme changes to ensure Ant Design components (Table, Select, Checkbox)
72+
// pick up the new theme from ConfigProvider. Without this, components render with wrong colors
73+
// until page refresh (e.g., dark mode Table in light theme).
74+
<div
75+
key={grafanaTheme.type}
76+
className="query-analytics-grid"
77+
id="antd"
78+
ref={queryAnalyticsWrapper}
79+
>
5480
<div className="overview-filters">
5581
<Filters />
5682
</div>
@@ -82,7 +108,9 @@ const QueryAnalyticsPanel: FC = () => {
82108
pane2Style={{ minHeight: '20%', zIndex: 999 }}
83109
>
84110
<Overview />
85-
<div className={styles.detailsWrapper}>{querySelected ? <Details /> : null}</div>
111+
<div className={styles.detailsWrapper}>
112+
{querySelected ? <Details /> : null}
113+
</div>
86114
</SplitPane>
87115
</div>
88116
</div>
@@ -91,10 +119,27 @@ const QueryAnalyticsPanel: FC = () => {
91119
);
92120
};
93121

94-
export default (props) => (
95-
<ConfigProvider theme={antdTheme}>
96-
<UrlParametersProvider {...props}>
97-
<QueryAnalyticsPanel />
98-
</UrlParametersProvider>
99-
</ConfigProvider>
100-
);
122+
const QueryAnalyticsRoot: FC<any> = (props) => {
123+
const grafanaTheme = useTheme();
124+
125+
const antdTheme = useMemo(
126+
() => getAntdTheme(grafanaTheme),
127+
[grafanaTheme],
128+
);
129+
130+
// Apply CSS variables for QAN theme (dropdowns, backgrounds, text colors)
131+
// useLayoutEffect runs synchronously before browser paint, ensuring styles are applied before render
132+
useLayoutEffect(() => {
133+
applyPmmCssVariables(grafanaTheme);
134+
}, [grafanaTheme, grafanaTheme.type]);
135+
136+
return (
137+
<ConfigProvider theme={antdTheme}>
138+
<UrlParametersProvider {...props}>
139+
<QueryAnalyticsPanel grafanaTheme={grafanaTheme} />
140+
</UrlParametersProvider>
141+
</ConfigProvider>
142+
);
143+
};
144+
145+
export default QueryAnalyticsRoot;

pmm-app/src/pmm-qan/panel/components/Filters/Filters.constants.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Databases } from 'shared/core';
22
import { getServiceType } from './Filters.tools';
3+
import { FilterGroup } from './Filters.types';
34

45
export const FILTERS_BODY_HEIGHT = 600;
56
export const FILTERS_HEADER_SIZE = 50;
@@ -20,7 +21,7 @@ export const HIDDEN_FILTER_LABELS = [
2021
'top_queryid',
2122
];
2223

23-
export const FILTERS_GROUPS = [
24+
export const FILTERS_GROUPS: FilterGroup[] = [
2425
{
2526
name: 'Environment',
2627
dataKey: 'environment',

pmm-app/src/pmm-qan/panel/components/Filters/Filters.styles.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export const getStyles = stylesFactory((theme: GrafanaTheme) => {
99

1010
return {
1111
getFiltersWrapper: (height) => css`
12-
border: 1px solid rgba(40, 40, 40);
12+
border: 1px solid ${theme.colors.border2};
1313
height: ${height}px;
1414
padding: 10px 16px !important;
1515
border-radius: 3px;
@@ -22,7 +22,7 @@ export const getStyles = stylesFactory((theme: GrafanaTheme) => {
2222
}
2323
`,
2424
icon: css`
25-
fill: #c6c6c6;
25+
fill: ${theme.colors.textWeak};
2626
`,
2727
filtersHeader: css`
2828
display: flex;

pmm-app/src/pmm-qan/panel/components/Filters/Filters.tsx

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -113,13 +113,17 @@ export const Filters: FC = () => {
113113
setFilter('');
114114
}}
115115
>
116-
<div ref={filtersWrapperRef} className={cx({ [styles.filtersDisabled]: loadingDetails })}>
116+
<div
117+
ref={filtersWrapperRef}
118+
className={cx({ [styles.filtersDisabled]: loadingDetails })}
119+
>
117120
<FiltersHeader loading={loading} />
118121
<Overlay isPending={loading}>
119122
<Scrollbar className={styles.getFiltersWrapper(height)}>
120123
<FilterInput filter={filter} />
121-
{filtersGroups.filter((group) => filters[group.dataKey]).map(
122-
({ name, dataKey, getDashboardURL }) => (
124+
{filtersGroups
125+
.filter((group) => filters[group.dataKey])
126+
.map(({ name, dataKey, getDashboardURL }) => (
123127
<CheckboxGroup
124128
key={name}
125129
name={name}
@@ -130,8 +134,7 @@ export const Filters: FC = () => {
130134
getDashboardURL={getDashboardURL}
131135
rawTime={rawTime}
132136
/>
133-
),
134-
)}
137+
))}
135138
</Scrollbar>
136139
</Overlay>
137140
</div>
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
export interface FilterGroup {
2+
name: string;
3+
dataKey: string;
4+
getDashboardURL?: (value: string) => string;
5+
}
6+
7+
export interface FilterItem {
8+
value: string;
9+
checked: boolean;
10+
main_metric_percent?: number;
11+
}
12+
13+
export interface FilterData {
14+
name: FilterItem[];
15+
}
16+
17+
export interface Filters {
18+
[key: string]: FilterData;
19+
}

pmm-app/src/pmm-qan/panel/components/Filters/components/CheckboxGroup/CheckboxGroup.styles.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,10 @@ export const getStyles = stylesFactory((theme: GrafanaTheme) => {
6969
margin-top: 3px !important;
7070
margin-bottom: 12px !important;
7171
height: 1px !important;
72-
background-color: #3d3d3d !important;
72+
background-color: ${theme.colors.border2} !important;
7373
`,
7474
showModeSwitcher: css`
75-
color: rgb(50, 179, 227) !important;
75+
color: ${theme.colors.linkExternal} !important;
7676
`,
7777
};
7878
});

pmm-app/src/pmm-qan/panel/components/Filters/hooks/useFilters.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,13 @@ import {
66
COMMENT_NAME_LENGTH,
77
HIDDEN_FILTER_LABELS,
88
} from '../Filters.constants';
9+
import { Filters, FilterGroup } from '../Filters.types';
910

10-
export const useFilters = (): [any, boolean, any, boolean] => {
11-
const [filters, setFilters] = useState({});
11+
export const useFilters = (): [Filters, boolean, FilterGroup[], boolean] => {
12+
const [filters, setFilters] = useState<Filters>({});
1213
const [error, setError] = useState(false);
1314
const [loading, setLoading] = useState(false);
14-
const [filtersGroups, setFiltersGroups] = useState(FILTERS_GROUPS);
15+
const [filtersGroups, setFiltersGroups] = useState<FilterGroup[]>(FILTERS_GROUPS);
1516

1617
const {
1718
panelState: {

pmm-app/src/pmm-qan/panel/components/ManageColumns/ManageColumns.scss

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,21 @@
22
border-radius: 2px;
33
}
44

5-
.ant-select-dropdown,
6-
li.ant-select-dropdown-menu-item {
7-
background-color: #3d3d3d;
8-
color: white;
5+
/* QAN dropdown theme styles are now in qan.scss for all dropdowns */
6+
/* Additional styles specific to add/manage columns dropdown */
7+
.ant-select-dropdown.add-columns-selector-dropdown,
8+
.ant-select-dropdown.manage-columns-selector-dropdown {
99
border-radius: 0;
10+
opacity: 1;
11+
12+
/* Make sure badges/spans inside options inherit the text color */
13+
.ant-select-item-option-content span {
14+
color: var(--qan-dropdown-text) !important;
15+
background-color: transparent !important;
16+
}
1017
}
1118

19+
/* Legacy rule for other dropdowns */
1220
.fields__select-field .ant-select-dropdown-menu-item-active .select-item {
1321
background-color: #cccccc;
1422
}

pmm-app/src/pmm-qan/panel/components/ManageColumns/ManageColumns.styles.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,16 @@ export const getStyles = stylesFactory((theme: GrafanaTheme) => {
2626
margin: 0 !important;
2727
margin-right: 4px !important;
2828
`,
29+
// Action buttons at the bottom of dropdown (Remove column, Swap with main metric)
30+
// Light theme: light gray background for readability of black text
31+
// Dark theme: dark gray background
2932
actionElement: css`
3033
padding: 4px 8px;
3134
cursor: pointer;
32-
background-color: #3d3d3d;
35+
background-color: ${theme.isLight ? '#e0e0e0' : '#3d3d3d'};
3336
transition: background 0.3s ease;
3437
&:hover {
35-
background-color: #2d2e2f;
38+
background-color: ${theme.isLight ? '#d0d0d0' : '#2d2e2f'};
3639
}
3740
`,
3841
metricsTooltip: css`

pmm-app/src/pmm-qan/panel/components/Overview/components/QanTable/Table.styles.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import { getPmmTheme } from 'shared/components/helpers/getPmmTheme';
66
export const getStyles = stylesFactory((theme: GrafanaTheme) => {
77
const parameters = getPmmTheme(theme);
88

9-
const selectedRowColor = theme.isLight ? 'deepskyblue' : '#234682';
10-
119
return {
1210
tableWrap: (size) => css`
1311
display: block;
@@ -33,7 +31,7 @@ export const getStyles = stylesFactory((theme: GrafanaTheme) => {
3331
3432
.selected-overview-row {
3533
.td {
36-
background-color: ${selectedRowColor};
34+
background-color: ${parameters.table.selectedRowColor};
3735
}
3836
}
3937
.tr {
@@ -126,7 +124,6 @@ export const getStyles = stylesFactory((theme: GrafanaTheme) => {
126124
sortBy: css`
127125
display: flex;
128126
flex-direction: column;
129-
padding: 10px;
130127
131128
.sort-by:before,
132129
.sort-by:after {
@@ -148,11 +145,11 @@ export const getStyles = stylesFactory((theme: GrafanaTheme) => {
148145
margin-top: 1px;
149146
}
150147
.sort-by.asc:after {
151-
border-top-color: deepskyblue;
148+
border-top-color: ${parameters.table.sortIconColor};
152149
}
153150
154151
.sort-by.desc:before {
155-
border-bottom-color: deepskyblue;
152+
border-bottom-color: ${parameters.table.sortIconColor};
156153
}
157154
`,
158155
headerRow: css`

0 commit comments

Comments
 (0)