Skip to content

Commit c7cbd49

Browse files
richardrichard
authored andcommitted
fix: remove deprecated testing library hook and simplify menu ref caching
- Replace @testing-library/react-hook with @testing-library/react - Remove redundant menuRef caching (menu already memoized via useMemo) - Simplify hook return statement
1 parent 97f96a3 commit c7cbd49

File tree

2 files changed

+9
-23
lines changed

2 files changed

+9
-23
lines changed

superset-frontend/src/dashboard/components/AutoRefreshStatus/StatusIndicatorDot.test.tsx

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ import { render, screen, act } from 'spec/helpers/testing-library';
2020
import { StatusIndicatorDot } from './StatusIndicatorDot';
2121
import { AutoRefreshStatus } from '../../types/autoRefresh';
2222

23+
afterEach(() => {
24+
jest.runOnlyPendingTimers();
25+
jest.useRealTimers();
26+
});
27+
2328
test('renders with success status', () => {
2429
render(<StatusIndicatorDot status={AutoRefreshStatus.Success} />);
2530
const dot = screen.getByTestId('status-indicator-dot');
@@ -64,7 +69,7 @@ test('has correct accessibility attributes', () => {
6469
expect(dot).toHaveAttribute('aria-label', 'Auto-refresh status: success');
6570
});
6671

67-
test('fetching status updates immediately without debounce', async () => {
72+
test('fetching status updates immediately without debounce', () => {
6873
jest.useFakeTimers();
6974

7075
const { rerender } = render(
@@ -76,11 +81,9 @@ test('fetching status updates immediately without debounce', async () => {
7681

7782
const dot = screen.getByTestId('status-indicator-dot');
7883
expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Fetching);
79-
80-
jest.useRealTimers();
8184
});
8285

83-
test('debounces non-fetching status changes to prevent flickering', async () => {
86+
test('debounces non-fetching status changes to prevent flickering', () => {
8487
jest.useFakeTimers();
8588

8689
const { rerender } = render(
@@ -102,8 +105,6 @@ test('debounces non-fetching status changes to prevent flickering', async () =>
102105
// Now should be error
103106
dot = screen.getByTestId('status-indicator-dot');
104107
expect(dot).toHaveAttribute('data-status', AutoRefreshStatus.Error);
105-
106-
jest.useRealTimers();
107108
});
108109

109110
test('accepts custom size prop', () => {

superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
* under the License.
1818
*/
1919
import type { Dispatch, ReactElement, SetStateAction } from 'react';
20-
import { useState, useEffect, useCallback, useMemo, useRef } from 'react';
20+
import { useState, useEffect, useCallback, useMemo } from 'react';
2121
import { useSelector } from 'react-redux';
2222
import { useHistory } from 'react-router-dom';
2323
import { Menu, MenuItem } from '@superset-ui/core/components/Menu';
@@ -72,7 +72,6 @@ export const useHeaderActionsMenu = ({
7272
Dispatch<SetStateAction<boolean>>,
7373
] => {
7474
const [isDropdownVisible, setIsDropdownVisible] = useState(false);
75-
const menuRef = useRef<React.ReactElement | null>(null);
7675
const history = useHistory();
7776
const directPathToChild = useSelector(
7877
(state: RootState) => state.dashboardState.directPathToChild,
@@ -339,19 +338,5 @@ export const useHeaderActionsMenu = ({
339338
userCanShare,
340339
]);
341340

342-
if (!menuRef.current) {
343-
menuRef.current = menu;
344-
}
345-
346-
useEffect(() => {
347-
if (!isDropdownVisible) {
348-
menuRef.current = menu;
349-
}
350-
}, [isDropdownVisible, menu]);
351-
352-
return [
353-
isDropdownVisible && menuRef.current ? menuRef.current : menu,
354-
isDropdownVisible,
355-
setIsDropdownVisible,
356-
];
341+
return [menu, isDropdownVisible, setIsDropdownVisible];
357342
};

0 commit comments

Comments
 (0)