Skip to content

Commit fa857c6

Browse files
committed
fixup! ✨(frontend) add keyboard navigation for subdocs with focus activation
1 parent af01958 commit fa857c6

File tree

3 files changed

+83
-5
lines changed

3 files changed

+83
-5
lines changed

src/frontend/apps/impress/src/features/docs/doc-tree/components/DocSubPageItem.tsx

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { Box, BoxButton, Icon, Text } from '@/components';
1212
import { useCunninghamTheme } from '@/cunningham';
1313
import { Doc, useTrans } from '@/features/docs/doc-management';
1414
import { useActionableMode } from '@/features/docs/doc-tree/hooks/useActionableMode';
15+
import { useLoadChildrenOnOpen } from '@/features/docs/doc-tree/hooks/useLoadChildrenOnOpen';
1516
import { useLeftPanelStore } from '@/features/left-panel';
1617
import { useResponsiveStore } from '@/stores';
1718

@@ -40,8 +41,8 @@ export const DocSubPageItem = (props: TreeViewNodeProps<Doc>) => {
4041
const { t } = useTranslation();
4142

4243
const [menuOpen, setMenuOpen] = useState(false);
43-
44-
const isActive = node.isFocused || menuOpen;
44+
const isSelectedNow = treeContext?.treeData.selectedNode?.id === doc.id;
45+
const isActive = node.isFocused || menuOpen || isSelectedNow;
4546

4647
const router = useRouter();
4748
const { togglePanel } = useLeftPanelStore();
@@ -81,13 +82,26 @@ export const DocSubPageItem = (props: TreeViewNodeProps<Doc>) => {
8182
}
8283
};
8384

84-
useKeyboardActivation(['Enter', ' '], isActive, handleActivate, true);
85+
// Disable activation keys while the menu is open to avoid conflicts
86+
useKeyboardActivation(
87+
['Enter', ' '],
88+
isActive && !menuOpen,
89+
handleActivate,
90+
true,
91+
);
92+
useLoadChildrenOnOpen(
93+
node.data.value.id,
94+
node.isOpen,
95+
treeContext?.treeData.handleLoadChildren,
96+
treeContext?.treeData.setChildren,
97+
(doc.children?.length ?? 0) > 0 || doc.childrenCount === 0,
98+
);
8599

86100
// prepare the text for the screen reader
87101
const docTitle = doc.title || untitledDocument;
88102
const hasChildren = (doc.children?.length || 0) > 0;
89103
const isExpanded = node.isOpen;
90-
const isSelected = treeContext?.treeData.selectedNode?.id === doc.id;
104+
const isSelected = isSelectedNow;
91105

92106
const ariaLabel = `${docTitle}${hasChildren ? `, ${isExpanded ? t('expanded') : t('collapsed')}` : ''}${isSelected ? `, ${t('selected')}` : ''}`;
93107

@@ -143,6 +157,15 @@ export const DocSubPageItem = (props: TreeViewNodeProps<Doc>) => {
143157
background: var(--c--theme--colors--greyscale-100);
144158
}
145159
}
160+
161+
/* Ensure actions are visible when hovering the whole item container */
162+
&:hover {
163+
.light-doc-item-actions {
164+
display: flex;
165+
opacity: 1;
166+
visibility: visible;
167+
}
168+
}
146169
`}
147170
>
148171
<TreeViewItem {...props} onClick={handleActivate}>

src/frontend/apps/impress/src/features/docs/doc-tree/components/DocTreeItemActions.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ export const DocTreeItemActions = ({
151151
};
152152

153153
useDropdownFocusManagement({
154-
isOpen: isOpen || false,
154+
isOpen: !!isOpen,
155155
docId: doc.id,
156156
actionsRef,
157157
});
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { useEffect, useRef } from 'react';
2+
3+
/**
4+
* Lazily loads children for a tree node the first time it is expanded.
5+
* Works for both mouse and keyboard expansions.
6+
*/
7+
export const useLoadChildrenOnOpen = <T>(
8+
nodeId: string,
9+
isOpen: boolean,
10+
handleLoadChildren?: (id: string) => Promise<T[]>,
11+
setChildren?: (id: string, children: T[]) => void,
12+
isAlreadyLoaded?: boolean,
13+
) => {
14+
const hasLoadedRef = useRef(false);
15+
16+
// Reset the local loaded flag when the node id changes
17+
useEffect(() => {
18+
hasLoadedRef.current = false;
19+
}, [nodeId]);
20+
21+
useEffect(() => {
22+
if (!isOpen) {
23+
return;
24+
}
25+
if (isAlreadyLoaded) {
26+
hasLoadedRef.current = true;
27+
return;
28+
}
29+
if (hasLoadedRef.current) {
30+
return;
31+
}
32+
if (!handleLoadChildren || !setChildren) {
33+
return;
34+
}
35+
36+
let isCancelled = false;
37+
// Mark as loading to prevent repeated fetches/renders that can cause flicker
38+
hasLoadedRef.current = true;
39+
void handleLoadChildren(nodeId)
40+
.then((children) => {
41+
if (isCancelled) {
42+
return;
43+
}
44+
setChildren(nodeId, children);
45+
})
46+
.catch(() => {
47+
// allow retry on next open
48+
hasLoadedRef.current = false;
49+
});
50+
51+
return () => {
52+
isCancelled = true;
53+
};
54+
}, [isOpen, nodeId, handleLoadChildren, setChildren, isAlreadyLoaded]);
55+
};

0 commit comments

Comments
 (0)