Taras/ux UI#10
Conversation
Defining VaultRow inside Sidebar created a new component type each render, remounting the subtree and dropping keyboard focus during filtering. Convert to renderVaultRow helper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces several UI and accessibility enhancements, including a new unified toast notification system (useAppToast), direct vault filtering and pinning in the sidebar, tab badges displaying item counts, and keyboard navigation support for tables and cards. Additionally, bulk deletion is now throttled to prevent API rate limiting, and the 'Purge' action is visually isolated in a 'Danger zone'. Feedback on these changes highlights two main areas for improvement: first, preventing keyboard event bubbling from interactive elements inside the table rows to avoid unintended selections; second, properly handling the promise returned by navigator.clipboard.writeText to prevent unhandled rejections and false success indicators.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
I am having trouble creating individual review comments. Click here to see my feedback.
src/components/common/ItemTable.tsx (137-142)
Keyboard events (like pressing Space) on interactive elements inside the row (such as the checkbox) will bubble up to the TableRow's onKeyDown handler. This triggers onSelect and calls e.preventDefault(), which can interfere with the default checkbox toggle behavior. We should ignore keydown events originating from interactive elements like inputs or buttons.
onKeyDown={(e) => {
const target = e.target as HTMLElement;
if (
target.tagName === 'INPUT' ||
target.tagName === 'BUTTON' ||
target.getAttribute('role') === 'checkbox'
) {
return;
}
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
onSelect?.(item);
}
}}
src/components/vault/VaultDashboard.tsx (161-166)
navigator.clipboard.writeText is an asynchronous operation that returns a Promise. If it fails (e.g., due to browser permissions or focus issues), it will result in an unhandled promise rejection, and the success toast will still be shown to the user. We should handle the promise resolution and rejection properly.
if (selectedVaultUri) {
navigator.clipboard.writeText(selectedVaultUri)
.then(() => {
setCopiedUri(true);
toast.success('Vault URI copied to clipboard');
setTimeout(() => setCopiedUri(false), 2000);
})
.catch((err) => {
toast.error('Failed to copy Vault URI', String(err));
});
}
No description provided.