chanzuckerberg / sci-components

2021 Science Design System Component Library
https://chanzuckerberg.github.io/sci-components/
MIT License
25 stars 7 forks source link

Allow disabling buttons in SegmentedControl #868

Closed TeunHuijben closed 1 week ago

TeunHuijben commented 1 month ago

Component Name

SegmentedControl

Issue description

I am working with the SegmentedControl ToggleButtonGroup (sci-components/packages/components/src/core/SegmentedControl). Would it be possible to allow control over disabling certain buttons?

One solution could be to change in `SegmentedControl/index.tsx':

PS. I could also do this myself by forking the repo and making a PR with this change, if that is desired :)

masoudmanson commented 1 month ago

Hi @TeunHuijben,

Thanks so much for reaching out! 🥳🎉

We’ve updated the SegmentedControl component to include the disabled prop in the button definitions, just like you suggested (link to the PR). Now you can disable specific buttons as needed. We also added support for controlled and uncontrolled states, so if you pass a value prop and an onChange function to the SegmentedControl, you can control the component’s state yourself.

Here is an example of controlled SegmentedControl with a disabled button (Demo):

import { useState } from "react";
import SegmentedControl from "@czi-sds/components";

export const ControlledSegmentedControl = (props: Args): JSX.Element => {
  const buttonDefinition = [
    {icon:"List",tooltipText:"List A",value:"A"},
    {disabled: true, icon:"List",tooltipText:"List B",value:"B"},
    {icon:"List",tooltipText:"List C",value:"C"},
    {icon:"List",tooltipText:"List D",value:"D"}
  ];

  const [value, setValue] = useState("C");
  return (
    <RawSegmentedControl
      buttonDefinition={buttonDefinition}
      onChange={(_event, newValue) => {
        console.log(newValue);
        setValue(newValue);
      }}
      value={value}
      {...rest}
    />
  );
};

Let me know if there’s anything else we can do to make the component fit your needs.

Cheers,

masoudmanson commented 1 month ago

10/09 Release

:computer: Coded Library (@czi-sds/components@21.4.0)

:gift: New Features:

TeunHuijben commented 1 month ago

Hi @masoudmanson,

Thanks a lot for implementing this! I will check it out and let you know if I have further questions

TeunHuijben commented 2 weeks ago

Hi @masoudmanson,

I tested disabling the button, and it works perfectly! :raised_hands:

Maybe one more "feature request"/question. In my case, I have a group with three buttons:

const buttonDefinition: SingleButtonDefinition[] = [
      { icon: "Cube", tooltipText: "Box", value: PointSelectionMode.BOX },
      { icon: "Starburst", tooltipText: "Sphere", value: PointSelectionMode.SPHERICAL_CURSOR },
      { icon: "Globe", tooltipText: "Adjustable sphere", value: PointSelectionMode.SPHERE },
];

<SegmentedControl
    id="selection-mode-control"
    buttonDefinition={buttonDefinition}
    onChange={(_e, v) => {
        props.setSelectionMode(v);
    }}
    value={props.selectionMode}
/>

Previously, it was possible to select none of the options. For example, when the first button is selected, and one clicks again on the first button, all buttons are deselected, and value=null. Now, I have updated czi-sds/components to the a newer version to include the option of disabling the buttons (I went from 20.0.1 to 21.4.0), and deselecting all buttons is no longer possible.

I this a result of adding the 'disable' functionality to the buttons, or because of other changes to components in between these two version?

Thanks again for the help! (let me know if it is easier if I make a new issue on this topic, or continue here)

masoudmanson commented 2 weeks ago

Hey @TeunHuijben ,

Thanks for catching this bug! I initially set a rule to always have an item selected, so it prevents deselecting all buttons and defaults to the first one. I’ll check in with @clarsen-czi to ensure this approach aligns with the design and Connor’s intended use cases. I’ll keep you updated once it’s fixed and released. 😊

Apologies for the inconvenience! 🫠

TeunHuijben commented 1 week ago

No problem, thanks a lot for the help and quick action! 😊

clarsen-czi commented 1 week ago

Thanks for bringing up this use case, @TeunHuijben. It's not something we've run into before but definitely makes sense. @masoudmanson I say let's go ahead and implement it. Thanks for checking!

masoudmanson commented 1 week ago

Hey @TeunHuijben,

The PR is live! https://github.com/chanzuckerberg/sci-components/pull/891. You can also check it out here: Storybook I’ll make sure to release it with the new package tomorrow. 🥳🎉😬

TeunHuijben commented 1 week ago

Thanks @masoudmanson!

masoudmanson commented 1 week ago

11/08 Release

:computer: Coded Library (@czi-sds/components@21.6.3)

:bug: Bug Fixes: