Skip to content

refactor: refactor Label with useRender; reuse in Field.Label#766

Merged
rohanchkrabrty merged 6 commits intomainfrom
worktree-label-component-removal
May 4, 2026
Merged

refactor: refactor Label with useRender; reuse in Field.Label#766
rohanchkrabrty merged 6 commits intomainfrom
worktree-label-component-removal

Conversation

@rohanchkrabrty
Copy link
Copy Markdown
Contributor

@rohanchkrabrty rohanchkrabrty commented Apr 29, 2026

Summary

  • Rebuild Label as a polymorphic, useRender-based component. Drops the old size variants and asterisk-style required/requiredIndicator; new props are required (renders an (optional) indicator when false) and optionalText (defaults to '(optional)'). CSS is typography-only with no margins/padding — pointer cursor is wired via .label[for] so layout is the consumer's responsibility (Flex).
  • Field.Label delegates to Label through the Base UI render prop. FieldLabelProps = FieldPrimitive.Label.Props & LabelProps, so future Label props flow through automatically; the (optional) indicator now lives in Label.
  • Aligns Field spacing with Figma (node 4908:11107): .field { gap: var(--rs-space-2) } (was --rs-space-1); strips padding-bottom from the label and padding-top from .description/.error; removes the now-redundant .label/.optional rules from field.module.css.
  • Migrates raw <label> callsites to Label (theme-customiser radio rows); updates Radio/Checkbox/Label playground examples and docs (index.mdx, props.ts, demo.ts) for the new API.
  • Bumps .scrub-area-label[for] specificity in NumberField so cursor: ew-resize wins over Label's [for] pointer rule. Label tests rewritten (11 cases); existing Field/NumberField tests still pass.

Removes the standalone Label component and folds its inline labeling role
into Field.Label via a new `orientation` prop ("vertical" | "horizontal").

- Field.Label gains orientation="horizontal" for inline use beside Radio
  and Checkbox controls (no padding-bottom, pointer cursor)
- Field.Label now falls back to a plain <label> when used outside a
  Field.Root (Base UI's primitive requires the context)
- NumberField.ScrubArea switches from Label to a plain <label> with the
  required typography moved into the scrub-area-label class
- Radio and Checkbox playground examples migrate from raw <label> to
  Field.Label orientation="horizontal" to demonstrate the new pattern
- Removes Label exports, tests, docs, and playground entry

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 29, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
apsara Ready Ready Preview, Comment May 4, 2026 10:16am

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 29, 2026

📝 Walkthrough

Walkthrough

The PR refactors the Label component API and implementation: removes size variants and requiredIndicator, adds optionalText?: string and a polymorphic render? prop, and changes required semantics so required={false} renders an optional indicator. Implementation moved from CVA/variant-driven code to a useRender/mergeProps approach and the CSS surface was simplified (added pointer/disabled attribute rules and an .optional class). Demos, playgrounds, and docs were updated to pair Label via htmlFor; Checkbox, Radio examples (now Radio.Group + Radio) and the theme customiser were adjusted; tests and a package barrel export for Label were added/updated.

Sequence Diagram(s)

(omitted)

Possibly related issues

Suggested reviewers

  • rsbh
  • paanSinghCoder
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: refactoring Label with useRender and reusing it in Field.Label, which aligns with the substantial changes across the codebase.
Description check ✅ Passed The description provides comprehensive details about the refactoring, covering the Label rebuild, new props, CSS changes, Field.Label delegation, spacing adjustments, and test updates—all directly relevant to the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

@rohanchkrabrty rohanchkrabrty changed the title refactor(label): remove standalone Label, add orientation to Field.Label chore: remove standalone Label, add orientation to Field.Label Apr 29, 2026
@rohilsurana rohilsurana changed the title chore: remove standalone Label, add orientation to Field.Label chore!: remove standalone Label, add orientation to Field.Label Apr 30, 2026
Copy link
Copy Markdown
Member

@rohilsurana rohilsurana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — clean consolidation of Label into Field.Label with good test coverage.

A few minor suggestions for consideration (non-blocking):

  • Commit prefix should probably be feat!: instead of chore: since removing the Label export is a breaking change for consumers.
  • The fallback <label> path could strip Base UI-specific props before spreading to avoid potential DOM warnings.
  • Consider a note in Field docs about Field.Label orientation="horizontal" as the replacement pattern for standalone Label usage with Radio/Checkbox.

@rohanchkrabrty rohanchkrabrty changed the title chore!: remove standalone Label, add orientation to Field.Label chore: remove standalone Label, add orientation to Field.Label Apr 30, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/www/src/components/playground/label-examples.tsx`:
- Around line 11-14: The Label examples use htmlFor='pg-label-email' and
htmlFor='pg-label-name' but there are no corresponding input elements, breaking
label association; update the playground so each <Label> in label-examples.tsx
is paired with a control that has the matching id (e.g., add inputs/selects with
id="pg-label-email" and id="pg-label-name") or change the htmlFor values to
match the existing control ids—modify the Label usages and/or add the missing
input elements so the Label component's htmlFor matches a rendered control.

In `@apps/www/src/components/playground/radio-examples.tsx`:
- Around line 20-21: The Label next to the disabled Radio (Radio value='3'
id='P3' disabled) doesn't reflect the disabled state; update the Label for the
disabled radio to carry a data-disabled attribute (or equivalent prop) when its
associated Radio has disabled set so it receives the correct styling and pointer
behavior; locate the Radio/Label pair (Radio id='P3', Label htmlFor='P3') in
radio-examples.tsx and change the Label to conditionally include data-disabled
(or pass disabled) when the Radio uses disabled.

In `@apps/www/src/content/docs/components/label/props.ts`:
- Around line 1-19: The docs interface LabelProps is missing the optionalText
prop that the component exposes; update the LabelProps interface to include
optionalText?: string with a brief JSDoc comment (e.g., "Custom text to display
for optional indicator") so it matches the runtime props used in
packages/raystack/components/label/label.tsx (look for optionalText usage in
that file) and keep other fields (required, htmlFor, render, className)
unchanged.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 16621b1a-b292-4745-8935-e69997118a19

📥 Commits

Reviewing files that changed from the base of the PR and between fddcf49 and 3b01c1d.

📒 Files selected for processing (13)
  • apps/www/src/components/playground/checkbox-examples.tsx
  • apps/www/src/components/playground/label-examples.tsx
  • apps/www/src/components/playground/radio-examples.tsx
  • apps/www/src/components/theme-customiser/theme-customiser.tsx
  • apps/www/src/content/docs/components/label/demo.ts
  • apps/www/src/content/docs/components/label/index.mdx
  • apps/www/src/content/docs/components/label/props.ts
  • packages/raystack/components/label/__tests__/label.test.tsx
  • packages/raystack/components/label/index.ts
  • packages/raystack/components/label/index.tsx
  • packages/raystack/components/label/label.module.css
  • packages/raystack/components/label/label.tsx
  • packages/raystack/components/number-field/number-field.module.css
💤 Files with no reviewable changes (1)
  • packages/raystack/components/label/index.tsx

Comment thread apps/www/src/components/playground/label-examples.tsx
Comment thread apps/www/src/components/playground/radio-examples.tsx
Comment thread apps/www/src/content/docs/components/label/props.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/www/src/content/docs/components/label/index.mdx`:
- Line 51: Replace the phrase "Maintains WCAG compliant color contrast ratios"
with the hyphenated compound adjective "Maintains WCAG-compliant color contrast
ratios" in the Label component docs (search for that exact text in the index.mdx
content for the Label docs).
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3897425e-a930-4c16-a85b-5695037c3e36

📥 Commits

Reviewing files that changed from the base of the PR and between 3b01c1d and a5da2bc.

📒 Files selected for processing (3)
  • apps/www/src/content/docs/components/label/demo.ts
  • apps/www/src/content/docs/components/label/index.mdx
  • apps/www/src/content/docs/components/label/props.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/www/src/content/docs/components/label/demo.ts
  • apps/www/src/content/docs/components/label/props.ts

- Renders a semantic HTML `<label>` element by default
- Supports programmatic association with form controls via `htmlFor`
- Required indicators are properly hidden from screen readers
- Maintains WCAG compliant color contrast ratios
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Hyphenate “WCAG-compliant” in the accessibility bullet.

Line 51 should use the compound adjective form for correct docs grammar.

Suggested patch
-- Maintains WCAG compliant color contrast ratios
+- Maintains WCAG-compliant color contrast ratios
📝 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.

Suggested change
- Maintains WCAG compliant color contrast ratios
- Maintains WCAG-compliant color contrast ratios
🧰 Tools
🪛 LanguageTool

[grammar] ~51-~51: Use a hyphen to join words.
Context: ... controls via htmlFor - Maintains WCAG compliant color contrast ratios - Shows ...

(QB_NEW_EN_HYPHEN)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/www/src/content/docs/components/label/index.mdx` at line 51, Replace the
phrase "Maintains WCAG compliant color contrast ratios" with the hyphenated
compound adjective "Maintains WCAG-compliant color contrast ratios" in the Label
component docs (search for that exact text in the index.mdx content for the
Label docs).

@rohanchkrabrty rohanchkrabrty changed the title chore: remove standalone Label, add orientation to Field.Label refactor(label): rebuild Label with useRender; reuse in Field.Label May 4, 2026
@rohanchkrabrty rohanchkrabrty changed the title refactor(label): rebuild Label with useRender; reuse in Field.Label refactor: refactor Label with useRender; reuse in Field.Label May 4, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/V1-migration.md`:
- Line 2039: The checklist link text "- [ ] Wrap InputField usages with
`<Field>` — move `label`, `helperText`, `error`, `optional` to Field props (see
[InputField](`#inputfield`))" points to a non-existent anchor; update the anchor
target to the actual section anchor by replacing `#inputfield` with
`#input-formerly-inputfield` (or alternatively rename the section anchor to
`#inputfield`) so the checklist jump works; locate the checklist line containing
"InputField" and fix the link target accordingly.

In `@packages/raystack/components/field/field-misc.tsx`:
- Around line 6-19: The FieldLabel component currently assigns render on
FieldPrimitive.Label but then spreads {...props} afterwards, allowing a
caller-provided props.render to override the intended wrapper; update FieldLabel
to extract render from the incoming props (i.e. const { render, ref, required,
optionalText, children, ...rest } = props or destructure render out in the
function signature), then pass the explicit Label wrapper into
FieldPrimitive.Label while spreading only the remaining props (rest) so callers
cannot bypass the Label behavior; ensure you reference FieldLabel,
FieldPrimitive.Label and Label and remove render from the spread so
optionalText/styling always apply.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f9c31527-fa53-4cf4-8a45-ae3f0d9d55f0

📥 Commits

Reviewing files that changed from the base of the PR and between a5da2bc and 6e758ae.

📒 Files selected for processing (4)
  • docs/V1-migration.md
  • packages/raystack/components/field/field-misc.tsx
  • packages/raystack/components/field/field.module.css
  • packages/raystack/components/number-field/number-field.module.css
✅ Files skipped from review due to trivial changes (1)
  • packages/raystack/components/field/field.module.css
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/raystack/components/number-field/number-field.module.css

Comment thread docs/V1-migration.md
Comment thread packages/raystack/components/field/field-misc.tsx
@rohanchkrabrty rohanchkrabrty merged commit 235c4e9 into main May 4, 2026
5 checks passed
@rohanchkrabrty rohanchkrabrty deleted the worktree-label-component-removal branch May 4, 2026 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants