From a584c220fec98336bad66ca5d60b21be35bb68fc Mon Sep 17 00:00:00 2001 From: Nayden Naydenov Date: Mon, 4 May 2026 13:03:18 +0300 Subject: [PATCH] chore: add skill for api review --- .claude/skills/ui5-api-review/SKILL.md | 153 ++++++++++++++++++ .../references/naming-patterns.md | 136 ++++++++++++++++ 2 files changed, 289 insertions(+) create mode 100644 .claude/skills/ui5-api-review/SKILL.md create mode 100644 .claude/skills/ui5-api-review/references/naming-patterns.md diff --git a/.claude/skills/ui5-api-review/SKILL.md b/.claude/skills/ui5-api-review/SKILL.md new file mode 100644 index 000000000000..0ebd4f966b8b --- /dev/null +++ b/.claude/skills/ui5-api-review/SKILL.md @@ -0,0 +1,153 @@ +--- +name: ui5-api-review +description: Reviews public API (properties, slots, events, methods) of UI5 Web Components for naming correctness, consistency with codebase conventions, and JSDoc completeness. Use this skill whenever someone asks to "check", "review", or "validate" a component API item — e.g. "check DateRangePicker#showTwoMonths", "review ViewSettingsDialog#reset-click", "check all new API on ViewSettingsDialog", or "does this property name follow project guidelines". Trigger on any reference in the form Component#apiName, a file path with a property/event/slot, or a request to validate naming conventions on new API before a release. +--- + +# UI5 Web Components — API Review Skill + +You are reviewing public API items in the UI5 Web Components monorepo for naming quality, convention consistency, and JSDoc completeness before a release. + +## How to use this skill + +**Input forms you'll receive:** +- `ComponentName#apiName` — e.g. `ViewSettingsDialog#reset-click` +- `ComponentName` alone — review all new API (typically `@since` the current release) +- A file path + property name +- Multiple items at once — e.g. "check enableReset, customTabs, and reset-click on ViewSettingsDialog" + +**Your job for each item:** +1. Locate the definition in the codebase +2. Classify the API type (property, slot, event, method) +3. Validate naming against conventions (see below) +4. Validate JSDoc completeness +5. Check consistency with sibling API on the same component +6. Output a verdict and, if needed, a concrete fix + +--- + +## Step 1 — Locate and classify + +Find the exact definition: decorator, type, default value, JSDoc, and where it is used in the template and component logic. + +For a batch request ("all new API on X"), find everything marked `@since ` on the component and its companion elements. + +**API types:** +| Type | Decorator | Identifies as | +|------|-----------|---------------| +| Property | `@property(...)` | A configurable attribute | +| Slot | `@slot(...)` | Accepted child elements | +| Event | `@event(...)` | A fired notification | +| Method | `public methodName(` | A callable action | + +--- + +## Step 2 — Validate naming + +### Start with the reference + +Read `references/naming-patterns.md` first. It contains curated patterns for the most common cases. If the pattern you need is there, use it directly — no codebase search needed. + +### Search the codebase when the pattern is missing + +If the naming pattern isn't in the reference (e.g. a novel kind of property or an unusual event shape), search `packages/main/src` and `packages/fiori/src` for comparable API. Look for: +- Properties with a similar semantic intent +- Events on the same component or sibling components +- Slots serving a similar structural role + +### Naming rules to apply + +**Properties — visibility/rendering:** +- Default-off visibility → `show*` (e.g. `showClearIcon`, `showFooter`) +- Default-on visibility → `hide*` (e.g. `hideWeekNumbers`, `hideArrow`) +- Never `enable*`/`disable*` for visibility + +**Properties — behavioral:** +- Preventing a physical interaction → `disable*` (e.g. `disableResizing`) +- Standard disabled state → `disabled` +- Never `enable*` — it implies toggling a feature's existence + +**Properties — icon accessible name:** +- On a host component exposing accessible text for an internal icon → `iconAccessibleName` +- Not `iconTooltip` (misleading — the value also drives screen reader output) + +**Events:** +- Named dialog/component actions → plain action verb: `confirm`, `cancel`, `reset`, `close`, `delete` +- Clicks on specific icon buttons or UI elements → `*-click`: `notifications-click`, `detail-click` +- **Never mix** these two patterns on the same component +- Consistency check: compare with sibling events on the same component + +**Slots:** +- Collections → plural noun: `sortItems`, `tabs`, `actions` +- User-extensible additions to built-in items → `additional*` preferred over `custom*` +- Structural/positional → semantic name: `header`, `footer`, `startContent` + +**General:** +- camelCase for properties and slots +- kebab-case for events +- Names must describe what the API does, not how it is implemented + +--- + +## Step 3 — Validate JSDoc + +Every `@public` API item must have: +- [ ] `@public` tag +- [ ] `@since X.Y.Z` tag +- [ ] `@default ` for properties (not slots/events) +- [ ] Description that explains **what** it does +- [ ] For non-obvious behavior: a `**Note:**` explaining **when** to use it or what caveats exist +- [ ] For slots: mention of the accepted element type and, if a companion element needs importing, a note with the import path +- [ ] For events with a detail payload: the payload fields documented + +--- + +## Step 4 — Check sibling consistency + +Before finalizing, check how the new API sits alongside existing API on the same component: +- Do events follow the same naming style as existing events? +- Do properties follow the same prefix/pattern as existing properties of the same semantic category? +- If a slot was added alongside a new companion element, do they form a coherent API pair? + +--- + +## Step 5 — Output + +For each API item, write a free-form analysis followed by a verdict tag. + +**Verdict tags:** +- `[PASS]` — naming and JSDoc are correct, no action needed +- `[FLAG]` — a concern worth discussing before release (inconsistency, ambiguity, minor JSDoc gap) +- `[RENAME]` — the name is wrong; a concrete alternative must be provided + +**For every `[FLAG]` or `[RENAME]`**, always include: +1. What is wrong and why (reference the relevant convention) +2. A concrete proposed fix (new name or JSDoc rewrite) +3. If a rename: note that tests, templates, and any documentation samples using the old name will also need updating + +**Format example:** + +--- +### `ComponentName#apiName` — [VERDICT] + +*Analysis paragraph explaining the finding.* + +**Proposed fix:** rename to `newName` because [reason tied to convention]. + +**Files to update:** template, cypress tests, website samples if any. + +--- + +## Batch requests + +When reviewing multiple related API items (e.g. all new API on a component), review them individually first, then add a short **Overall picture** section at the end summarizing: +- Which items need action before release +- Whether the items form a coherent feature API together (naming style, interaction model) +- Any cross-item inconsistencies not visible when reviewing items in isolation + +--- + +## Silent failure check + +While reviewing, also check: +- Does any slot silently ignore items that are missing a required property (e.g. filtering without a warning)? If so, flag it — a `console.warn` should be added. +- Does the component rely on a property being set for correct behavior but not document this as a `**Note:**`? Flag it. diff --git a/.claude/skills/ui5-api-review/references/naming-patterns.md b/.claude/skills/ui5-api-review/references/naming-patterns.md new file mode 100644 index 000000000000..ba75446ef05e --- /dev/null +++ b/.claude/skills/ui5-api-review/references/naming-patterns.md @@ -0,0 +1,136 @@ +# UI5 Web Components — API Naming Patterns Reference + +This file documents established naming conventions derived from the codebase. Use it as the first lookup before searching the codebase. + +--- + +## Properties + +### Visibility / Rendering toggles → `show*` / `hide*` + +Controls whether a UI element is rendered or visible. Dominant pattern across both `packages/main` and `packages/fiori`. + +**Prefix `show*`** — element is hidden by default, opt-in to show it: +- `showClearIcon` (Input, DatePicker, MultiComboBox, ComboBox) +- `showTooltip` (Icon, SliderBase) +- `showTickmarks` (SliderBase) +- `showColon` (Label) +- `showRecentColors`, `showMoreColors`, `showDefaultColor` (ColorPalette) +- `showSelectAll` (MultiComboBox) +- `showExceededText` (TextArea) +- `showValueHelpIcon` (MultiInput) +- `showTwoMonths` (DateRangePicker) +- `showSearchField`, `showNotifications`, `showProductSwitch`, `showFooter` (ShellBar, DynamicPage) + +**Prefix `hide*`** — element is shown by default, opt-in to hide it: +- `hideWeekNumbers` (Calendar, DatePicker) +- `hideNavigationArrows`, `hidePageIndicator` (Carousel) +- `hideIcon`, `hideCloseButton` (MessageStrip) +- `hideDeleteButton`, `hidePinButton`, `hideRetryButton` (UploadCollectionItem) +- `hideArrow` (Popover) +- `hideStateIcon` (Tag) +- `hideValue` (ProgressIndicator) +- `hideSideContent`, `hideMainContent` (DynamicSideContent) + +**Rule:** Choose `show*` when the feature is off by default; choose `hide*` when the feature is on by default. Never use `enable*`/`disable*` for visibility. + +--- + +### Behavioral / functional toggles → `disabled`, `readonly`, specific verb + +- Standard `disabled` attribute is used across all interactive elements (Button, Input, Select, etc.) +- `readonly` for read-only state +- `disableResizing` (FlexibleColumnLayout) — prevents a physical interaction +- `disableSearchCollapse` (ShellBar) — prevents a collapse behavior +- Avoid `enable*` prefix for properties — it implies toggling a feature's existence rather than controlling its state + +**Rule:** Use `disabled` for the standard disabled state. Use `disable*` only for preventing specific behaviors. Never use `enable*`. + +--- + +### Icon accessible name → `iconAccessibleName` (not `iconTooltip`) + +When a host component exposes a property to set the accessible name / tooltip of its internal icon: +- `iconAccessibleName` (TreeItemBase) — public property +- Private computed variants: `_iconAccessibleNameText`, `decIconTitle`, `incIconTitle` + +The `accessibleName` + `showTooltip` split on `ui5-icon` itself is the underlying model. Host properties should reflect the semantic purpose (`iconAccessibleName`), not just the visual one (`iconTooltip`). + +--- + +## Events + +### Action-verb events (no suffix) — preferred for named actions + +Used when the event represents a meaningful, named action or state change: +- `confirm`, `cancel`, `close`, `open` — dialog/popup lifecycle +- `delete`, `rename`, `terminate`, `retry` — item actions (UploadCollectionItem) +- `change`, `input`, `search`, `filter`, `sort` — value/state changes +- `toggle`, `select` — state toggles + +**Rule:** For dialog footer buttons and named component actions, always use a plain action verb with no suffix. + +### `*-click` suffix — for icon buttons and clickable UI elements + +Used when exposing a click on a specific named UI element (icon, logo, area), not a semantic action: +- `notifications-click`, `profile-click`, `logo-click`, `product-switch-click`, `search-button-click` (ShellBar) +- `expand-button-click`, `pin-button-click` (DynamicPageHeaderActions) +- `detail-click` (ListItem) +- `item-click`, `row-click` (List, Table, Tree) +- `overflow-click`, `display-area-click` (MediaGallery) +- `name-click` (TimelineItem) +- `arrow-click` (SplitButton) + +**Rule:** Use `*-click` only for icon buttons or clickable display elements. Never use it for dialog footer action buttons (those get plain verbs like `confirm`, `cancel`, `reset`). + +### Consistency within a component + +Events on the same component must follow the same naming pattern. Mixed patterns (e.g. `confirm` + `cancel` + `reset-click` on the same component) are always wrong. + +--- + +## Slots + +### Collection slots → plural noun matching the item type + +- `sortItems`, `filterItems`, `groupItems` (ViewSettingsDialog) +- `items` (TabContainer, List, Tree) +- `tabs` (UserSettingsItem) +- `actions` (various) + +### Custom / user-extensible slots → `custom*` or `additional*` + +- `customTabs` (ViewSettingsDialog) — user-provided tabs beyond built-ins +- `additionalContent` (UserSettingsAppearanceView) + +**Rule:** Prefer `additionalTabs` over `customTabs` when the items extend a set of built-in items of the same type (more idiomatic). `customTabs` is acceptable but `additionalTabs` better signals "added on top of built-ins". + +### Structural / positional slots → semantic name + +- `header`, `footer`, `content` (layout) +- `startContent`, `endContent`, `middleContent` (positioning) + +--- + +## JSDoc requirements for public API + +Every `@public` property, slot, and event must have: +- `@public` tag +- `@since X.Y.Z` tag +- `@default ` tag for properties (except slots) +- A description explaining **what** it does and, for non-obvious cases, **why** / when to use it +- For slots: a note about required imports if a companion element must be imported separately + +### Description quality + +- Properties controlling visibility: state the default behavior and what changes when set +- Properties controlling behavior: explain the use case, not just the mechanical effect +- Events: describe what user action triggers it; if it carries a detail payload, document the fields +- Slots: name the accepted element type(s); note ordering constraints if any + +--- + +## Silent failures to flag + +- A slot that silently ignores items missing a required property (e.g. filtering out items without `icon`) should emit a console warning +- A property whose effect is not obvious from the name alone needs a JSDoc `**Note:**` clarifying the behavior