Skip to content

Commit 50d4e2a

Browse files
Remove duplicate section
1 parent 74c1511 commit 50d4e2a

1 file changed

Lines changed: 23 additions & 137 deletions

File tree

.agents/review-checklists/react/code-quality.md

Lines changed: 23 additions & 137 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ Maintainability
122122

123123
### Confidence Threshold
124124

125-
Flag JSX return statements >50 lines or those with multiple conditional branches (3+ `{condition && <...>}` or ternary operators). Simple single-element returns or small layout wrappers are exempt.
125+
Flag JSX return statements >25 lines or those with multiple conditional branches (3+ `{condition && <...>}` or ternary operators). Simple single-element returns or small layout wrappers are exempt.
126126

127127
### Exceptions / False Positives
128128

@@ -191,20 +191,30 @@ interface TabPanelProps {
191191
onChange: (tab: 'properties' | 'warnings') => void;
192192
}
193193

194-
const TabPanel: FC<TabPanelProps> = ({ active, onChange }) => (
195-
<div className="tabs" role="tablist">
196-
{['properties', 'warnings'].map(tab => (
194+
const TabPanel: FC<TabPanelProps> = ({ active, onChange }) => {
195+
const handlePropertiesClick = useCallback(() => onChange('properties'), [onChange]);
196+
const handleWarningsClick = useCallback(() => onChange('warnings'), [onChange]);
197+
198+
return (
199+
<div className="tabs" role="tablist">
197200
<button
198-
key={tab}
199201
role="tab"
200-
aria-selected={active === tab}
201-
onClick={() => onChange(tab as 'properties' | 'warnings')}
202+
aria-selected={active === 'properties'}
203+
onClick={handlePropertiesClick}
202204
>
203-
{tab.charAt(0).toUpperCase() + tab.slice(1)}
205+
Properties
204206
</button>
205-
))}
206-
</div>
207-
);
207+
<button
208+
role="tab"
209+
aria-selected={active === 'warnings'}
210+
onClick={handleWarningsClick}
211+
>
212+
Warnings
213+
</button>
214+
</div>
215+
);
216+
};
217+
TabPanel.displayName = 'TabPanel';
208218

209219
const DesignerPanel: FC<Props> = ({ data, showWarnings }) => {
210220
const [rightTab, setRightTab] = useState<'properties' | 'warnings'>('properties');
@@ -217,14 +227,15 @@ const DesignerPanel: FC<Props> = ({ data, showWarnings }) => {
217227
</div>
218228
);
219229
};
230+
DesignerPanel.displayName = 'DesignerPanel';
220231
```
221232

222233
### How to Detect
223234

224235
1. **Multiple conditional branches:** Count `{condition && <...>}` and ternary operators. 3+ suggests extraction.
225236
2. **Repeated patterns:** Similar button/input groups with duplicated props (e.g., `role="tab"`, `aria-selected`, `onClick`) → extract into component.
226237
3. **Multiple state sections:** Component with separate state for tabs/panels (e.g., `const [tab, setTab]` and `const [panel, setPanel]`) often signals multiple concerns.
227-
4. **Line count + nesting:** Return >50 lines OR deeply nested (3+ levels) conditional JSX → consider extraction.
238+
4. **Line count + nesting:** Return >25 lines OR deeply nested (3+ levels) conditional JSX → consider extraction.
228239

229240
## React components and hooks should have unit tests
230241

@@ -295,131 +306,6 @@ test('increments count when button clicked', async () => {
295306
2. If missing, flag if component has: hooks, event handlers, or conditional logic.
296307
3. If tests exist, verify they assert on behavior (not just render existence).
297308

298-
## Large JSX/TSX blocks should be extracted into separate components
299-
300-
**Urgency:** suggestion
301-
302-
### Category
303-
304-
Maintainability
305-
306-
### Confidence Threshold
307-
308-
Flag JSX return statements >25 lines or those with multiple conditional branches (3+ `{condition && <...>}` or ternary operators). Simple single-element returns or small layout wrappers are exempt.
309-
310-
### Exceptions / False Positives
311-
312-
- Do not flag simple, single-element returns or small layout wrappers.
313-
- Do not flag when extraction would require excessive prop-drilling that makes code less readable.
314-
315-
### Description
316-
317-
Large JSX blocks reduce readability, increase cognitive load, and make testing harder. Red flags: multiple conditional rendering branches, repeated patterns (like tab buttons with similar structure), deeply nested element hierarchies, and components managing multiple independent concerns.
318-
319-
### Anti-patterns to Flag
320-
321-
```tsx
322-
// ❌ Return block with multiple conditional branches and repeated patterns
323-
const DesignerPanel: FC<Props> = ({ data, showWarnings }) => {
324-
const [rightTab, setRightTab] = useState<'properties' | 'warnings'>('properties');
325-
326-
return (
327-
<div className="designer">
328-
{showWarnings && (
329-
<div className="tabs" role="tablist">
330-
<button
331-
role="tab"
332-
aria-selected={rightTab === 'properties'}
333-
onClick={() => setRightTab('properties')}
334-
>
335-
Properties
336-
</button>
337-
<button
338-
role="tab"
339-
aria-selected={rightTab === 'warnings'}
340-
onClick={() => setRightTab('warnings')}
341-
>
342-
Warnings
343-
</button>
344-
</div>
345-
)}
346-
{(!showWarnings || rightTab === 'properties') && (
347-
<div role="tabpanel">
348-
<PropertiesPanel data={data} />
349-
</div>
350-
)}
351-
{showWarnings && rightTab === 'warnings' && (
352-
<div role="tabpanel">
353-
<WarningsPanel data={data} />
354-
</div>
355-
)}
356-
</div>
357-
);
358-
};
359-
```
360-
361-
**Red flags in this example:**
362-
- 3 separate conditional render blocks
363-
- Repeated tab button structure with duplicated accessibility attrs
364-
- Multiple independent concerns (tabs, properties, warnings)
365-
- Mixed conditional logic (both &&-chaining and ternaries)
366-
367-
### Suggested Fix
368-
369-
Extract the tab control into a reusable component:
370-
371-
```tsx
372-
interface TabPanelProps {
373-
active: 'properties' | 'warnings';
374-
onChange: (tab: 'properties' | 'warnings') => void;
375-
}
376-
377-
const TabPanel: FC<TabPanelProps> = ({ active, onChange }) => {
378-
const handlePropertiesClick = useCallback(() => onChange('properties'), [onChange]);
379-
const handleWarningsClick = useCallback(() => onChange('warnings'), [onChange]);
380-
381-
return (
382-
<div className="tabs" role="tablist">
383-
<button
384-
role="tab"
385-
aria-selected={active === 'properties'}
386-
onClick={handlePropertiesClick}
387-
>
388-
Properties
389-
</button>
390-
<button
391-
role="tab"
392-
aria-selected={active === 'warnings'}
393-
onClick={handleWarningsClick}
394-
>
395-
Warnings
396-
</button>
397-
</div>
398-
);
399-
};
400-
TabPanel.displayName = 'TabPanel';
401-
402-
const DesignerPanel: FC<Props> = ({ data, showWarnings }) => {
403-
const [rightTab, setRightTab] = useState<'properties' | 'warnings'>('properties');
404-
405-
return (
406-
<div className="designer">
407-
{showWarnings && <TabPanel active={rightTab} onChange={setRightTab} />}
408-
{(!showWarnings || rightTab === 'properties') && <PropertiesPanel data={data} />}
409-
{showWarnings && rightTab === 'warnings' && <WarningsPanel data={data} />}
410-
</div>
411-
);
412-
};
413-
DesignerPanel.displayName = 'DesignerPanel';
414-
```
415-
416-
### How to Detect
417-
418-
1. **Multiple conditional branches:** Count `{condition && <...>}` and ternary operators. 3+ suggests extraction.
419-
2. **Repeated patterns:** Similar button/input groups with duplicated props (e.g., `role="tab"`, `aria-selected`, `onClick`) → extract into component.
420-
3. **Multiple state sections:** Component with separate state for tabs/panels (e.g., `const [tab, setTab]` and `const [panel, setPanel]`) often signals multiple concerns.
421-
4. **Line count + nesting:** Return >25 lines OR deeply nested (3+ levels) conditional JSX → consider extraction.
422-
423309
## Optional props that are always passed should be required
424310

425311
**Urgency:** suggestion

0 commit comments

Comments
 (0)