Skip to content

Commit dcc3cc7

Browse files
chore: remove redudant context from the skill
1 parent 97c62ef commit dcc3cc7

2 files changed

Lines changed: 40 additions & 45 deletions

File tree

.claude/skills/code-review/SKILL.md

Lines changed: 38 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -28,38 +28,47 @@ If refactor/docs/tests-only: changelog entry not required — confirm with autho
2828

2929
### Mendix-specific
3030

31-
- **XML ↔ TSX alignment**: lowerCamelCase keys, TS props updated with XML changes, unique widget ID
32-
- **Data API**: check `ActionValue.canExecute` before `execute()`, use `EditableValue.setValue()` for two-way binding, render loading/empty states until values are ready
31+
`AGENTS.md` covers the core rules (canExecute, loading states, lowerCamelCase XML keys). Flag these additional review issues:
32+
33+
- XML changed but TS props not updated, or widget ID is not unique
34+
- `EditableValue` read without checking `.status` — can render stale/undefined data
35+
- `ActionValue.execute()` called without checking `.canExecute` first
3336

3437
### React
3538

36-
- **Hooks**: correct `useEffect`/`useMemo`/`useCallback` deps; no stale closures; guard async effects:
39+
Flag these patterns — general React conventions are assumed known:
40+
41+
- Missing or wrong `useEffect`/`useMemo`/`useCallback` deps; stale closures
42+
- Async effect sets state without a cleanup guard:
3743
```ts
3844
useEffect(() => {
3945
let active = true;
40-
(async () => {
41-
const data = await load();
42-
if (active) setData(data);
43-
})();
46+
fetchData().then(data => {
47+
if (active) setState(data);
48+
});
4449
return () => {
4550
active = false;
4651
};
47-
}, [load]);
52+
}, [fetchData]);
4853
```
49-
- **State**: functional updates (`setX(x => x + 1)`); no mirroring props in state without sync logic; stable `key`s in lists (not array index)
50-
- **Props**: don't spread unknown props onto DOM nodes; prefer composition over prop drilling
54+
- Array index used as list `key`requires a stable unique key
55+
- Props spread onto DOM nodes (`<div {...props}>`) — strips unknown HTML attributes
5156

5257
### MobX
5358

54-
- `makeAutoObservable` or `makeObservable` in every store constructor
55-
- State mutations only inside `action`; `computed` must be pure (no side effects)
56-
- React integration via `observer` HOC or `useSubscribe()` from `@mendix/widget-plugin-mobx-kit`
59+
- `makeAutoObservable` or `makeObservable` missing from store constructor
60+
- State mutation outside `action`suggest `runInAction` or `action` wrapper
61+
- `computed` with side effectsmust be pure
62+
- Missing `observer` HOC or `useSubscribe()` from `@mendix/widget-plugin-mobx-kit` on React components that read MobX state
5763

5864
### Styling
5965

60-
- SCSS onlyno inline styles for static design
61-
- Atlas UI classes preferred (`btn`, `badge`); never override core Atlas classes
62-
- BEM-like naming prefixed with widget name; no `!important`
66+
Conventions are in `docs/requirements/frontend-guidelines.md`. Flag only deviations:
67+
68+
- Inline styles used for static design
69+
- Core Atlas UI classes overridden
70+
- `!important` used
71+
- CSS class not prefixed with widget name
6372

6473
### Unit tests
6574

@@ -168,37 +177,23 @@ expect(results.violations).toEqual([]);
168177
169178
### Accessibility
170179
171-
Follow WCAG 2.2 AA. Prefer semantic HTML over ARIAonly add ARIA when native elements don't convey the right semantics.
180+
Full requirements are in `docs/requirements/frontend-guidelines.md`. Flag only deviations:
172181
173-
- **Semantic elements**: use `<button>` for actions, `<a>` for navigation, `<dialog>` for modalsnot `<div onClick>`
174-
- **Keyboard navigation**: all interactive elements reachable and operable by keyboard; focus order is logical
175-
- Arrow keys for menu/list navigation
176-
- Enter/Space to activate
177-
- Escape to dismiss floating elements
178-
- Roving `tabIndex` pattern for lists/menus (`tabIndex=0` on active item, `-1` on others)
179-
- **ARIA labels**: interactive elements without visible text need `aria-label` or `aria-labelledby`; dynamic regions need `aria-live` where appropriate
180-
- **Focus management**: dialogs/modals must trap focus on open and restore it on close; use `FloatingFocusManager` from Floating UI for popovers/menus
181-
- **Images**: decorative images use `alt=""`; informative images have descriptive `alt` text
182-
- **Colour contrast**: minimum 4.5:1 for normal text, 3:1 for large text and UI components (AA)
183-
- **No content conveyed by colour alone**: pair colour cues with text or icons
182+
- Interactive element is a `<div>` or `<span>` with `onClick` — replace with `<button>` or `<a>`
183+
- Icon-only button missing `aria-label`
184+
- `<img>` missing `alt` attribute (use `alt=""` for decorative)
185+
- Dialog/modal does not trap focus or restore it on close — use `FloatingFocusManager`
186+
- Colour used as the sole differentiator — pair with text or icon
184187
185188
## Heuristics
186189
187-
| Situation | Comment |
188-
| ---------------------------------------------- | -------------------------------------------------------------------------------------------- |
189-
| Code/XML changed, no CHANGELOG entry | "Please add a changelog entry (`pnpm -w changelog`). Version bumps happen in a separate PR." |
190-
| Feature/fix without tests | "Please add unit tests in `src/components/__tests__/` and consider E2E tests." |
191-
| XML changed, TS props not updated | "XML props changed but TS types/usage aren't aligned." |
192-
| Async effect sets state without guard | Suggest the `active` flag pattern above |
193-
| `index` used as list `key` | Request a stable unique key |
194-
| MobX mutation outside `action` | Suggest `runInAction` or `action` wrapper |
195-
| `dangerouslySetInnerHTML` without sanitization | Flag as high severity; require DOMPurify or equivalent |
196-
| Hardcoded secret, token, or credential | Flag as critical; must be removed and rotated |
197-
| `javascript:` or `data:` URI from user input | Flag as XSS risk; require validation before use |
198-
| Interactive element is a `<div>` with onClick | Replace with `<button>` or `<a>`; add keyboard handler if div must be kept |
199-
| Missing `alt` on `<img>` | Add descriptive `alt` or `alt=""` for decorative images |
200-
| No `aria-label` on icon-only button | Add `aria-label` describing the action |
201-
| Colour used as sole differentiator | Pair with text or icon; check contrast ratio meets 4.5:1 (3:1 for large text) |
190+
| Situation | Severity | Comment |
191+
| ---------------------------------------------- | -------- | ------------------------------------------------------ |
192+
| Code/XML changed, no CHANGELOG entry | Medium | `pnpm -w changelog` — version bumps are a separate PR |
193+
| Feature/fix without tests | Medium | Add unit tests; consider E2E for interactive behaviour |
194+
| `dangerouslySetInnerHTML` without sanitization | High | Require DOMPurify or equivalent before merge |
195+
| Hardcoded secret, token, or credential | Critical | Must be removed and rotated immediately |
196+
| `javascript:` or `data:` URI from user input | High | Validate before use — XSS risk |
202197
203198
## Scope
204199

.github/workflows/ClaudeReview.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ jobs:
5858
5959
Follow the skill guidelines precisely. Focus only on changed files in the diff; ignore dist/, lockfiles, and generated files.
6060
claude_args: >-
61-
--model ${{ env.CLAUDE_MODEL }} --allowedTools "Bash(gh pr view:*),Bash(gh pr diff:*),Bash(gh pr comment:*),Bash(gh api:*),mcp__github_inline_comment__create_inline_comment,Read,Grep,Glob"
61+
--model ${{ env.CLAUDE_MODEL }} --allowedTools "Bash(gh pr view:*),Bash(gh pr diff:*),Bash(gh pr comment:*),Bash(gh api:*),Bash(git log:*),Bash(git diff:*),Bash(ls:*),Bash(find:*),mcp__github_inline_comment__create_inline_comment,Read,Grep,Glob"
6262
6363
interactive:
6464
name: Claude Interactive
@@ -106,4 +106,4 @@ jobs:
106106
- `AGENTS.md` — repo conventions, commands, and constraints
107107
- `.claude/skills/code-review/SKILL.md` — review checklist and heuristics
108108
claude_args: >-
109-
--model ${{ env.CLAUDE_MODEL }} --allowedTools "Bash(gh pr view:*),Bash(gh pr diff:*),Bash(gh pr comment:*),Bash(gh api:*),mcp__github_inline_comment__create_inline_comment,Read,Grep,Glob"
109+
--model ${{ env.CLAUDE_MODEL }} --allowedTools "Bash(gh pr view:*),Bash(gh pr diff:*),Bash(gh pr comment:*),Bash(gh api:*),Bash(git log:*),Bash(git diff:*),Bash(ls:*),Bash(find:*),mcp__github_inline_comment__create_inline_comment,Read,Grep,Glob"

0 commit comments

Comments
 (0)