Skip to content

Commit 96cdf21

Browse files
aks07aks07
authored
Fix: Opening logs link broken (Pref framework) (#9182)
* fix: logs popover content logic extracted out * fix: logs popover content in live view * fix: destory popover on close * feat: add logs format tests * feat: minor refactor * feat: test case refactor * feat: remove menu refs in logs live view
1 parent 1aa5f5d commit 96cdf21

File tree

3 files changed

+183
-37
lines changed

3 files changed

+183
-37
lines changed

‎frontend/src/components/LogsFormatOptionsMenu/LogsFormatOptionsMenu.tsx‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ interface LogsFormatOptionsMenuProps {
2626
config: OptionsMenuConfig;
2727
}
2828

29-
export default function LogsFormatOptionsMenu({
29+
function OptionsMenu({
3030
items,
3131
selectedOptionFormat,
3232
config,
@@ -49,7 +49,6 @@ export default function LogsFormatOptionsMenu({
4949
const [selectedValue, setSelectedValue] = useState<string | null>(null);
5050
const listRef = useRef<HTMLDivElement>(null);
5151
const initialMouseEnterRef = useRef<boolean>(false);
52-
const [isPopoverOpen, setIsPopoverOpen] = useState<boolean>(false);
5352

5453
const onChange = useCallback(
5554
(key: LogViewMode) => {
@@ -209,7 +208,7 @@ export default function LogsFormatOptionsMenu({
209208
};
210209
}, [selectedValue]);
211210

212-
const popoverContent = (
211+
return (
213212
<div
214213
className={cx(
215214
'nested-menu-container',
@@ -447,15 +446,30 @@ export default function LogsFormatOptionsMenu({
447446
)}
448447
</div>
449448
);
449+
}
450+
451+
function LogsFormatOptionsMenu({
452+
items,
453+
selectedOptionFormat,
454+
config,
455+
}: LogsFormatOptionsMenuProps): JSX.Element {
456+
const [isPopoverOpen, setIsPopoverOpen] = useState<boolean>(false);
450457
return (
451458
<Popover
452-
content={popoverContent}
459+
content={
460+
<OptionsMenu
461+
items={items}
462+
selectedOptionFormat={selectedOptionFormat}
463+
config={config}
464+
/>
465+
}
453466
trigger="click"
454467
placement="bottomRight"
455468
arrow={false}
456469
open={isPopoverOpen}
457470
onOpenChange={setIsPopoverOpen}
458471
rootClassName="format-options-popover"
472+
destroyTooltipOnHide
459473
>
460474
<Button
461475
className="periscope-btn ghost"
@@ -465,3 +479,5 @@ export default function LogsFormatOptionsMenu({
465479
</Popover>
466480
);
467481
}
482+
483+
export default LogsFormatOptionsMenu;

‎frontend/src/components/LogsFormatOptionsMenu/__tests__/LogsFormatOptionsMenu.test.tsx‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
import { FontSize } from 'container/OptionsMenu/types';
2+
import { fireEvent, render, waitFor } from 'tests/test-utils';
3+
4+
import LogsFormatOptionsMenu from '../LogsFormatOptionsMenu';
5+
6+
const mockUpdateFormatting = jest.fn();
7+
8+
jest.mock('providers/preferences/sync/usePreferenceSync', () => ({
9+
usePreferenceSync: (): any => ({
10+
preferences: {
11+
columns: [],
12+
formatting: {
13+
maxLines: 2,
14+
format: 'table',
15+
fontSize: 'small',
16+
version: 1,
17+
},
18+
},
19+
loading: false,
20+
error: null,
21+
updateColumns: jest.fn(),
22+
updateFormatting: mockUpdateFormatting,
23+
}),
24+
}));
25+
26+
describe('LogsFormatOptionsMenu (unit)', () => {
27+
beforeEach(() => {
28+
mockUpdateFormatting.mockClear();
29+
});
30+
31+
function setup(): {
32+
getByTestId: ReturnType<typeof render>['getByTestId'];
33+
findItemByLabel: (label: string) => Element | undefined;
34+
formatOnChange: jest.Mock<any, any>;
35+
maxLinesOnChange: jest.Mock<any, any>;
36+
fontSizeOnChange: jest.Mock<any, any>;
37+
} {
38+
const items = [
39+
{ key: 'raw', label: 'Raw', data: { title: 'max lines per row' } },
40+
{ key: 'list', label: 'Default' },
41+
{ key: 'table', label: 'Column', data: { title: 'columns' } },
42+
];
43+
44+
const formatOnChange = jest.fn();
45+
const maxLinesOnChange = jest.fn();
46+
const fontSizeOnChange = jest.fn();
47+
48+
const { getByTestId } = render(
49+
<LogsFormatOptionsMenu
50+
items={items}
51+
selectedOptionFormat="table"
52+
config={{
53+
format: { value: 'table', onChange: formatOnChange },
54+
maxLines: { value: 2, onChange: maxLinesOnChange },
55+
fontSize: { value: FontSize.SMALL, onChange: fontSizeOnChange },
56+
addColumn: {
57+
isFetching: false,
58+
value: [],
59+
options: [],
60+
onFocus: jest.fn(),
61+
onBlur: jest.fn(),
62+
onSearch: jest.fn(),
63+
onSelect: jest.fn(),
64+
onRemove: jest.fn(),
65+
},
66+
}}
67+
/>,
68+
);
69+
70+
// Open the popover menu by default for each test
71+
const formatButton = getByTestId('periscope-btn-format-options');
72+
fireEvent.click(formatButton);
73+
74+
const getMenuItems = (): Element[] =>
75+
Array.from(document.querySelectorAll('.menu-items .item'));
76+
const findItemByLabel = (label: string): Element | undefined =>
77+
getMenuItems().find((el) => (el.textContent || '').includes(label));
78+
79+
return {
80+
getByTestId,
81+
findItemByLabel,
82+
formatOnChange,
83+
maxLinesOnChange,
84+
fontSizeOnChange,
85+
};
86+
}
87+
88+
// Covers: opens menu, changes format selection, updates max-lines, changes font size
89+
it('opens and toggles format selection', async () => {
90+
const { findItemByLabel, formatOnChange } = setup();
91+
92+
// Assert initial selection
93+
const columnItem = findItemByLabel('Column') as Element;
94+
expect(document.querySelectorAll('.menu-items .item svg')).toHaveLength(1);
95+
expect(columnItem.querySelector('svg')).toBeInTheDocument();
96+
97+
// Change selection to 'Raw'
98+
const rawItem = findItemByLabel('Raw') as Element;
99+
fireEvent.click(rawItem as HTMLElement);
100+
await waitFor(() => {
101+
const rawEl = findItemByLabel('Raw') as Element;
102+
expect(document.querySelectorAll('.menu-items .item svg')).toHaveLength(1);
103+
expect(rawEl.querySelector('svg')).toBeInTheDocument();
104+
});
105+
expect(formatOnChange).toHaveBeenCalledWith('raw');
106+
});
107+
108+
it('increments max-lines and calls onChange', async () => {
109+
const { maxLinesOnChange } = setup();
110+
111+
// Increment max lines
112+
const input = document.querySelector(
113+
'.max-lines-per-row-input input',
114+
) as HTMLInputElement;
115+
const initial = Number(input.value);
116+
const buttons = document.querySelectorAll(
117+
'.max-lines-per-row-input .periscope-btn',
118+
);
119+
const incrementBtn = buttons[1] as HTMLElement;
120+
fireEvent.click(incrementBtn);
121+
122+
await waitFor(() => {
123+
expect(Number(input.value)).toBe(initial + 1);
124+
});
125+
await waitFor(() => {
126+
expect(maxLinesOnChange).toHaveBeenCalledWith(initial + 1);
127+
});
128+
});
129+
130+
it('changes font size to MEDIUM and calls onChange', async () => {
131+
const { fontSizeOnChange } = setup();
132+
// Open font dropdown
133+
const fontButton = document.querySelector(
134+
'.font-size-container .value',
135+
) as HTMLElement;
136+
fireEvent.click(fontButton);
137+
138+
// Choose MEDIUM
139+
const optionButtons = Array.from(
140+
document.querySelectorAll('.font-size-dropdown .option-btn'),
141+
);
142+
const mediumBtn = optionButtons[1] as HTMLElement;
143+
fireEvent.click(mediumBtn);
144+
145+
await waitFor(() => {
146+
expect(
147+
document.querySelectorAll('.font-size-dropdown .option-btn .icon'),
148+
).toHaveLength(1);
149+
expect(
150+
(optionButtons[1] as Element).querySelector('.icon'),
151+
).toBeInTheDocument();
152+
});
153+
await waitFor(() => {
154+
expect(fontSizeOnChange).toHaveBeenCalledWith(FontSize.MEDIUM);
155+
});
156+
});
157+
});

‎frontend/src/container/LiveLogs/LiveLogsContainer/index.tsx‎

Lines changed: 6 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,15 @@
11
import './LiveLogsContainer.styles.scss';
22

3-
import { Button, Switch, Typography } from 'antd';
3+
import { Switch, Typography } from 'antd';
44
import LogsFormatOptionsMenu from 'components/LogsFormatOptionsMenu/LogsFormatOptionsMenu';
55
import { MAX_LOGS_LIST_SIZE } from 'constants/liveTail';
66
import { LOCALSTORAGE } from 'constants/localStorage';
77
import GoToTop from 'container/GoToTop';
88
import { useOptionsMenu } from 'container/OptionsMenu';
99
import { useGetCompositeQueryParam } from 'hooks/queryBuilder/useGetCompositeQueryParam';
1010
import { useQueryBuilder } from 'hooks/queryBuilder/useQueryBuilder';
11-
import useClickOutside from 'hooks/useClickOutside';
1211
import useDebouncedFn from 'hooks/useDebouncedFunction';
1312
import { useEventSourceEvent } from 'hooks/useEventSourceEvent';
14-
import { Sliders } from 'lucide-react';
1513
import { useEventSource } from 'providers/EventSource';
1614
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
1715
import { useLocation } from 'react-router-dom';
@@ -41,9 +39,6 @@ function LiveLogsContainer(): JSX.Element {
4139

4240
const batchedEventsRef = useRef<ILiveLogsLog[]>([]);
4341

44-
const [showFormatMenuItems, setShowFormatMenuItems] = useState(false);
45-
const menuRef = useRef<HTMLDivElement>(null);
46-
4742
const prevFilterExpressionRef = useRef<string | null>(null);
4843

4944
const { options, config } = useOptionsMenu({
@@ -73,18 +68,6 @@ function LiveLogsContainer(): JSX.Element {
7368
},
7469
];
7570

76-
const handleToggleShowFormatOptions = (): void =>
77-
setShowFormatMenuItems(!showFormatMenuItems);
78-
79-
useClickOutside({
80-
ref: menuRef,
81-
onClickOutside: () => {
82-
if (showFormatMenuItems) {
83-
setShowFormatMenuItems(false);
84-
}
85-
},
86-
});
87-
8871
const {
8972
handleStartOpenConnection,
9073
handleCloseConnection,
@@ -231,21 +214,11 @@ function LiveLogsContainer(): JSX.Element {
231214
/>
232215
</div>
233216

234-
<div className="format-options-container" ref={menuRef}>
235-
<Button
236-
className="periscope-btn ghost"
237-
onClick={handleToggleShowFormatOptions}
238-
icon={<Sliders size={14} />}
239-
/>
240-
241-
{showFormatMenuItems && (
242-
<LogsFormatOptionsMenu
243-
items={formatItems}
244-
selectedOptionFormat={options.format}
245-
config={config}
246-
/>
247-
)}
248-
</div>
217+
<LogsFormatOptionsMenu
218+
items={formatItems}
219+
selectedOptionFormat={options.format}
220+
config={config}
221+
/>
249222
</div>
250223

251224
{showLiveLogsFrequencyChart && (

0 commit comments

Comments
 (0)