-
Notifications
You must be signed in to change notification settings - Fork 0
polish(console): a11y, finality lifecycle, loading, disabled-state, responsive #54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -134,7 +134,28 @@ export function Td({ children, align = "left", className = "" }: { children?: Re | |
| return <td className={`border-t border-border px-4 py-2.5 text-fg ${ALIGN[align]} ${className}`}>{children}</td>; | ||
| } | ||
| export function Tr({ children, onClick, className = "", ...rest }: { children: ReactNode; onClick?: () => void; className?: string } & Omit<HTMLAttributes<HTMLTableRowElement>, "onClick">) { | ||
| return <tr onClick={onClick} className={`${onClick ? "cursor-pointer hover:bg-border/30" : ""} ${className}`} {...rest}>{children}</tr>; | ||
| // A clickable row is keyboard-operable: focusable, Enter/Space activate it, and it | ||
| // shows an inset focus ring. Non-interactive rows stay out of the tab order. | ||
| return ( | ||
| <tr | ||
| onClick={onClick} | ||
| tabIndex={onClick ? 0 : undefined} | ||
| onKeyDown={ | ||
| onClick | ||
| ? (e) => { | ||
| if (e.key === "Enter" || e.key === " ") { | ||
| e.preventDefault(); | ||
| onClick(); | ||
| } | ||
| } | ||
| : undefined | ||
| } | ||
| className={`${onClick ? "cursor-pointer outline-none hover:bg-border/30 focus-visible:bg-border/30 focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-primary/40" : ""} ${className}`} | ||
| {...rest} | ||
| > | ||
| {children} | ||
| </tr> | ||
| ); | ||
|
greptile-apps[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| // ---------------------------------------------------------------- tabs | ||
|
|
@@ -209,7 +230,7 @@ export function CopyButton({ value }: { value: string }) { | |
| void copyTextToClipboard(value).then((ok) => setDone(ok ? "copied" : "blocked")); | ||
| setTimeout(() => setDone("idle"), 1200); | ||
| }} | ||
| className="rounded p-1 text-muted outline-none transition hover:bg-border/50 focus-visible:ring-2 focus-visible:ring-primary/40" | ||
| className="inline-flex h-6 w-6 items-center justify-center rounded text-muted outline-none transition hover:bg-border/50 focus-visible:ring-2 focus-visible:ring-primary/40" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Increase the copy control hit area to the required minimum.
🤖 Prompt for AI Agents |
||
| aria-label={done === "blocked" ? "Copy blocked" : done === "copied" ? "Copied" : "Copy"} | ||
| title={done === "blocked" ? "Copy blocked. Select the value manually." : done === "copied" ? "Copied" : "Copy"} | ||
| > | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve the row keyboard contract and ignore nested-control events.
Because
{...rest}is spread after this handler, a caller-providedonKeyDownortabIndexcan overwriteTr’s computed behavior.AuditLog.tsxsupplies both, and its row handler also receives bubbled keys from nested copy and receipt controls. Destructure/combine the caller handler, keep controlled props after the spread, and only activate the row whenevent.target === event.currentTarget.🤖 Prompt for AI Agents