Skip to content

Conversation

@emmanuel-ferdman
Copy link

@emmanuel-ferdman emmanuel-ferdman commented Oct 1, 2025

User description

PR Summary

Previously, toggling the fullWidth prop in the Storybook Props Table had no visible effect because the Overview story didn't provide a full-width container for the component. This fix updates the Overview story to conditionally wrap ButtonGroup in a full-width container when fullWidth={true}.

Resolves #3130.


PR Type

Bug fix


Description

  • Conditionally wrap ButtonGroup in full-width container when fullWidth prop is true

  • Remove unused buttonGroupTemplate helper function

  • Enable proper fullWidth prop visualization in Storybook


Diagram Walkthrough

flowchart LR
  A["ButtonGroup Story"] -->|fullWidth true| B["Wrapped in 100% width div"]
  A -->|fullWidth false| C["Rendered directly"]
  D["Remove unused template"] -.-> A
Loading

File Walkthrough

Relevant files
Bug fix
ButtonGroup.stories.tsx
Add conditional full-width wrapper to ButtonGroup story   

packages/docs/src/pages/components/ButtonGroup/ButtonGroup.stories.tsx

  • Replaced buttonGroupTemplate helper with inline render function
  • Added conditional wrapper that applies 100% width div when
    fullWidth={true}
  • Removed unused template binding and improved story render logic
  • Maintains all existing args and component behavior
+4/-4     

@qodo-free-for-open-source-projects
Copy link
Contributor

qodo-free-for-open-source-projects bot commented Oct 1, 2025

PR Reviewer Guide 🔍

(Review updated until commit dc68f76)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅

3130 - PR Code Verified

Compliant requirements:

  • The fullWidth prop now visually affects the Overview story by wrapping the ButtonGroup in a 100% width container when true
  • The story properly responds to changes in the fullWidth prop through conditional rendering

Requires further human verification:

  • Visual verification in Storybook that the ButtonGroup actually takes 100% width when fullWidth={true} is set in the Props Table
  • Verification that the behavior works correctly when toggling between true and false values
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Wrapper Necessity

The fix wraps ButtonGroup in a div when fullWidth is true, but this may not be the ideal solution. Consider if the ButtonGroup component itself should handle the fullWidth styling internally rather than requiring a wrapper in the story. This approach might not reflect how the component would be used in production code.

render: args => {
  const content = <ButtonGroup {...args} />;
  return args.fullWidth ? <div style={{ width: "100%" }}>{content}</div> : content;
},

Copy link
Contributor

@rivka-ungar rivka-ungar left a comment

Choose a reason for hiding this comment

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

Hi @emmanuel-ferdman, thanks for your contribution.
Please notice that this change is considered a breaking change. The full width seems to work, see here, so it is the actual story of the overview that should be fixed.
Thanks

@emmanuel-ferdman
Copy link
Author

@rivka-ungar Thanks for the guidance! I've updated the PR to fix the Overview story instead of modifying the component.

…havior

Signed-off-by: Emmanuel Ferdman <emmanuelferdman@gmail.com>
Signed-off-by: Emmanuel Ferdman <emmanuelferdman@gmail.com>
@emmanuel-ferdman
Copy link
Author

@rivka-ungar so, how can we progress from here?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: ButtonGroup fullWidth prop has no affect in storybook

2 participants