dash-daq icon indicating copy to clipboard operation
dash-daq copied to clipboard

Get rid of `BooleanSwitch`, just use `ToggleSwitch`?

Open alexcjohnson opened this issue 5 years ago • 2 comments

It was unclear to me what the difference is between BooleanSwitch and ToggleSwitch, and in fact I was a bit annoyed to see they have different names for the toggle prop - on vs value.

In fact, looking at their implementations, BooleanSwitch is actually just a ToggleSwitch with a hard-coded booleanSwitch={true} prop - and ToggleSwitch hides the fact that it even has a booleanSwitch prop, so it’s always falsy. So what's the difference?

As @shammamah pointed out to me on slack: the difference between the ToggleSwitch and the BooleanSwitch just appears to be that ToggleSwitch has two indicator lights to display which “side” is currently activated, whereas BooleanSwitch gets its background color changed when it's activated. Even knowing that difference it's not clear to me from their names which is which, and the difference is so minor that it seems better to just have one component type, with an extra prop to define this behavior. Rather than a boolean prop as we currently have behind the scenes, let's make it an enum, something like:

indicatorMode: PropTypes.oneOf([
    'background', // current BooleanSwitch behavior
    'both sides', // current ToggleSwitch behavior
    'on side' // new idea: put an indicator only on the on side
]);

alexcjohnson avatar Jan 28 '19 23:01 alexcjohnson

To me, the toggle switch should act like a switch between two states, not only True|False, but say 'log|linear'. So I'd also like to see that we can set two labels on each side of the toggle. This will make this widget much more useful.

Jerry-Ma avatar Dec 17 '19 16:12 Jerry-Ma

To me, the toggle switch should act like a switch between two states, not only True|False, but say 'log|linear'. So I'd also like to see that we can set two labels on each side of the toggle. This will make this widget much more useful.

I understand your point of view, but that prop name - on vs value is just pain to unify with input fields that have normal value prop.. At least this fact should be unified between those two..

pitris90 avatar Feb 13 '24 13:02 pitris90