This repository was archived by the owner on Jul 31, 2026. It is now read-only.
fix: preexisting bugs surfaced in agent review - #353
Merged
Conversation
…ssing aria-label Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lor index bugs - ExternalPill: add rel="noopener noreferrer" when target="_blank" - useAppLayoutKeys: functional updater for setCollapsed to prevent stale closure on rapid keypresses - Tabs: ?? over || for tab fallback so empty-string IDs are handled correctly - KeyHint: replace dismiss div with button for keyboard/a11y; add React.Fragment keys to mapped modifier/key arrays - UserAvatar: Math.abs() on color index to handle non-letter initials Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
key={modifier} / key={key} would collide if the same value appeared
twice in the array; key={value-index} guarantees uniqueness.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
alx-xo
force-pushed
the
alexm/fix-preexisting-bugs
branch
from
June 15, 2026 22:52
1efe197 to
d6b1038
Compare
Context API types setCollapsed as (collapsed: boolean) => void, not Dispatch<SetStateAction<boolean>>, so the functional updater form fails type-check. Use !collapsed directly and add it to the useCallback deps. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/components/AppLayout/useAppLayoutKeys.ts">
<violation number="1">
P1: Toggle uses captured `collapsed` instead of functional updater, causing stale-state behavior on rapid keypresses. Use updater form to make each key event apply to latest state.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes 11 preexisting bugs surfaced during automated code review of #352. Stacked on top of the oxfmt formatting branch — no formatting changes here, purely logic/accessibility/security fixes.
State / stale closure
SegmentedButton: addhoveredIdtouseMemodeps — memo was returning stalehighlightedstate after clicksTableProvider: removeexpandedRowKeysfromuseCallbackdeps — unnecessary sincetoggleExpandeduses the functional updater form(prev) => ...useAppLayoutKeys:setCollapsed(!collapsed)→setCollapsed((prev) => !prev)— captured stalecollapsedon rapid keypressesCorrectness
Tabs:||→??for tab ID fallback —||treated empty-string IDs as falsy and fell through to the first childLanguageIndicator: remove spurious standalonegapclass —gapalone is not a valid Tailwind utility;gap-2already provides spacingUserAvatar: wrap color index inMath.abs()— non-letter initials (numbers, symbols) produced a negative index; JS%preserves sign, returningundefinedfor the color classAccessibility
SearchBox: addaria-label="Clear search"andtype="button"to icon-only clear buttonKeyHint: replace dismissdivwith<button type="button" aria-label="Dismiss">—divis not keyboard-operableReact warnings
KeyHint: replace bare<>fragments in modifier/key maps with<React.Fragment key={value-index}>— eliminates missing-key warnings; compound key avoids collisions if a value appears twiceSecurity
ExternalPill: addrel="noopener noreferrer"whentarget="_blank"— prevents reverse-tabnabbingTest Plan