Skip to content

Addon-docs: Extract SetValueButton shared component from control components#34263

Open
mixelburg wants to merge 7 commits into
storybookjs:nextfrom
mixelburg:refactor/extract-set-value-button
Open

Addon-docs: Extract SetValueButton shared component from control components#34263
mixelburg wants to merge 7 commits into
storybookjs:nextfrom
mixelburg:refactor/extract-set-value-button

Conversation

@mixelburg
Copy link
Copy Markdown

@mixelburg mixelburg commented Mar 22, 2026

Summary

The 'Set X' button pattern was duplicated across four control components in code/addons/docs/src/blocks/controls/. All four shared the same Button props (ariaLabel={false}, variant='outline', size='medium') with only the onClick handler and label text differing.

Closes #34211

Changes

  • New file: SetValueButton.tsx — a small shared component that accepts name, storyId, label, onClick, and disabled
  • Boolean.tsx, Number.tsx, Text.tsx, Object.tsx — replaced the duplicated Button blocks with <SetValueButton />

Bonus: Object's button now consistently uses variant='outline' and size='medium', matching the other three controls (previously these props were missing from the Object control's button).

Impact

Any future change to the button's appearance, accessibility attributes, or sizing now only needs to be made in one place.

Summary by CodeRabbit

  • Refactor
    • Unified the “Set …” buttons across Boolean, Number, Object, and Text controls so empty/undefined values now present a consistent, accessible button to set values. Standardizes labels, disabled-state handling, and click behavior for a more uniform user experience.
  • Tests
    • Switched the control tests to a DOM-like test environment for more reliable UI behavior testing.

…components

The 'Set X' button pattern (rendered when a control value is undefined)
was duplicated verbatim across four control files: Boolean, Number, Text,
and Object. The only differences were the button label and the onClick
handler.

Extract a shared SetValueButton component to eliminate the repetition.
As a side effect, Object's button now consistently uses variant='outline'
and size='medium', matching the other three controls.

Closes storybookjs#34211
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 22, 2026

📝 Walkthrough

Walkthrough

Replaces inline control-setter buttons in Boolean, Number, Object, and Text controls with a new exported SetValueButton helper; adds SetValueButton export and a test environment directive for helpers tests. Imports and button props adjusted accordingly.

Changes

Cohort / File(s) Summary
Control Components
code/addons/docs/src/blocks/controls/Boolean.tsx, code/addons/docs/src/blocks/controls/Number.tsx, code/addons/docs/src/blocks/controls/Object.tsx, code/addons/docs/src/blocks/controls/Text.tsx
Replaced inline Button used in value === undefined / no-data fallbacks with SetValueButton. Removed direct Button and getControlSetterButtonId usage; preserved click handlers and disabled={readonly} and moved labels into SetValueButton children.
Helper Module
code/addons/docs/src/blocks/controls/helpers.tsx
Added exported SetValueButton component (props: name, optional storyId, children, onClick, optional disabled) that wraps the internal Button and computes the control-setter id via getControlSetterButtonId.
Tests
code/addons/docs/src/blocks/controls/helpers.test.ts
Added Vitest environment directive (// @vitest-environment happy-dom) at the top of the test file; test logic unchanged.

Sequence Diagram(s)

(Skipped — changes are straightforward UI rendering swaps and a small new helper; no new multi-component control flow requiring a sequence diagram.)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs


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

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

@valentinpalkovic valentinpalkovic moved this to Empathy Queue (prioritized) in Core Team Projects Mar 24, 2026
Copy link
Copy Markdown
Contributor

@JReinhold JReinhold left a comment

Choose a reason for hiding this comment

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

Thanks. 🙏 Added a few suggestions.

Comment thread code/addons/docs/src/blocks/controls/SetValueButton.tsx Outdated
Comment thread code/addons/docs/src/blocks/controls/SetValueButton.tsx Outdated
@JReinhold JReinhold self-assigned this Mar 25, 2026
@JReinhold JReinhold moved this from Empathy Queue (prioritized) to In Progress in Core Team Projects Mar 27, 2026
- Move SetValueButton component directly into helpers.tsx (renamed from .ts)
- Replace label prop with children for more idiomatic React composition
- Update all consumers (Boolean, Object, Text, Number) to use merged import
- Delete separate SetValueButton.tsx file

Addresses review feedback from JReinhold on storybookjs#34263
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 `@code/addons/docs/src/blocks/controls/Text.tsx`:
- Line 8: The relative import of helper symbols getControlId and SetValueButton
in Text.tsx must include the explicit file extension; update the import
statement for './helpers' to include the correct extension that matches the
helpers module (e.g., './helpers.ts' or './helpers.tsx') so the import of
getControlId and SetValueButton uses an explicit extension per repo rules.
🪄 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: 66232d3d-25a0-4bf0-8fec-ca45267b9707

📥 Commits

Reviewing files that changed from the base of the PR and between fe43b00 and a9d487b.

📒 Files selected for processing (5)
  • code/addons/docs/src/blocks/controls/Boolean.tsx
  • code/addons/docs/src/blocks/controls/Number.tsx
  • code/addons/docs/src/blocks/controls/Object.tsx
  • code/addons/docs/src/blocks/controls/Text.tsx
  • code/addons/docs/src/blocks/controls/helpers.tsx
✅ Files skipped from review due to trivial changes (1)
  • code/addons/docs/src/blocks/controls/Object.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • code/addons/docs/src/blocks/controls/Number.tsx
  • code/addons/docs/src/blocks/controls/Boolean.tsx

import { styled } from 'storybook/theming';

import { getControlId, getControlSetterButtonId } from './helpers';
import { getControlId, SetValueButton } from './helpers';
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

Use explicit extension for this relative import.

Line 8 should include the file extension to match repository import rules.

Suggested fix
-import { getControlId, SetValueButton } from './helpers';
+import { getControlId, SetValueButton } from './helpers.tsx';

As per coding guidelines: **/*.{ts,tsx,js,jsx} requires explicit file extensions for relative code imports/exports.

📝 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
import { getControlId, SetValueButton } from './helpers';
import { getControlId, SetValueButton } from './helpers.tsx';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@code/addons/docs/src/blocks/controls/Text.tsx` at line 8, The relative import
of helper symbols getControlId and SetValueButton in Text.tsx must include the
explicit file extension; update the import statement for './helpers' to include
the correct extension that matches the helpers module (e.g., './helpers.ts' or
'./helpers.tsx') so the import of getControlId and SetValueButton uses an
explicit extension per repo rules.

@JReinhold JReinhold changed the title refactor(docs): extract SetValueButton shared component from control components Addon-docs: Extract SetValueButton shared component from control components Apr 23, 2026
@JReinhold JReinhold added maintenance User-facing maintenance tasks addon: docs ci:normal labels Apr 23, 2026
@storybook-app-bot
Copy link
Copy Markdown

storybook-app-bot Bot commented Apr 23, 2026

Package Benchmarks

Commit: 7776d43, ran on 5 May 2026 at 11:29:32 UTC

No significant changes detected, all good. 👏

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 `@code/addons/docs/src/blocks/controls/Object.tsx`:
- Line 11: The import statement in Object.tsx uses a bare relative path; update
it to include the explicit file extension per repo rules by importing from the
helpers file with its extension (e.g., replace the current import of
getControlId and SetValueButton from './helpers' with './helpers.ts' or
'./helpers.tsx' as appropriate) so the symbols getControlId and SetValueButton
are resolved with an explicit extension.
🪄 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: efe2c828-f29b-4106-aa0f-7623bc44f703

📥 Commits

Reviewing files that changed from the base of the PR and between 4bf1c9d and ae209b4.

📒 Files selected for processing (5)
  • code/addons/docs/src/blocks/controls/Boolean.tsx
  • code/addons/docs/src/blocks/controls/Number.tsx
  • code/addons/docs/src/blocks/controls/Object.tsx
  • code/addons/docs/src/blocks/controls/Text.tsx
  • code/addons/docs/src/blocks/controls/helpers.test.ts
✅ Files skipped from review due to trivial changes (1)
  • code/addons/docs/src/blocks/controls/helpers.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • code/addons/docs/src/blocks/controls/Boolean.tsx
  • code/addons/docs/src/blocks/controls/Number.tsx

import { styled, useTheme } from 'storybook/theming';

import { getControlId, getControlSetterButtonId } from './helpers';
import { getControlId, SetValueButton } from './helpers';
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.

🛠️ Refactor suggestion | 🟠 Major

Use explicit extension for this relative import.

The import path ./helpers should include the file extension to match repository import rules.

Suggested fix
-import { getControlId, SetValueButton } from './helpers';
+import { getControlId, SetValueButton } from './helpers.tsx';

As per coding guidelines: **/*.{ts,tsx,js,jsx} requires "explicit file extensions for relative code imports and exports in TypeScript source, such as './foo.ts' or './bar.tsx'".

📝 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
import { getControlId, SetValueButton } from './helpers';
import { getControlId, SetValueButton } from './helpers.tsx';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@code/addons/docs/src/blocks/controls/Object.tsx` at line 11, The import
statement in Object.tsx uses a bare relative path; update it to include the
explicit file extension per repo rules by importing from the helpers file with
its extension (e.g., replace the current import of getControlId and
SetValueButton from './helpers' with './helpers.ts' or './helpers.tsx' as
appropriate) so the symbols getControlId and SetValueButton are resolved with an
explicit extension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

addon: docs ci:normal maintenance User-facing maintenance tasks

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

Duplicate Code: "Set Value" Button Pattern Repeated Across 4 Control Components

4 participants