feat(rules): flag a text input inside ButtonGroup - #28
Conversation
ButtonGroup joins its children into one shape — its variants apply rounded-l-none and border-l-0 to every child after the first — and its whole answer to focus is [&>*]:focus-visible:relative plus [&>*]:focus-visible:z-10, which lifts the focused child above its neighbours rather than lighting the group. focus-within appears nowhere in it. That is correct for buttons, which share a geometry and a ring. Around a text input it reads as broken. The input's ring is drawn around its own box, including the edge flush against the button, so the ring runs down the middle of a control that looks like a single pill. InputGroup is the component for that composition: its wrapper reacts to descendant focus and InputGroupInput gives up its own ring. The rule points there rather than asking anyone to patch ButtonGroup. The rule descends the whole subtree instead of reading direct children, because the shape that actually ships wraps the control in a form primitive (ButtonGroup > FormControl > Input); a direct-children scan misses every real instance. It reports only text entry — ButtonGroup is an open container that legitimately holds buttons, selects, separators and arbitrary wrappers, and radio, checkbox and submit inputs carry no offset ring. The existing anatomy machinery was the wrong tool: it reads direct children only, and it reports any unrecognised child as a violation, which an open container cannot satisfy. Ships advisory. Burn-in across nine real shadcn projects found four instances and no false positives, but all nine share one author, so the sample is not independent enough to move anyone's score yet. Ruleset 2026.07.42. Catalog regenerated to 60 rules; both hardcoded counts updated (packed smoke and the public-API test).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughChangesA new advisory rule detects text-entry controls nested within shadcn ButtonGroup advisory rule
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ProjectConfig
participant SourceFiles
participant buttonGroupHoldsOnlyButtonsRule
participant AuditResult
ProjectConfig->>buttonGroupHoldsOnlyButtonsRule: provide shadcn configuration
SourceFiles->>buttonGroupHoldsOnlyButtonsRule: provide JSX ButtonGroup usages
buttonGroupHoldsOnlyButtonsRule->>buttonGroupHoldsOnlyButtonsRule: traverse descendants and classify controls
buttonGroupHoldsOnlyButtonsRule->>AuditResult: return audit result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/cli/test/public-api.test.ts (1)
10-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winName the expected catalog length.
The literal
60is an unlabelled catalog contract. DefineEXPECTED_RULE_CATALOG_LENGTHbeside the suite and use it in the assertion.Proposed change
+const EXPECTED_RULE_CATALOG_LENGTH = 60; + describe("public library API", () => { - expect(publicApi.RULE_CATALOG).toHaveLength(60); + expect(publicApi.RULE_CATALOG).toHaveLength( + EXPECTED_RULE_CATALOG_LENGTH + );As per coding guidelines, use meaningful names instead of magic numbers, extracting constants with descriptive names.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/test/public-api.test.ts` at line 10, Define an EXPECTED_RULE_CATALOG_LENGTH constant beside the test suite in public-api.test.ts, set it to the current catalog contract value, and replace the literal 60 in the RULE_CATALOG length assertion with that named constant.Source: Coding guidelines
plans/019-button-group-holds-only-buttons.md (1)
3-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the completed plan to match the implementation.
The plan says this rule uses
evaluateComponentAnatomyand aComponentAnatomyManifest. The shipped rule instead defines a customfindTextControlandscanFiletraversal. Record this design choice and why the custom traversal was selected.Also applies to: 130-151
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plans/019-button-group-holds-only-buttons.md` around lines 3 - 8, The completed plan incorrectly describes the implementation as using evaluateComponentAnatomy and ComponentAnatomyManifest. Update the plan to document the shipped findTextControl and scanFile custom traversal, including the rationale for choosing this targeted traversal instead of the shared anatomy machinery.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/rules/button-group-holds-only-buttons.ts`:
- Around line 130-151: Update findTextControl’s visit traversal to inspect only
actual rendered JSX descendants, excluding JSX nested within opening-element
attributes such as callback expressions. Preserve detection through wrapper
elements like FormControl, and add the requested fixture verifying a ButtonGroup
with JSX inside onClick plus a Button child returns pass.
In `@plans/019-button-group-holds-only-buttons.md`:
- Line 48: Add a language info string to the opening fenced code blocks on lines
48, 56, and 69, using text or the most accurate syntax identifier for each
example, so the Markdown passes MD040.
In `@plans/README.md`:
- Line 25: Update the Plan 019 row in the plans index to include its landing
commit hash if the plan has merged; otherwise change its status from DONE to IN
PROGRESS. Preserve the existing plan link and metadata, and record the commit in
the designated landing-commit column.
---
Nitpick comments:
In `@packages/cli/test/public-api.test.ts`:
- Line 10: Define an EXPECTED_RULE_CATALOG_LENGTH constant beside the test suite
in public-api.test.ts, set it to the current catalog contract value, and replace
the literal 60 in the RULE_CATALOG length assertion with that named constant.
In `@plans/019-button-group-holds-only-buttons.md`:
- Around line 3-8: The completed plan incorrectly describes the implementation
as using evaluateComponentAnatomy and ComponentAnatomyManifest. Update the plan
to document the shipped findTextControl and scanFile custom traversal, including
the rationale for choosing this targeted traversal instead of the shared anatomy
machinery.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: af5e8a01-bcab-49ba-a7b9-031b92ac69b5
⛔ Files ignored due to path filters (1)
lib/generated/rule-catalog.jsonis excluded by!**/generated/**
📒 Files selected for processing (11)
CHANGELOG.mddocs/rules.mdpackages/cli/scripts/smoke-package.mjspackages/cli/src/anatomy.tspackages/cli/src/rules/button-group-holds-only-buttons.tspackages/cli/src/rules/default-rules.tspackages/cli/src/scan.tspackages/cli/test/button-group-holds-only-buttons.test.tspackages/cli/test/public-api.test.tsplans/019-button-group-holds-only-buttons.mdplans/README.md
| // Descends the whole subtree rather than reading direct children: the shape | ||
| // that actually ships wraps the control in a form primitive | ||
| // (ButtonGroup > FormControl > Input), and a direct-children scan misses it. | ||
| const visit = (node: Node): void => { | ||
| if (hit) { | ||
| return; | ||
| } | ||
|
|
||
| const element = getOpeningElement(node); | ||
|
|
||
| if (element && isTextControl(element, controls)) { | ||
| hit = { | ||
| label: `<${getJsxTagName(element)}>`, | ||
| line: getLineNumber(file, element), | ||
| }; | ||
| return; | ||
| } | ||
|
|
||
| forEachChild(node, visit); | ||
| }; | ||
|
|
||
| forEachChild(root, visit); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'button-group-holds-only-buttons\.ts$' . || true
echo "== git status/stat =="
git status --short || true
echo "== file outline =="
file="$(fd 'button-group-holds-only-buttons\.ts$' . | head -n 1 || true)"
if [ -n "${file:-}" ]; then
wc -l "$file"
ast-grep outline "$file" || true
echo "== relevant section =="
sed -n '1,220p' "$file" | nl -ba
fi
echo "== imports/functions names in repo =="
rg -n "function forEachChild|declare .*forEachChild|forEachChild\(" . --glob '*.ts' --glob '*.tsx' | head -n 80Repository: TheOrcDev/shadscan
Length of output: 1097
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Behavioral probe: inspect local tooling declarations and run a static AST read if TypeScript package is present locally.
echo "== local TS package availability =="
if [ -d node_modules/typescript ]; then
node - <<'JS'
const ts = require('typescript');
const version = require('typescript/package.json').version;
console.log('typescript version', version);
const source = `
function ButtonGroup() {}
function Input() {}
function setPreview(x) {}
function setFallback() {}
<ButtonGroup onClick={() => setPreview(<Input />)}><Button /></ButtonGroup>
`;
const sf = ts.createSourceFile('probe.tsx', source, ts.ScriptTarget.Latest, true, ts.ScriptKind.TSX);
console.log('root kind', ts.SyntaxKind[sf.kind]);
console.log('root statements count', sf.statements.length);
function visit(node, pad='') {
const tag = ts.SyntaxKind[node.kind] + (node.pos === node.end ? '' : ` [${node.pos}..${node.end}]`);
console.log(pad + tag);
if (node.jsxElem && !ts.isNode(node)) throw new Error('non-node');
if (node.kind === ts.SyntaxKind.JsxOpeningElement || node.kind === ts.SyntaxKind.JsxAttributes) {
for (const child of node.getChildren(sf)) {
visit(child, pad + ' ');
}
} else if (node.children && node.children.length) {
for (const child of node.children) {
if (!ts.isNode(child)) {
console.log(pad + ' TEXT token', child.kind, typeof child.text);
continue;
}
visit(child, pad + ' ');
}
}
}
const btn = sf.statements[3];
console.log('ButtonGroup element kind', ts.SyntaxKind[btn.kind]);
console.log('ButtonGroup attributes children:');
const attrs = btn.getElementsByTagName()[0].getTypeArgument ? null : btn.getTypeArgument; // not AST; fallback manual
console.log('has openingElement', !!btn.openingElement);
const el = btn.getElement().jsxElem || btn.openingElement || btn;
console.log('tag name', el.tagName && el.tagName.getText(sf));
for (const child of btn.getChildren(sf)) {
console.log('child kind', ts.SyntaxKind[child.kind]);
}
const open = btn.openingElement;
console.log('opening element kind', open && ts.SyntaxKind[open.kind]);
for (const child of open.getChildren(sf)) {
console.log('opening child kind', ts.SyntaxKind[child.kind], child.getText && child.getText(sf));
}
JS
else
echo "no local node_modules/typescript"
fiRepository: TheOrcDev/shadscan
Length of output: 225
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="./packages/cli/src/rules/button-group-holds-only-buttons.ts"
echo "== relevant source =="
tmp="$(mktemp)"
cat "$file" | awk '{printf "%5d %s\n", NR, $0}' > "$tmp"
sed -n '1,244p' "$tmp"
echo "== TypeScript compiler package availability =="
node - <<'JS'
try {
const typescript = require('typescript');
console.log('available', typescript.version);
} catch (e) {
console.log('missing', e.code || e.message);
}
JS
echo "== AST behavior probe if TypeScript is available =="
node - <<'JS'
try {
const ts = require('typescript');
const source = '<ButtonGroup onClick={() => setPreview(<Input />)}><Button /></ButtonGroup>';
const sf = ts.createSourceFile('probe.tsx', source, ts.ScriptTarget.Latest, true, ts.ScriptKind.TSX);
const btn = sf.statements[0];
console.log('root statement kind', ts.SyntaxKind[btn.kind]);
console.log('openingElement kind', ts.SyntaxKind[btn.openingElement.kind]);
let forEachChildren = 0;
let openingChildren = [];
const visit = (node) => {
forEachChildren++;
if (node.kind === ts.SyntaxKind.JsxOpeningElement) {
const tokens = [];
btn.openingElement.forEachChild((child) => {
tokens.push(ts.SyntaxKind[child.kind]);
});
openingChildren = tokens;
}
btn.openingElement.forEachChild(visit);
};
visit(btn.openingElement);
console.log('openingChildCount', openingChildren.length);
console.log('openingChildren', openingChildren.join(', '));
console.log('jsxExpressionInsideProp count', btn.openingElement.forEachChild((child) => {
return child.kind === ts.SyntaxKind.JsxExpression;
}).filter(Boolean).length === undefined ? 'collector not returned' : 'collector used');
// Collect JSX expressions attached to opening element attributes under a simple visitor pattern.
const textControlNodes = [];
const foundJsxExpr = [];
ts.forEachChild(btn.openingElement, (child) => {
if (child.kind === ts.SyntaxKind.JsxExpression) {
foundJsxExpr.push(true);
}
});
console.log('openingForEachChild finds jsx expressions', foundJsxExpr.length > 0);
} catch (e) {
console.log('probe skipped', e.message);
}
JSRepository: TheOrcDev/shadscan
Length of output: 8980
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const ts = require('typescript');
const source = '<ButtonGroup onClick={() => setPreview(<Input />)}><Button /></ButtonGroup>';
const sf = ts.createSourceFile('probe.tsx', source, ts.ScriptTarget.Latest, true, ts.ScriptKind.TSX);
const btn = sf.statements[0];
console.log('source files', sf.statements.length);
console.log('ButtonGroup open kind', ts.SyntaxKind[btn.openingElement.kind]);
const openingTokens = [];
btn.openingElement.forEachChild((child) => {
openingTokens.push({
kind: ts.SyntaxKind[child.kind],
text: child.getText && child.getText(sf),
});
});
console.log('opening foreach count', openingTokens.length);
const exprCount = openingTokens.filter((t) => t.kind === 'JsxExpression').length;
const exprContent = openingTokens
.filter((t) => t.kind === 'JsxExpression')
.map((t) => t.text)
.join(',');
console.log('opening foreach jsx expression count', exprCount);
console.log('opening foreach jsx expression content', exprContent);
const jsxElementNodes = [];
function findJsxElements(node) {
if (ts.isJsxElement(node) || ts.isJsxSelfClosingElement(node)) {
jsxElementNodes.push(ts.SyntaxKind[node.tagName.kind] + ':' + node.tagName.getText(sf));
}
if (ts.isElementAccessExpression(node)) {
for (const child of node.getChildren(sf)) {
findJsxElements(child);
}
} else if (node.getChildren) {
for (const child of node.getChildren(sf)) {
findJsxElements(child);
}
}
}
findJsxElements(btn.openingElement);
console.log('opening element find JSX descendants', jsxElementNodes.join(','));
JSRepository: TheOrcDev/shadscan
Length of output: 911
🌐 Web query:
TypeScript forEachChild JsxOpeningLikeElement attributes children documentation
💡 Result:
In the TypeScript Compiler API, JsxOpeningLikeElement is a union type comprising JsxOpeningElement and JsxSelfClosingElement [1][2]. These elements represent the opening tag of a JSX element (e.g., <div...>) [1]. Attributes The attributes of a JsxOpeningLikeElement are accessed via the.attributes property, which returns an instance of JsxAttributes [1][2]. JsxAttributes itself extends ObjectLiteralExpressionBase and provides a.properties collection, which contains the individual attributes [1][3]. Each property in this collection is typically a JsxAttribute or a JsxSpreadAttribute [1][2]. You can traverse or manipulate these attributes by accessing the.properties array [3][4]. Children It is important to note that a JsxOpeningLikeElement does not directly contain a children property because it represents only the opening tag [1][2]. The children of a JSX expression belong to the parent JsxElement (or JsxFragment), which contains the opening element, the closing element, and an array of children [1][2]. To access the children associated with a JSX expression: 1. Locate the parent JsxElement or JsxFragment [1]. 2. Access the.children property on that parent node, which is a NodeArray of JsxChild nodes [1][2]. Using forEachChild The ts.forEachChild function is a utility that allows you to visit the child nodes of any AST node [5]. When you call forEachChild on a JsxOpeningLikeElement, it will visit its immediate AST children, such as its tagName and its JsxAttributes node [6][7]. It will not automatically traverse into the "children" of the JSX element (the content between the tags), as those are structurally separate from the opening element in the TypeScript AST [1]. If you need to retrieve all nodes in the AST hierarchy, the.getChildren method can be used, though forEachChild is generally preferred for efficient traversal during tree processing [5].
Citations:
- 1: https://github.com/microsoft/TypeScript/blob/b57b5fe5ee31a334a1e2b63eced4ba5a046bff61/lib/typescript.d.ts
- 2: https://github.com/Microsoft/TypeScript/blob/ee25cdecbca49b2b5a290ecd65224f425b1d6a9c/lib/typescript.d.ts
- 3: https://github.com/microsoft/TypeScript/blob/0aac72020ee8414218273f654eb7ce1dc2dd0d6b/src/compiler/transformers/jsx.ts
- 4: https://stackoverflow.com/questions/63871563/how-to-add-extra-props-in-jsx-components-during-tsc-compile
- 5: https://basarat.gitbook.io/TypeScript/overview/ast/ast-tip-children
- 6: https://thejohnfreeman.github.io/TypeScript/interfaces/jsxopeningelement.html
- 7: https://github.com/dots-cljs/dots-typescript/blob/v5.4.5-10/src/dots/typescript/jsx_opening_element.cljs
🌐 Web query:
TypeScript compiler API forEachChild JsxOpeningElement JsxAttributes JsxExpression properties
💡 Result:
In the TypeScript Compiler API, traversing the AST is typically performed using the ts.forEachChild(node, callback) function [1][2]. When applied to a JsxOpeningElement, this function recursively visits the child nodes of the element [1][2]. JsxOpeningElement Structure The JsxOpeningElement interface represents the opening tag of a JSX element (e.g., <div...>) [3][4]. Its key properties include: tagName: A JsxTagNameExpression representing the name of the tag [4]. typeArguments: An optional NodeArray of type arguments [4]. attributes: A JsxAttributes object [4]. JsxAttributes and JsxExpression The attributes property of a JsxOpeningElement is of type JsxAttributes [4]. JsxAttributes acts as a container for the attributes defined within the opening tag [4]. JsxAttributes holds a properties array (inherited from ObjectLiteralExpressionBase), which contains instances of JsxAttribute or JsxSpreadAttribute [5][4]. If an attribute's value is a dynamic expression (e.g., prop={value}), that value is represented as a JsxExpression node [5][3]. Traversing Children When you use ts.forEachChild on a JsxOpeningElement, it iterates through its direct children, which typically include: The tagName [4]. The typeArguments (if present) [4]. The attributes (as a single JsxAttributes node) [4]. To inspect the individual attributes or expressions within the JsxAttributes node, you must either: 1. Continue the recursion into the JsxAttributes node using ts.forEachChild [1]. 2. Directly access the properties array on the attributes node (node.attributes.properties) to process JsxAttribute, JsxSpreadAttribute, or JsxExpression nodes specifically [5][4]. For example, when transforming or linting, you would typically check if a node is a JsxOpeningElement, then access its attributes.properties to handle individual attributes and their potential JsxExpression values [5].
Citations:
- 1: https://github.com/microsoft/TypeScript/wiki/Using-the-Compiler-API
- 2: https://microsoft-typescript.mintlify.app/api/parser
- 3: https://github.com/Microsoft/TypeScript/blob/7b48a182c05ea4dea81bab73ecbbe9e013a79e99/src/compiler/types.ts
- 4: https://github.com/Microsoft/TypeScript/blob/be2db9db12bd58f02d78f8a906f98f5ee9527663/src/compiler/types.ts
- 5: https://github.com/microsoft/TypeScript/blob/main/src/compiler/transformers/jsx.ts
🌐 Web query:
TypeScript for...of AST node array children forEachChild jsx opening element attributes
💡 Result:
In the TypeScript Compiler API, a JsxOpeningElement node contains an attributes property, which holds a NodeArray of JSX attributes [1][2]. To access and iterate over these attributes, you should interact with the attributes property directly rather than relying solely on forEachChild [3][4]. Key points for working with JSX attributes and forEachChild: 1. Direct Access: The JsxOpeningElement interface includes a property named attributes [1][2]. This property is an instance of NodeArray (specifically, JsxAttributes), which contains the list of JSXAttribute or JsxSpreadAttribute nodes [5][4]. You can access this array directly (e.g., node.attributes) to iterate, filter, or modify the attributes [5][4]. 2. forEachChild Limitations: While ts.forEachChild is a standard utility for traversing an AST, it is designed to visit child nodes that are part of the structural tree [6][7]. In many cases, it may not automatically visit or yield specific NodeArray collections like the attributes list of a JSX element in the way you might expect for custom traversal logic [7][8]. If you need to perform operations on the attributes, it is best practice to access the attributes property explicitly [4]. 3. Example Usage: // Accessing attributes directly from a JsxOpeningElement node if (node.kind === ts.SyntaxKind.JsxOpeningElement || node.kind === ts.SyntaxKind.JsxSelfClosingElement) { const openingElement = node as ts.JsxOpeningLikeElement; openingElement.attributes.forEach((attr) => { // Logic for processing each JSXAttribute or JsxSpreadAttribute console.log(attr); }); } If you require a more comprehensive tree traversal that includes all nodes (including tokens and specific arrays), the.getChildren method on a Node can be used to retrieve all children, though it is more verbose and includes tokens that forEachChild often skips [7][8].
Citations:
- 1: https://thejohnfreeman.github.io/TypeScript/interfaces/jsxopeningelement.html
- 2: https://github.com/dots-cljs/dots-typescript/blob/v5.4.5-10/src/dots/typescript/jsx_opening_element.cljs
- 3: Finding list of attributes from JSX Elements using the Compiler API microsoft/TypeScript#12598
- 4: https://github.com/NetEase/tango/blob/c48a49961a3b0e8b49f8a617d893e0b4f047147f/packages/core/src/helpers/ast/traverse.ts
- 5: https://github.com/microsoft/TypeScript/blob/main/src/compiler/transformers/jsx.ts
- 6: https://github.com/microsoft/TypeScript/wiki/Using-the-Compiler-API
- 7: https://isme2n.gitbooks.io/typescript/content/docs/compiler/ast-tip-children.html
- 8: https://basarat.gitbook.io/TypeScript/overview/ast/ast-tip-children
Skip JSX inside ButtonGroup attribute expressions.
findTextControl() visits opening elements and uses forEachChild(), so dynamic JSX inside props such as onClick={() => setPreview(<Input />)} is treated as a descendant of ButtonGroup. Restrict the scan to actual rendered JSX children/attribute expressions and add a fixture where <ButtonGroup onClick={() => setPreview(<Input />)}><Button /></ButtonGroup> returns pass.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/rules/button-group-holds-only-buttons.ts` around lines 130 -
151, Update findTextControl’s visit traversal to inspect only actual rendered
JSX descendants, excluding JSX nested within opening-element attributes such as
callback expressions. Preserve detection through wrapper elements like
FormControl, and add the requested fixture verifying a ButtonGroup with JSX
inside onClick plus a Button child returns pass.
| `-ml-px`, no `overflow-hidden`. The call site is clean. All the joining | ||
| happens inside `ButtonGroup` itself: | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Declare languages for the fenced examples.
Lines 48, 56, and 69 have opening fences without an info string. markdownlint-cli2 reports MD040 for each. Add text or an accurate syntax identifier.
Also applies to: 56-56, 69-69
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 48-48: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plans/019-button-group-holds-only-buttons.md` at line 48, Add a language info
string to the opening fenced code blocks on lines 48, 56, and 69, using text or
the most accurate syntax identifier for each example, so the Markdown passes
MD040.
Source: Linters/SAST tools
| | [017](017-stats-page.md) | /stats page: npm usage charted with Bklit | P2 | M | - | DONE (`e2e0c51`) | | ||
| | [018](018-mcp-server.md) | MCP server: typed audit tools for coding agents | P1 | L | - | PLANNED | | ||
| | [018](018-mcp-server.md) | MCP server: typed audit tools for coding agents | P1 | L | - | DONE (`bb95ec7`) | | ||
| | [019](019-button-group-holds-only-buttons.md) | ButtonGroup holds buttons, not text inputs | P2 | S | - | DONE | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the landing commit for Plan 019.
Line 5 says every completed plan records its landing commit. Line 25 marks Plan 019 DONE without one. If no landing commit exists yet, keep the status IN PROGRESS and update this row after merge.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plans/README.md` at line 25, Update the Plan 019 row in the plans index to
include its landing commit hash if the plan has merged; otherwise change its
status from DONE to IN PROGRESS. Preserve the existing plan link and metadata,
and record the commit in the designated landing-commit column.
1 participant
Adds
button-group-holds-only-buttons(advisory, ruleset2026.07.42), implementing plan 019.The defect
shadcn's
ButtonGroupjoins its children into one shape — its own variants applyrounded-l-noneandborder-l-0to every child after the first. Its entire answer to focus is:focus-withinappears zero times in it. It lifts the focused child above its neighbours so the child's own ring paints over the adjacent border. That's correct for buttons — identical geometry, identical rings, lifting one reads as "this segment".It breaks around a text input. The input's ring is drawn around its own box including the edge flush against the button, so the ring runs vertically down the middle of a control that reads as a single pill, with the button hanging outside it.
InputGroupis the component for that composition: its wrapper carrieshas-[[data-slot=input-group-control]:focus-visible]:border-b-ringandInputGroupInputcarriesfocus-visible:ring-0, so the child delegates its ring upward and the whole shape lights up. The rule points there rather than asking anyone to patchButtonGroup.Two findings that changed the implementation
It must descend the subtree, not read direct children. The shape that actually ships is
ButtonGroup > FormControl > Input— the control is nested. A direct-children scan misses every real instance. That's the first test in the file.The existing anatomy machinery was the wrong tool.
evaluateComponentAnatomyreads direct children only (anatomy.ts:229), and it reports any unrecognised child as a violation.InputGrouphas a closed anatomy so that fits;ButtonGroupis an open container that legitimately holds buttons, selects, separators and arbitrary wrappers, and would have false-positived onFormControlalone. Hence a purpose-built walk that reports only text entry.Scope
Reports
Input,Textarea, native<input>and<textarea>inside aButtonGroupresolved through the ui alias. Radio, checkbox, submit, reset, range, image and hidden inputs are excluded — they're buttons, not text entry, and carry no offset ring. An unreadabletypeis treated as text, since that's the HTML default.Burn-in: 9 real projects, 4 hits, 0 false positives
orcdevkit-newsletter-form.tsx:128✅ true positivegithub-creaturegithub-form.tsx:109✅ true positivestar-historyrepo-search.tsx:84✅ true positiveyoutubetoblogmain-form.tsx:86✅ true positive8bitcn,agent-starter-react,i-f-love-shadcn,shipper-club,sponsors-shipper-clubAll four hits verified by reading the source — every one is a genuine
ButtonGroup > Input + Button.Why it ships advisory despite a clean burn-in
Plan 019 said promote to
maxScore: 2on zero false positives, and the burn-in delivered zero. I'm not promoting, because the burn-in surfaced something the plan didn't anticipate: the pattern appears in 4 of 9 projects, and all nine share one author. "Zero false positives" here measures the rule against one person's idioms, which isn't an independent enough sample to start moving everyone's score — and the defect is cosmetic, so scoring it at 2 would misprice it next tofocus-visible-not-suppressed(a real a11y failure) at 3.Promotion stays a one-line change with the trap documented:
fail()atconfidence: "low"is silently auto-downgraded back to advisory (audit.ts:541), somaxScore,confidenceand the result helper must move together. There's a test asserting the current calibration.Verification
11 gates green. 10 new tests — the nested
FormControlshape, directInput, native<input>,Textarea, plus silence on segmented button groups, non-text controls,InputGroup, a non-ui-aliasButtonGroup, and a project with nocomponents.json. Self-audit 100/100 A.Catalog regenerated to 60 rules. Two hardcoded counts needed updating — the packed smoke assertion (which plan 019 flagged) and
test/public-api.test.ts:10(which it did not; found by the test run).Summary by CodeRabbit
New Features
ButtonGroupcomponents.InputGroupfor text-entry controls.Documentation
2026.07.42, increasing the total to 60 rules.ButtonGrouprule.