Repository navigation
docs: dual-audience custom-component skill, helper API pages, drift fixes - #268
Conversation
…app-context first Moves the monorepo walkthrough to references/library-contributor.md and fixes template drift: forwardRef guidance, @storybook/react-vite import, required ConfigProvider prefix test, validSizes trap comment, SCSS register-everything rule, docs footer sections.
…-registered vars A wrapping Theme's bulmaVars cannot override vars that register-vars declares on the component's own selector; show the working override paths instead.
Makes the classname-prefix helpers and the 18 valid* arrays discoverable in the API docs and the generated catalog (85 -> 87 entries). Also fixes an unterminated import quote in classnames.md.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughUpdates custom-component guidance for app and monorepo workflows, adds a StatCard example and contributor reference, expands helper API documentation, and clarifies component-level theming variable overrides. ChangesCustom component guidance
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
skills/bestax-custom-component/examples/stat-card.tsxParsing error: "parserOptions.project" has been provided for Comment |
Preview DeploymentPreview URL: https://926f36f1.bestax.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
skills/bestax-custom-component/references/library-contributor.md (1)
34-36: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winImport
BulmaClassesPropsas a type.The template currently imports it in the value import list. Use an inline
typespecifier or a separateimport typestatement so copied examples remain compatible with strict TypeScript settings, matchingstat-card.tsx.🤖 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 `@skills/bestax-custom-component/references/library-contributor.md` around lines 34 - 36, Import BulmaClassesProps as a type-only import in the component template, using an inline type specifier or separate import type statement, while keeping useBulmaClasses as a value import; match the pattern used in stat-card.tsx.
🤖 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 `@docs/docs/api/helpers/useprefixedclassnames.md`:
- Around line 79-89: Add an explicit type-only React import for the HTML
attribute type used by the ChipProps interface, and update the interface to
reference that imported type while preserving the existing BulmaClassesProps
extension.
- Around line 56-69: Update the createPrefixedClassNames factory documentation
to declare a typed rest parameter using ClassValue[] instead of an untyped args
tuple. Correct the return descriptions so usePrefixedClassNames and
prefixedClassNames return a string, while createPrefixedClassNames returns a
classNames function.
In `@skills/bestax-custom-component/examples/stat-card.tsx`:
- Around line 83-99: The documented StatCard styling only applies accent
overrides for success and danger, despite StatCardProps.color supporting all
Bulma colors. Update the optional StatCard CSS selectors to map link, info, and
warning to their corresponding --bulma-* tokens alongside the existing variants,
or revise the StatCardProps.color contract to permit only the currently
supported colors.
In `@skills/bestax-custom-component/references/component-catalog.md`:
- Line 22: Update the introductory count in the component catalog to clarify
that 87 refers to all documented API entries, including 81 components and six
helper APIs; replace “87 documented components” with “87 documented API entries”
or explicitly state the breakdown.
In `@skills/bestax-custom-component/references/library-contributor.md`:
- Around line 280-283: Add sidebar_position to the MyComponent documentation
frontmatter template alongside title and sidebar_label, using the repository’s
expected positioning format.
- Line 13: Update the file-layout code fence in the library contributor
documentation to specify an explicit language, using ```text or ```plaintext
instead of an unlabeled fence.
---
Nitpick comments:
In `@skills/bestax-custom-component/references/library-contributor.md`:
- Around line 34-36: Import BulmaClassesProps as a type-only import in the
component template, using an inline type specifier or separate import type
statement, while keeping useBulmaClasses as a value import; match the pattern
used in stat-card.tsx.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ec01f336-4fbc-4399-a657-3a93a5104a9c
📒 Files selected for processing (13)
bulma-ui/CLAUDE.mddocs/docs/api/helpers/classnames.mddocs/docs/api/helpers/useprefixedclassnames.mddocs/docs/api/helpers/valid-values.mddocs/docs/skills/custom-component.mdxskills/README.mdskills/bestax-custom-component/SKILL.mdskills/bestax-custom-component/examples/stat-card.tsxskills/bestax-custom-component/references/api.mdskills/bestax-custom-component/references/component-catalog.mdskills/bestax-custom-component/references/library-contributor.mdskills/bestax-custom-component/references/patterns.mdskills/bestax-theming/references/css-variables.md
| // Factory: returns a classNames function bound to a fixed prefix | ||
| function createPrefixedClassNames(classPrefix: string): (...args) => string; // args: same union as classNames | ||
| ``` | ||
|
|
||
| ### Parameters | ||
|
|
||
| | Function | Parameter | Type | Description | | ||
| | -------------------------- | ------------- | --------------------- | -------------------------------------------------------------------------------------------------- | | ||
| | `usePrefixedClassNames` | `...args` | same as `classNames` | Class values to join. The `classPrefix` from `ConfigProvider` is applied to every resulting class. | | ||
| | `prefixedClassNames` | `prefix` | `string \| undefined` | Prefix to apply. When `undefined` (or empty), behaves exactly like `classNames`. | | ||
| | `prefixedClassNames` | `...args` | same as `classNames` | Class values to join. | | ||
| | `createPrefixedClassNames` | `classPrefix` | `string` | Prefix baked into the returned function. | | ||
|
|
||
| All three return a space-separated string of unique class names. With no `ConfigProvider` (or no `classPrefix` set), `usePrefixedClassNames` produces the same output as `classNames`. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix the factory signature and return description. createPrefixedClassNames should use a typed rest parameter (...args: ClassValue[]), and the summary should say the hook/plain function return a string while the factory returns a function.
🤖 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 `@docs/docs/api/helpers/useprefixedclassnames.md` around lines 56 - 69, Update
the createPrefixedClassNames factory documentation to declare a typed rest
parameter using ClassValue[] instead of an untyped args tuple. Correct the
return descriptions so usePrefixedClassNames and prefixedClassNames return a
string, while createPrefixedClassNames returns a classNames function.
| ```tsx | ||
| import { | ||
| usePrefixedClassNames, | ||
| useBulmaClasses, | ||
| classNames, | ||
| type BulmaClassesProps, | ||
| } from '@allxsmith/bestax-bulma'; | ||
|
|
||
| interface ChipProps | ||
| extends React.HTMLAttributes<HTMLSpanElement>, BulmaClassesProps { | ||
| color?: 'primary' | 'link' | 'info' | 'success' | 'warning' | 'danger'; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,180p' docs/docs/api/helpers/useprefixedclassnames.mdRepository: allxsmith/bestax
Length of output: 5897
Import the React HTML attribute type used by the example. React.HTMLAttributes needs an explicit type import here, otherwise the snippet isn’t self-contained in a standard TypeScript setup.
import {
usePrefixedClassNames,
useBulmaClasses,
classNames,
type BulmaClassesProps,
} from '`@allxsmith/bestax-bulma`';
+import type { HTMLAttributes } from 'react';
interface ChipProps
- extends React.HTMLAttributes<HTMLSpanElement>, BulmaClassesProps {
+ extends HTMLAttributes<HTMLSpanElement>, BulmaClassesProps {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ```tsx | |
| import { | |
| usePrefixedClassNames, | |
| useBulmaClasses, | |
| classNames, | |
| type BulmaClassesProps, | |
| } from '@allxsmith/bestax-bulma'; | |
| interface ChipProps | |
| extends React.HTMLAttributes<HTMLSpanElement>, BulmaClassesProps { | |
| color?: 'primary' | 'link' | 'info' | 'success' | 'warning' | 'danger'; | |
| import { | |
| usePrefixedClassNames, | |
| useBulmaClasses, | |
| classNames, | |
| type BulmaClassesProps, | |
| } from '`@allxsmith/bestax-bulma`'; | |
| import type { HTMLAttributes } from 'react'; | |
| interface ChipProps | |
| extends HTMLAttributes<HTMLSpanElement>, BulmaClassesProps { | |
| color?: 'primary' | 'link' | 'info' | 'success' | 'warning' | 'danger'; |
🤖 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 `@docs/docs/api/helpers/useprefixedclassnames.md` around lines 79 - 89, Add an
explicit type-only React import for the HTML attribute type used by the
ChipProps interface, and update the interface to reference that imported type
while preserving the existing BulmaClassesProps extension.
Source: Coding guidelines
| // Rung 2 (optional) — src/components/StatCard.css, imported from this file: | ||
| // | ||
| // .statcard { | ||
| // /* Component-scoped custom props initialized from Bulma tokens, so any | ||
| // ancestor (or <Theme>) can re-theme the card by overriding them. */ | ||
| // --statcard-accent: var(--bulma-primary); | ||
| // --statcard-radius: var(--bulma-radius); | ||
| // border-left: 0.25rem solid var(--statcard-accent); | ||
| // border-radius: var(--statcard-radius); | ||
| // } | ||
| // .statcard.is-success { --statcard-accent: var(--bulma-success); } | ||
| // .statcard.is-danger { --statcard-accent: var(--bulma-danger); } | ||
| // | ||
| // Only --bulma-*-derived values — never literal colors — so dark mode and | ||
| // Theme overrides keep working. Caveat: if the app uses the prefixed CSS | ||
| // flavor / ConfigProvider classPrefix, usePrefixedClassNames renders | ||
| // `bestax-statcard`; adjust the selectors (or use plain classNames). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Apply the accent for every declared color.
StatCardProps.color promises a Bulma color for both the icon and accent, but the optional CSS only overrides success and danger; link, info, and warning keep the primary accent.
Add selectors for the remaining variants or narrow the documented contract.
🤖 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 `@skills/bestax-custom-component/examples/stat-card.tsx` around lines 83 - 99,
The documented StatCard styling only applies accent overrides for success and
danger, despite StatCardProps.color supporting all Bulma colors. Update the
optional StatCard CSS selectors to map link, info, and warning to their
corresponding --bulma-* tokens alongside the existing variants, or revise the
StatCardProps.color contract to permit only the currently supported colors.
| escape-hatch variants of the convenience wrappers above them; see the Form docs. | ||
|
|
||
| 85 documented components. Generated from the API docs — every exported | ||
| 87 documented components. Generated from the API docs — every exported |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify what the 87 count includes.
The catalog contains 81 component entries plus six helper API entries, so “87 documented components” is inaccurate. Change this to “87 documented API entries” or state the component/helper breakdown.
🤖 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 `@skills/bestax-custom-component/references/component-catalog.md` at line 22,
Update the introductory count in the component catalog to clarify that 87 refers
to all documented API entries, including 81 components and six helper APIs;
replace “87 documented components” with “87 documented API entries” or
explicitly state the breakdown.
| Every custom component has five files. Mirror the existing names exactly (PascalCase TSX, | ||
| `_kebab.scss` partial): | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Specify the language for the file-layout code fence.
Use ```text (or ```plaintext) instead of an unlabeled fence to satisfy Markdown linting.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 13-13: 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 `@skills/bestax-custom-component/references/library-contributor.md` at line 13,
Update the file-layout code fence in the library contributor documentation to
specify an explicit language, using ```text or ```plaintext instead of an
unlabeled fence.
Source: Linters/SAST tools
| --- | ||
| title: MyComponent | ||
| sidebar_label: MyComponent | ||
| --- |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add sidebar_position to the documentation template.
Repository guidance requires doc pages to include expected frontmatter, especially title and sidebar_position; this template only demonstrates title and sidebar_label.
🤖 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 `@skills/bestax-custom-component/references/library-contributor.md` around
lines 280 - 283, Add sidebar_position to the MyComponent documentation
frontmatter template alongside title and sidebar_label, using the repository’s
expected positioning format.
Source: Coding guidelines
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| <Title size="6" textColor="grey" mb="1"> | ||
| {label} | ||
| </Title> | ||
| <Title size="3" mb="0"> | ||
| {value} | ||
| </Title> |
There was a problem hiding this comment.
Worked example emits misused, out-of-order headings — 🟡 Minor · Accessibility
What: Title renders a real heading element h{size} whenever size is set (only as="p" opts out — see Title.tsx: Tag = element === 'p' ? 'p' : validSize ? \h${validSize}` : element). So this card produces
Active users
followed by12,481
` — the metric label becomes a deeper heading than the value, and the numeric value is marked up as a section heading it isn't.Why it matters: This is a shipped skill example that agents copy verbatim, and the skill leans hard on accessibility. A dashboard of these StatCards floods the accessibility tree / document outline with dozens of out-of-order h6→h3 headings (heading-level jumps flagged by axe/WCAG 1.3.1), and screen-reader users navigating by heading land on data values announced as headings. Title here is being used purely for typographic scale.
Fix: Render both as <p> with as="p" — the is-{size} styling class is still applied, so the visual result is identical without the heading semantics:
| <Title size="6" textColor="grey" mb="1"> | |
| {label} | |
| </Title> | |
| <Title size="3" mb="0"> | |
| {value} | |
| </Title> | |
| <Title as="p" size="6" textColor="grey" mb="1"> | |
| {label} | |
| </Title> | |
| <Title as="p" size="3" mb="0"> | |
| {value} | |
| </Title> |
Why the styling is preserved
Title always adds is-${validSize} to the class list regardless of the tag, and only switches the rendered element to <p> when as="p". <Title as="p" size="3"> → <p class="title is-3">, so the size/weight styling is unchanged — only the semantics change from <h3> to <p>.
There was a problem hiding this comment.
Deep review — 1 finding
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🟡 Minor | Accessibility | Worked example uses Title size=… for typographic scale, so the label/value render as out-of-order <h6>/<h3> headings; pass as="p" |
skills/bestax-custom-component/examples/stat-card.tsx:72-77 |
Overall: This is a docs/skills-only change with no runtime code, and it is in good shape. I verified the load-bearing factual claims against source: the usePrefixedClassNames / prefixedClassNames / createPrefixedClassNames signatures (helpers/classNames.ts), all 18 valid* constants and their values (bulmaClassHelpers.ts), that every one of those constants is publicly re-exported (useBulmaClasses.tsx), the Icon/Title/Box props used in stat-card.tsx, and the corrected CSS-variable inheritance claim in css-variables.md (register-vars does declare --bulma-avatar-size on .avatar, so an inherited Theme value loses — the new text is right, the old was wrong). The catalog delta (85→87, ordering) and the fixed import quote in classnames.md are consistent. The only real defect is the heading-semantics issue in the copy-pasteable StatCard example, which agents will propagate — worth fixing since the skill itself preaches accessibility.
🏄 Mellow, well-scoped cleanup swell, brah — the facts all hold water and the drift fixes are legit. Only ripple is that stat-card using headings as font-size dials; drop an
as="p"on it and it is a clean ride to shore.
Fixes the deep-review finding on #268: the copy-pasteable example used bare <Title size> for visual scale, emitting h6/h3 headings that break the document outline; as="p" keeps the scale without the semantics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pohc8xLkdx4gwXkW3xd7up
Preview DeploymentPreview URL: https://5be34ab2.bestax.pages.dev |
|
@claude Full re-review of this PR at current head d97de20 (you have 60 turns, diff tools, and pnpm).
Finish by updating your comment with a findings table and an overall ship/fix-first verdict. Generated by Claude Code |
|
Claude encountered an error after 5m 6s —— View job Re-review in progress
|
Preview DeploymentPreview URL: https://bb3645ca.bestax.pages.dev |
There was a problem hiding this comment.
Deep review — 0 finding(s)
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| — | — | — | No blocking defects found. | — |
Overall: This is a docs/skills-only change (no runtime code), and every load-bearing factual claim verifies against source. I checked the public exports the new templates import, all 18 valid* constant values, Title's as="p"+size behavior and Icon's prop surface, the @storybook/react-vite majority (71 vs 10 files), the ConfigProvider prefix test path/assertions, that bulma is a genuine runtime dependency (so the rung-3 in-app Sass claim holds), and that the css-variables.md correction is itself accurate — register-vars really does target .avatar/.badge's own selector, so a wrapping Theme's inherited value does lose to it. The catalog is at 87 with both new helper entries, and the new API pages are reachable via the autogenerated api sidebar. Riskiest surface is the gen:catalog:check gate — CI enforces it and the committed catalog already contains the two new entries, so it should be green. Nothing here needs a human to block on; the deferred skill-examples/ refresh is correctly called out as follow-up.
🏄 Total cruise, dude — this PR just reshapes the guidance and the docs, no gnarly runtime waves to wipe out on. Facts all line up clean against the source, so paddle it out to the human and let 'em squash-merge. Good to go.
|
🎉 This PR is included in version 3.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 5.4.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
2 participants
Description
Part of the AI-enrichment plan (PR 2): fix guidance that was actively wrong for AI agents, and
make the custom-component skill work for its larger audience — app developers who got it from
npm create bestaxand cannot follow monorepo instructions.Affected package(s):
@allxsmith/bestax-bulma) — skills content + CLAUDE.md pointer (no runtime code)create-bestax) — picks the skill up automatically on its next build (sync-skills)@allxsmith/bestax-docs) — two new helper API pages, skill page introRelated Issue(s)
Refs #263. Also fixes item 7 of #266 (the css-variables Theme-override claim).
Type of Change
What changed
bestax-custom-componentis now dual-audience, app-context firstSKILL.md(389 → 159 lines) opens with a "Which context are you in?" fork: monorepo →references/library-contributor.md(new file, the old walkthrough moved verbatim);app → the new app path: public package imports, composition-first, and a styling ladder
(helper props → plain CSS on
--bulma-*vars with component-scoped custom props → optionalnpm i -D sassfor the full register-vars pattern, which works in a Vite app becausebulmais a runtime dep of the library). States plainly that scaffolded apps have no jest/storybook.
examples/stat-card.tsxapp-side worked example.Drift fixes (wrong guidance costs agents review rounds)
Applied to the moved contributor walkthrough:
forwardRef" → use it when consumers need the DOM node; match foldersiblings (reality: ~77
React.FCvs ~28forwardReffiles).@storybook/react-vite(68/78 real stories;@storybook/reactwas drift) and carries
descriptionon every argType (enforced by the meta-test in ci: add conformance gates for house conventions (listings, docs sections, SCSS, stories, inline-style) #267).ConfigProvider classPrefixtest (it was missing — a99%-coverage trap).
validSizestrap inlined as a comment on the size prop; SCSS section now says registerall themable values (durations/offsets too), prefer Bulma tokens
(
cv.getVar('radius-rounded'), never9999px), scheme-aware colors for dark mode.sections; frontmatter
title:is load-bearing forgen:catalog).Corrected a false claim in the theming skill
css-variables.mdsaid the new avatar/badge vars could be overridden via a wrappingTheme'sbulmaVars— they can't (register-vars declares them on the component's own selector, whichbeats inheritance). Now shows the working override paths. (#266 item 7)
Helpers are now discoverable
New API pages
helpers/useprefixedclassnames.md(+prefixedClassNames,createPrefixedClassNames) andhelpers/valid-values.md(all 18valid*constants, the(typeof validColors)[number]idiom, and the validSizes-vs-element-size trap). The generatedcomponent catalog picks both up (85 → 87 entries), so agents consuming the catalog can now
find the exact primitives the skill tells them to import. Drive-by: fixed an unterminated
import quote in
classnames.md.Checklist
CLAUDE.mdfiles are updated (bulma-ui walkthrough pointer)Test plan
pnpm run gen:catalog:check(catalog regenerated, 87 components)pnpm run format:checkbulma-ui/src/index.ts, helpersignatures in
src/helpers/classNames.ts, the 18 constants inbulmaClassHelpers.ts,register-vars selectors in
_avatar.scss/_badge.scss, vite-ts template contentsbulma-ui/src/skill-examples/showcase with the StatCardexample per
skills/CLAUDE.md🤖 Generated with Claude Code
Summary by CodeRabbit
usePrefixedClassNamesand added documentation for valid-value constants (e.g., valid colors/sizes/viewports).StatCardexample demonstrating Bulma helper props and class prefix behavior.