hemasrishalini commented on code in PR #38791:
URL: https://github.com/apache/superset/pull/38791#discussion_r2971634922
##########
superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel/index.tsx:
##########
@@ -49,148 +44,97 @@ const DetailsPanelPopover = ({
onHighlightFilterSource,
children,
popoverVisible,
- popoverContentRef,
- popoverTriggerRef,
setPopoverVisible,
}: DetailsPanelProps) => {
+ const theme = useTheme();
const activeTabs = useSelector<RootState>(
state => state.dashboardState?.activeTabs,
);
- // Combined ref array for all filter indicator elements
- const indicatorRefs = useRef<(HTMLButtonElement | null)[]>([]);
-
- const handleKeyDown = (event: KeyboardEvent<HTMLDivElement>) => {
- switch (event.key) {
- case 'Escape':
- case 'Enter':
- // timing out to allow for filter selection to happen first
- setTimeout(() => {
- // move back to the popover trigger element
- popoverTriggerRef?.current?.focus();
- // Close popover on ESC or ENTER
- setPopoverVisible(false);
- });
- break;
- case 'ArrowDown':
- case 'ArrowUp': {
- event.preventDefault(); // Prevent scrolling
- // Navigate through filters with arrows up/down
- const currentFocusIndex = indicatorRefs.current.findIndex(
- ref => ref === document.activeElement,
- );
- const maxIndex = indicatorRefs.current.length - 1;
- let nextFocusIndex = 0;
-
- if (event.key === 'ArrowDown') {
- nextFocusIndex =
- currentFocusIndex >= maxIndex ? 0 : currentFocusIndex + 1;
- } else if (event.key === 'ArrowUp') {
- nextFocusIndex =
- currentFocusIndex <= 0 ? maxIndex : currentFocusIndex - 1;
- }
- indicatorRefs.current[nextFocusIndex]?.focus();
- break;
- }
- case 'Tab':
- // forcing popover context until ESC or ENTER are pressed
- event.preventDefault();
- break;
- default:
- break;
- }
- };
-
- const handleVisibility = (isOpen: boolean) => {
- setPopoverVisible(isOpen);
- };
-
- // we don't need to clean up useEffect, setting { once: true } removes the
event listener after handle function is called
- useEffect(() => {
- if (popoverVisible) {
- window.addEventListener('resize', () => setPopoverVisible(false), {
- once: true,
- });
- }
- }, [popoverVisible]);
- // if tabs change, popover doesn't close automatically
useEffect(() => {
setPopoverVisible(false);
- }, [activeTabs]);
+ }, [activeTabs, setPopoverVisible]);
const indicatorKey = (indicator: Indicator): string =>
`${indicator.column} - ${indicator.name}`;
Review Comment:
The issue has been resolved in commit ce004ca. I've updated the indicatorKey
to include the path attribute, ensuring unique keys for AntD Menu and
preventing potential collisions across different dashboard filter paths. Thanks
for the catch!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
