fix(skills): scope success banner and add menu focus management

Gate the skills install success banner behind a local just-installed flag
so it no longer re-appears stale after reopening and cancelling the add
dialog, and disable the cancel/close controls while an install is in
flight. Add keyboard focus management to ActionMenu: focus the first item
on open and restore focus to the trigger on Escape or item selection.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Simos Mikelatos
2026-06-30 22:46:29 +00:00
parent 3a9b1d6011
commit 6317896cd8
2 changed files with 41 additions and 2 deletions

View File

@@ -214,6 +214,7 @@ export default function ProviderSkills({ selectedProvider, currentProjects }: Pr
const [queuedFiles, setQueuedFiles] = useState<QueuedSkillFile[]>([]); const [queuedFiles, setQueuedFiles] = useState<QueuedSkillFile[]>([]);
const [submitError, setSubmitError] = useState<string | null>(null); const [submitError, setSubmitError] = useState<string | null>(null);
const [isSubmitting, setIsSubmitting] = useState(false); const [isSubmitting, setIsSubmitting] = useState(false);
const [justInstalled, setJustInstalled] = useState(false);
const [searchQuery, setSearchQuery] = useState(''); const [searchQuery, setSearchQuery] = useState('');
const [isAddDialogOpen, setIsAddDialogOpen] = useState(false); const [isAddDialogOpen, setIsAddDialogOpen] = useState(false);
const [showInstallPath, setShowInstallPath] = useState(false); const [showInstallPath, setShowInstallPath] = useState(false);
@@ -230,6 +231,7 @@ export default function ProviderSkills({ selectedProvider, currentProjects }: Pr
setSearchQuery(''); setSearchQuery('');
setIsAddDialogOpen(false); setIsAddDialogOpen(false);
setShowInstallPath(false); setShowInstallPath(false);
setJustInstalled(false);
}, [selectedProvider]); }, [selectedProvider]);
useEffect(() => { useEffect(() => {
@@ -357,6 +359,7 @@ export default function ProviderSkills({ selectedProvider, currentProjects }: Pr
}))); })));
await addSkills({ entries }); await addSkills({ entries });
setQueuedFiles([]); setQueuedFiles([]);
setJustInstalled(true);
setIsAddDialogOpen(false); setIsAddDialogOpen(false);
} catch (error) { } catch (error) {
setSubmitError(error instanceof Error ? error.message : 'Failed to import skills'); setSubmitError(error instanceof Error ? error.message : 'Failed to import skills');
@@ -369,6 +372,7 @@ export default function ProviderSkills({ selectedProvider, currentProjects }: Pr
if (open) { if (open) {
setSubmitError(null); setSubmitError(null);
setShowInstallPath(false); setShowInstallPath(false);
setJustInstalled(false);
setIsAddDialogOpen(true); setIsAddDialogOpen(true);
return; return;
} }
@@ -376,6 +380,7 @@ export default function ProviderSkills({ selectedProvider, currentProjects }: Pr
setQueuedFiles([]); setQueuedFiles([]);
setSubmitError(null); setSubmitError(null);
setShowInstallPath(false); setShowInstallPath(false);
setJustInstalled(false);
setIsAddDialogOpen(false); setIsAddDialogOpen(false);
}, []); }, []);
@@ -592,6 +597,7 @@ export default function ProviderSkills({ selectedProvider, currentProjects }: Pr
size="sm" size="sm"
className="h-8 w-8 p-0 text-muted-foreground hover:text-foreground" className="h-8 w-8 p-0 text-muted-foreground hover:text-foreground"
aria-label="Close add skill dialog" aria-label="Close add skill dialog"
disabled={isSubmitting}
onClick={() => handleAddDialogOpenChange(false)} onClick={() => handleAddDialogOpenChange(false)}
> >
<X className="h-4 w-4" /> <X className="h-4 w-4" />
@@ -605,7 +611,7 @@ export default function ProviderSkills({ selectedProvider, currentProjects }: Pr
<div className="flex flex-shrink-0 flex-col gap-3 border-t border-border/60 px-4 py-3 sm:flex-row sm:items-center sm:justify-between"> <div className="flex flex-shrink-0 flex-col gap-3 border-t border-border/60 px-4 py-3 sm:flex-row sm:items-center sm:justify-between">
<div className="min-w-0 flex-1"> <div className="min-w-0 flex-1">
{(submitError || loadError || saveStatus === 'success') ? ( {(submitError || loadError || (justInstalled && saveStatus === 'success')) ? (
<div className={cn( <div className={cn(
'max-h-24 overflow-y-auto whitespace-pre-wrap rounded-lg border px-3 py-2 text-sm', 'max-h-24 overflow-y-auto whitespace-pre-wrap rounded-lg border px-3 py-2 text-sm',
submitError || loadError submitError || loadError
@@ -626,6 +632,7 @@ export default function ProviderSkills({ selectedProvider, currentProjects }: Pr
variant="outline" variant="outline"
size="sm" size="sm"
className="w-full sm:w-auto" className="w-full sm:w-auto"
disabled={isSubmitting}
onClick={() => handleAddDialogOpenChange(false)} onClick={() => handleAddDialogOpenChange(false)}
> >
Cancel Cancel
@@ -651,7 +658,7 @@ export default function ProviderSkills({ selectedProvider, currentProjects }: Pr
</div> </div>
)} )}
{saveStatus === 'success' && !isAddDialogOpen && ( {justInstalled && saveStatus === 'success' && !isAddDialogOpen && (
<div className="inline-flex items-center gap-2 rounded-full border border-emerald-500/30 bg-emerald-500/10 px-3 py-1 text-xs font-medium text-emerald-700 dark:text-emerald-300"> <div className="inline-flex items-center gap-2 rounded-full border border-emerald-500/30 bg-emerald-500/10 px-3 py-1 text-xs font-medium text-emerald-700 dark:text-emerald-300">
<CheckCircle2 className="h-4 w-4" /> <CheckCircle2 className="h-4 w-4" />
Skills saved successfully. Skills saved successfully.

View File

@@ -47,6 +47,13 @@ export default function ActionMenu({
}: ActionMenuProps) { }: ActionMenuProps) {
const [isOpen, setIsOpen] = React.useState(false); const [isOpen, setIsOpen] = React.useState(false);
const rootRef = React.useRef<HTMLDivElement | null>(null); const rootRef = React.useRef<HTMLDivElement | null>(null);
const triggerRef = React.useRef<HTMLButtonElement | null>(null);
const menuRef = React.useRef<HTMLDivElement | null>(null);
// Whether closing should move focus back to the trigger. Set for keyboard
// (Escape) and item selection, but left false for outside pointer clicks so
// focus is not stolen from wherever the user clicked.
const restoreFocusRef = React.useRef(false);
const wasOpenRef = React.useRef(false);
const menuId = React.useId(); const menuId = React.useId();
React.useEffect(() => { React.useEffect(() => {
@@ -63,6 +70,7 @@ export default function ActionMenu({
const closeOnEscape = (event: KeyboardEvent) => { const closeOnEscape = (event: KeyboardEvent) => {
if (event.key === 'Escape') { if (event.key === 'Escape') {
restoreFocusRef.current = true;
setIsOpen(false); setIsOpen(false);
} }
}; };
@@ -75,11 +83,32 @@ export default function ActionMenu({
}; };
}, [isOpen]); }, [isOpen]);
// Move focus into the menu on open and back to the trigger on a keyboard or
// selection close, so keyboard and screen-reader navigation match the menu role.
React.useEffect(() => {
if (isOpen) {
wasOpenRef.current = true;
const menu = menuRef.current;
const firstItem = menu?.querySelector<HTMLButtonElement>('[role="menuitem"]:not([disabled])');
(firstItem ?? menu)?.focus();
return;
}
if (wasOpenRef.current) {
wasOpenRef.current = false;
if (restoreFocusRef.current) {
triggerRef.current?.focus();
}
restoreFocusRef.current = false;
}
}, [isOpen]);
const runItem = (item: ActionMenuItem) => { const runItem = (item: ActionMenuItem) => {
if (item.disabled || item.loading) { if (item.disabled || item.loading) {
return; return;
} }
restoreFocusRef.current = true;
setIsOpen(false); setIsOpen(false);
item.onSelect(); item.onSelect();
}; };
@@ -87,6 +116,7 @@ export default function ActionMenu({
return ( return (
<div ref={rootRef} className={cn('relative inline-flex', className)}> <div ref={rootRef} className={cn('relative inline-flex', className)}>
<Button <Button
ref={triggerRef}
type="button" type="button"
variant={variant} variant={variant}
size={size} size={size}
@@ -105,8 +135,10 @@ export default function ActionMenu({
{isOpen && ( {isOpen && (
<div <div
ref={menuRef}
id={menuId} id={menuId}
role="menu" role="menu"
tabIndex={-1}
className={cn( className={cn(
'absolute top-full z-50 mt-2 min-w-[220px] rounded-lg border border-border bg-popover p-1 text-popover-foreground shadow-lg', 'absolute top-full z-50 mt-2 min-w-[220px] rounded-lg border border-border bg-popover p-1 text-popover-foreground shadow-lg',
'animate-in fade-in-0 zoom-in-95', 'animate-in fade-in-0 zoom-in-95',