Repository navigation
UI Consistency: 2 issues found
Two findings, both posted as inline comments:
apps/web/src/components/DiffFilePathCopyButton.tsx— the new control is a raw<button>that reconstructsButtonsize="icon-micro"+variant="ghost"viaDIFF_HEADER_ICON_BUTTON_CLASS_NAME, losing the primitive's focus-visible ring (it setsfocus-visible:outline-hiddenwith no replacement), coarse-pointer hit target, and disabled styling, and diverging from the sibling collapse control andMessageCopyButton.apps/web/src/components/DiffPanel.tsx:924— the same raw class string is appended to aButtonthat already usesicon-micro/ghost, overriding the primitive's border (border-0) and transition (transition-colorsvstransition-shadow) and duplicating its geometry from the call site.
The shared anchoredCopyToast extraction and the renderHeaderFilenameSuffix prop threading (including the test update) look correct and are not flagged.
Details
Note
Your check run agent prompt is: .macroscope/check-run-agents/ui-consistency.md
More information about how Check Run Agents work can be found in our Help Center.
Methodology:
- Compared
DIFF_HEADER_ICON_BUTTON_CLASS_NAME(apps/web/src/components/diffs/diffHeaderControls.ts) againstbuttonVariantsinapps/web/src/components/ui/button.tsx;size="icon-micro"already yieldssize-5 rounded-sm p-0 [&_svg]:size-3andvariant="ghost"supplies transparent background plus accent hover, while the base recipe suppliescursor-pointer,inline-flex,shrink-0,focus-visible:ring-2 ring-ring ring-offset-1,disabled:*, andpointer-coarse:after:min-h-11/min-w-11. - Confirmed via
git showon the head merge commit thatmainhad already migrated the diff-header collapse control toButton size="icon-micro" variant="ghost"; the branch's raw-class constant was merged on top of it, so the class string now fights the primitive. - Checked
cn(twMerge) semantics:border-0andtransition-colorswin over the primitive'sborderandtransition-shadow; the focus ring survives on theDiffPanelcall site but is absent entirely on the raw button inDiffFilePathCopyButton. - Verified
renderHeaderFilenameSuffixis threaded consistently throughAnnotatableCodeViewtoStyledDiffCodeView, thatAnnotatableCodeView.test.tsxis the only other consumer of the now-required prop, and that the header row click handler inDiffPanelalready short-circuits onHTMLButtonElementso the new button does not double-trigger file open/collapse. - Verified the
anchoredCopyToasthelpers preserve the previous timeout, tooltip-style data, anchor positioning, and error description behavior of the code removed fromMessageCopyButton.
🔍 Agent activity — 1 tool call
💭 Looking at the code structure, I'm noticing that the new DiffFilePathCopyButton is using a raw button element with a shared class constant instead of a Button primitive, even though there are now two call sites with the same styling. The question is whether there's a Button variant that would be more appropriate here rather than duplicating the class-based approach. I also need to check if renderHeaderFilenameSuffix is being used elsewhere as a required prop on AnnotatableCodeView, since adding required props can break existing call sites. I'm noticing that DiffFilePathCopyButton is using a raw button element with a hand-rolled class string that mimics the Button primitive's ghost icon styling, which fits the pattern of a concrete finding. The tricky part is that this class name is also shared with the collapse button in DiffPanel, so I need to check how it's being used there to understand the full scope. I see that the new DiffFilePathCopyButton is using a raw button element that recreates the styling of Button with size="icon-micro" and variant="ghost", which matches the DIFF_HEADER_ICON_BUTTON_CLASS_NAME pattern exactly. But there's a real accessibility problem here: the raw class includes
focus-visible:outline-hiddenwhich removes the focus ring without providing a replacement. The Button component normally providesfocus-visible:ring-2 ring-ring ring-offsetfor keyboard navigation visibility, so this is a regression. Additionally, the raw button lacks the minimum 44px hit target for coarse-pointer devices that the Button primitive provides — both of these violate the accessibility rules about preserving focus rings and interaction semantics. There's also an issue where this class constant gets applied to the existing DiffPanel collapse Button at line 924, layering the raw class string onto a Button that already has size icon-micro, which creates conflicting focus styles. The collapse button got refactored on main to use the Button component with icon-micro size and ghost variant, but the PR branch had kept the raw button element with a custom class. When these merged together, the custom class got layered on top of the Button primitive, which is now overriding the Button's styling—removing its border, transitions, and focus states while creating redundant styles. The diff is showing the comparison from the merge base through to the current head. The real issue is that DIFF_HEADER_ICON_BUTTON_CLASS_NAME is being applied to the Button component, undoing its core geometry and focus-visible ring. The new DiffFilePathCopyButton is also using a raw button element that's trying to recreate what the Button primitive already provides, missing the focus ring and pointer-coarse hit target. I should verify where this class name is actually being used and check if the header row click handler properly avoids triggering on button clicks. I also need to confirm whetherrenderHeaderFilenameSuffixis a required prop on AnnotatableCodeView and if other consumers depend on it, plus verify that the underlying diff viewer library supports this prop. I need to verify whether StyledDiffCodeView already has the renderHeaderFilenameSuffix property, since the diff didn't modify it. Let me search the repo to see if this component exists and what it supports. Looking at the CodeView component from the pierre library, it seems renderHeaderFilenameSuffix is a prop that works similarly to the existing renderHeaderPrefix — both can render React nodes into the shadow root, and since Tailwind styles already work with the prefix through the Button primitive, the suffix should work the same way, likely via React portals into light DOM slots. Now I'm spotting some concrete issues: DiffFilePathCopyButton is using a raw<button>element instead of the Button primitive, which means it's missing the focus-visible ring styling and other accessibility features that the sibling collapse control and MessageCopyButton have. Additionally, in DiffPanel there's a class name override that's duplicating the Button primitive's core styles like border, transitions, and focus states — this looks like a merge artifact from when the code switched to using raw buttons. Let me trace through how the class merging works with Tailwind's cn utility to understand the exact style conflicts. The raw button also duplicates that same background styling, which is redundant. The icon color handling through the ghost Button's CSS variable is fine, but the raw button doesn't use it. The accessibility issue is more serious here — there's no focus ring at all on the raw button, so keyboard users won't see focus state. The aria-label and tooltip changes are appropriate, though the hover text doesn't update. For event handling, the header row's onClick returns early when it detects a button in the composed path, which works correctly even with shadow DOM nodes. The new copy button doesn't call stopPropagation, but that's fine since the row handler already returns early. I should verify the Tailwind class merging logic and check if getDiffCollapseIconClassName creates any conflicts. I'm noticing that DiffFilePathCopyButton uses a template string for its className instead of thecnutility, so it's not getting the benefits of tailwind-merge. The raw button also applies its class directly as a constant plus text-muted-foreground without going throughcn. I should check if there's a lint rule in the vite config that enforces using the Button primitive instead of raw buttons, similar to the StyledDiffCodeView rule I saw earlier. Looking at the lint rules and git history to see if there's precedent for copy buttons in diff headers, and checking the AGENTS.md guidance on using the Button primitive. The DiffFilePathCopyButton lacks a ring class for keyboard focus visibility, which is a problem. MessageCopyButton disables itself while copied, though that's optional. I should also verify the header's styling approach. Now I'm drafting the review comments. For the raw button on lines 24-31, I'll suggest replacing it with the Button component using icon-micro size and ghost variant, which requires both an import addition and JSX changes—so I'll provide a unified diff showing the replacement Button with proper styling and aria-label, plus note that the import needs to be added. For the second comment on line 924 in DiffPanel, I'm considering whether to suggest removing the DIFF_HEADER_ICON_BUTTON_CLASS_NAME from the className prop. Since only that line is in the diff hunk, I can use an inline suggestion to delete it or clean up the className attribute across the relevant lines. I should also note that if both comments are applied, the diffHeaderControls.ts file becomes unused and can be deleted entirely — I'll mention that in the first comment. Now I need to find the exact line numbers in the head file for DiffFilePathCopyButton to reference in my suggestions. Given the consistency issues and the regression risk from losing the focus ring while duplicating a primitive, this warrants a failed review. I'll write concise, targeted comments.
1. modify_pr — Two consistency issues around the new diff-header icon button. Both come from reconstructing the Button primitive as a raw class string instead of using the existing icon-micro / ghost contract.
Agent Credits: 76 credits