studio
studio copied to clipboard
foxglove
Hide `x` buttons in settings color picker when control is already set to the default
Description https://github.com/foxglove/studio/pull/4060 introduced the ability to "clear" color fields, resetting them to the default.
Unfortunately, this leads to some instances where I have an x button that does nothing when clicked, because the field is already at the default.
I think we should hide the x button when a field is already set to its default value.
According to https://github.com/foxglove/studio/pull/4060#issuecomment-1213346122, "default" is not actually a concept, we just send undefined to the panel and let it figure out how it wants to handle things.
It's up to the panel implementor. They'll receive an incoming value of undefined but can process that however they want. In the case of that panel it's reverting it to the panel's own concept of the default. But at the level of the settings ui there's no "default" value.
Therefore, to fix this we may need to introduce a concept of "default" for controls like this. Opening up for discussion.
cc @foxymiles @2metres
I think rather than introducing a concept of a default value and leaving it up to settings to decide if the value is == default we can just add a enableClear property on the color input type and leave it up to the panel author to choose when to show the clear button.
Our general pattern in settings has been to make the actual controls as dumb as possible and give as much control to the panel author as possible.
On a separate note I think the x button should maybe be gray instead of purple. It's a very inviting button when it's purple, but probably not something we expect people would need too often.
Our general pattern in settings has been to make the actual controls as dumb as possible and give as much control to the panel author as possible.
This makes sense but I would think that it could be more confusing for users if different panels' color controls behave in different ways. Telling the control what the default value is and hiding the button in that case doesn't seem too "smart" to me?
Our general pattern in settings has been to make the actual controls as dumb as possible and give as much control to the panel author as possible.
This makes sense but I would think that it could be more confusing for users if different panels' color controls behave in different ways. Telling the control what the default value is and hiding the button in that case doesn't seem too "smart" to me?
There might be other cases where the panel author doesn't want to allow users to clear the color whether it's a "default" or not. Giving them control seems just as easy and more flexible.
If we leave it completely up to the panels, it means that we have to rely on every panel implementing the hide-if-default behavior correctly.
I think the "enableClear" option could be an additional thing we allow the panel to choose, but in addition to the automatic default-clear behavior.
+1 to jacob's comment about changing color from purple to white/gray.
On Fri, Aug 12, 2022 at 10:52 AM, Miles Egan < @.*** > wrote:
Our general pattern in settings has been to make the actual controls as dumb as possible and give as much control to the panel author as possible.
This makes sense but I would think that it could be more confusing for users if different panels' color controls behave in different ways. Telling the control what the default value is and hiding the button in that case doesn't seem too "smart" to me?
There might be other cases where the panel author doesn't want to allow users to clear the color whether it's a "default" or not. Giving them control seems just as easy and more flexible.
— Reply to this email directly, view it on GitHub ( https://github.com/foxglove/studio/issues/4122#issuecomment-1213367611 ) , or unsubscribe ( https://github.com/notifications/unsubscribe-auth/AAE3VZ6Z2VTR3LU3VW7NRZ3VY2FMPANCNFSM56MLPI5Q ). You are receiving this because you authored the thread. Message ID: <foxglove/studio/issues/4122/1213367611 @ github. com>
If we leave it completely up to the panels, it means that we have to rely on every panel implementing the hide-if-default behavior correctly.
Would that be any harder than implementing the "default" behavior correctly? Setting it to some magical default value seems harder to me especially when you're generating a lot of these on the fly as in the case of the new3d panel.
What do you mean by "magical default value"? Wouldn't the panel specify the default value when it creates the control?
It sounds like we're mostly all saying the same thing (that it would be more flexible if the default behavior were implemented by each panel) but disagreeing on whether that flexibility is desired or undesired.
If the set of settings is dynamically generated then determining what the default value is may not be so straightforward.
In any case if you can easily determine your default value then enabled = value === default seems pretty easy and also gives you the option of disabling the control for other reasons.
We don't have any concept of a default value for any other controls but we do have the concept of enabling or disabling particular features of inputs so this also seems more inline with the existing API.
This might be a good topic for product review. We can talk through the various competing goals here.