carbon icon indicating copy to clipboard operation
carbon copied to clipboard

feat: add fluid button set

Open lee-chase opened this issue 1 year ago β€’ 17 comments
trafficstars

The documentation on buttons https://carbondesignsystem.com/components/button/usage/ mentions fluid buttons and how they're handled. However, this does not appear to be implemented.

This PR adds the fluid property to the ButtonSet component.

With the fluid property set to true the button set exhibits the following behavior

  • Controls the inline-size of the buttons
  • Auto stacks the buttons based on $min-inline-button-size: convert.to-rem(176px)
  • Re-orders buttons based on guidance
  • Adds withDisplayBox to allow control of container width in storybook
  • Adds SetOfButonsFluid story

Further work

  • Setting a default button set in the story is not functional. NOTE: A default not functional elsewhere e.g. MenuButton Playground MenuAlignment.
  • Notify users (via event handler) of stacking change.
  • Allow for gaps between buttons
  • Control auto stack with a property.

Changelog

New

  • packages/react/.storybook/templates/WithDisplayBox/WithDisplayBox.scss
  • packages/react/.storybook/templates/WithDisplayBox/index.js
  • packages/react/src/components/Button/story/fluid-button-set-args.js

Changed

  • packages/react/src/components/Button/Button.stories.js
  • packages/react/src/components/ButtonSet/ButtonSet.tsx
  • packages/styles/scss/components/button/_button.scss

Testing / Reviewing

Checked functional in storybook

lee-chase avatar Feb 28 '24 17:02 lee-chase

Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name Link
Latest commit ba82d66564e74c1548c99165d567afa36514cc56
Latest deploy log https://app.netlify.com/sites/v11-carbon-react/deploys/65df6bfcd464160008281fad
Deploy Preview https://deploy-preview-15843--v11-carbon-react.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] avatar Feb 28 '24 17:02 netlify[bot]

Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name Link
Latest commit 61fea393475a1928004e5a406e01c058a529e1d8
Latest deploy log https://app.netlify.com/projects/v11-carbon-react/deploys/69205908761aa5000814c696
Deploy Preview https://deploy-preview-15843--v11-carbon-react.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

netlify[bot] avatar Feb 28 '24 17:02 netlify[bot]

This is great, @lee-chase. Thanks for taking the time to add this in. I had a question: Should the single button max out at 320px? It seems to grow until 520px and then becomes a non-fluid variant, but I'm just trying to determine the correct spec. Similarly, what should they grow to in a 2, 3, or 4-button group? The 4 button group has them all stack at a 700px width

tw15egan avatar Mar 11 '24 14:03 tw15egan

This is great, @lee-chase. Thanks for taking the time to add this in. I had a question: Should the single button max out at 320px? It seems to grow until 520px and then becomes a non-fluid variant, but I'm just trying to determine the correct spec. Similarly, what should they grow to in a 2, 3, or 4-button group? The 4 button group has them all stack at a 700px width

Not sure I have an answer to that. The sizes and behaviour chosen were very much a reflection of the ActionSet used in @carbon/ibm-products SidePanel. The values chosen in the case of SidePanel were judged to be aesthetically pleasing for the SidePanel rather than a fluid button set by itself.

Given that custom CSS properties cannot be used in media queries (container or otherwise) perhaps these value could be defined as SASS variables or part of a mixin call to allow a consumer could override should they need to.

lee-chase avatar Mar 14 '24 15:03 lee-chase

Hi, sorry for the late review. This looks great. I have a few questions and concerns:

  • This experience is different from the Set of Buttons. I was confused for 5 sec not knowing what to do. It could populate an option upfront.
  • From interacting with the "Stacked" prop, I'm not sure I see differences, unless I'm missing something.
  • @lee-chase Can the visual change here if I provide a spec? I was a little bit confused about how this worked since we don't have this experience anywhere in the storybook.

thyhmdo avatar May 01 '24 20:05 thyhmdo

@thyhmdo apologies for the lack of clarity.

  • I did try top populate this up front, but had some difficulty with Storybook, which seems to behave differently to IBM Products. Perhaps one of the devs could advise, happy to update. The simplest thing to do would be to move the various fluid button options into a folder.
  • Stacked - if I'm honest I do not recall why I left this in this story. I have removed it and can see no issues in doing so.
  • Not sure what you are asking with your final query.

The wrapper around the button story is something done in IBM Products and could probably do with a UX tidy up. The intent is to show the component but provide a simple mechanism to allow the user to see how the component behaves with different container sizes. image

lee-chase avatar May 02 '24 09:05 lee-chase

@thyhmdo apologies for the lack of clarity.

  • I did try top populate this up front, but had some difficulty with Storybook, which seems to behave differently to IBM Products. Perhaps one of the devs could advise, happy to update. The simplest thing to do would be to move the various fluid button options into a folder.
  • Stacked - if I'm honest I do not recall why I left this in this story. I have removed it and can see no issues in doing so.
  • Not sure what you are asking with your final query.

The wrapper around the button story is something done in IBM Products and could probably do with a UX tidy up. The intent is to show the component but provide a simple mechanism to allow the user to see how the component behaves with different container sizes. image

  1. @tw15egan @guidari Do you have suggestions for this?
  2. Nice! I checked and it seems like the problem is solved.
  3. Similar to components with layers, Anna provided some specs to have a particular look for it. In this visual, the label is quite long, the width extension could be replaced with something else that is close to our design spec (see image-red line)
  • The label could be simplified, maybe something like: "Max width (component container)
  • The other label could be "Width varies based on the max width adjustment" image

thyhmdo avatar May 02 '24 13:05 thyhmdo

Hey @lee-chase I did a commit on your PR to populate an option up front like @thyhmdo mentioned.

https://deploy-preview-15843--v11-carbon-react.netlify.app/?path=/story/components-button--set-of-buttons-fluid

guidari avatar May 02 '24 18:05 guidari

Hey @lee-chase I did a commit on your PR to populate an option up front like @thyhmdo mentioned.

https://deploy-preview-15843--v11-carbon-react.netlify.app/?path=/story/components-button--set-of-buttons-fluid

that looks good! thanks @guidari

thyhmdo avatar May 07 '24 18:05 thyhmdo

@andreancardona I do not believe this is waiting for response from me.

lee-chase avatar Jul 02 '24 13:07 lee-chase

Deploy Preview for carbon-elements ready!

Name Link
Latest commit 61fea393475a1928004e5a406e01c058a529e1d8
Latest deploy log https://app.netlify.com/projects/carbon-elements/deploys/69205908901dc9000893f5a4
Deploy Preview https://deploy-preview-15843--carbon-elements.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

netlify[bot] avatar Jul 30 '24 20:07 netlify[bot]

Hey, thanks again for your work here! Had a brief discussion with the team about any blockers we see preventing this from merging.

One thing that came up was the container slider. We've had users struggle to understand and use controls inside stories like this. We'd like to move away from them where possible. One idea is to instead have a new custom toolbar item in the storybook toolbar. It would contain a list of px values stepping in multiples of 160px. 160px, 320, 480, ... up to 1600. The graph bar can be used for the icon. Selecting one of these items would modify the styles on a decorator that wraps each story to set the width of that container to the selected value. Benefits here are that it standardizes controls within the storybook toolbar outside of the story frame, and it fully parameterizes these container sizes into the url. With this we could vrt components at one/many/all of these container widths.

On the code level I worry about determining if the buttons are currently stacked by imperatively evaluating the styles applied via getComputedStyle(). Is there another way we could determine this? It's not testable functionality as is because getComputedStyle() is a noop.

https://github.com/carbon-design-system/carbon/blob/184d1a9d72b6941c70d4804166371d85b3e3896f/config/jest-config-carbon/setup/setup.js#L33-L35

Sorting the buttons feels like something we would typically leave up to the consumer. I see the intended benefit, but using Children methods immediately limits the structure of what consumers can provide there. I'm not sure the benefits outweigh that cost?

tay1orjones avatar Aug 05 '24 19:08 tay1orjones

@tay1orjones & team

Switching from the container slider to a toolbar item is a possibility, but would need visual input as to how that is represented in the story. The viewport control has a similar behaviour and clearly marks out the usable area.

Whether or not the buttons are stacked is currently used to reposition box shadows and sorting. Even if sorting is deferred to the user they would need to monitor when stacking had occurred. In an ideal world we would provide onStack as an event. Could we not just mock window.getComputedStyle for some tests?

If there is no possibility of a tooltip (currently icon button only) then --stacked css modifier can be simplified and replaced with an overflow: hidden on the button set. This does not solve the sort/stacked detection.

lee-chase avatar Sep 04 '24 17:09 lee-chase

Deploy Preview for v11-carbon-web-components ready!

Name Link
Latest commit 61fea393475a1928004e5a406e01c058a529e1d8
Latest deploy log https://app.netlify.com/projects/v11-carbon-web-components/deploys/6920590805401500089a76da
Deploy Preview https://deploy-preview-15843--v11-carbon-web-components.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

netlify[bot] avatar Nov 22 '24 16:11 netlify[bot]

Codecov Report

:x: Patch coverage is 67.27273% with 18 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 92.55%. Comparing base (9c563ed) to head (61fea39). :warning: Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...mponents/Button/__story__/fluid-button-set-args.js 0.00% 17 Missing :warning:
...kages/react/src/components/ButtonSet/ButtonSet.tsx 97.36% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #15843      +/-   ##
==========================================
- Coverage   92.55%   92.55%   -0.01%     
==========================================
  Files         515      516       +1     
  Lines       37660    38273     +613     
  Branches     5820     5818       -2     
==========================================
+ Hits        34858    35423     +565     
- Misses       2652     2701      +49     
+ Partials      150      149       -1     
Flag Coverage Ξ”
main-packages 85.51% <67.27%> (-0.08%) :arrow_down:
web-components 96.87% <ΓΈ> (-0.02%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Nov 22 '24 16:11 codecov[bot]

@guidari any further work you would like on this?

lee-chase avatar Apr 22 '25 15:04 lee-chase

Going to work on a feature and would love to have this in for use πŸ™‚

AlexanderMelox avatar Jun 11 '25 15:06 AlexanderMelox

@tay1orjones got nudged regarding this a couple of days back by with a desire to use it in an AI Chat workflow.

I think I addressed the points, only outstanding is the magic number 176px, with mine being the 6th time this number has appeared in Carbon styles. It probably should be configurable, or at least adjustable for the IBM Products Narrow Tearsheet with 4 buttons. This could be added in if Tearsheet decides to adopt.

lee-chase avatar Nov 20 '25 14:11 lee-chase