search

maxmarcon / live_select

Dynamic (multi)selection field for LiveView
https://hex.pm/packages/live_select
Apache License 2.0
185 stars 35 forks source link

use different styling options for clear_button and clear_tag_button #75

Closed Munksgaard closed 4 months ago

Munksgaard commented 4 months ago

According to the documentation, clear_tag_button uses the clear_button_class and clear_button_extra_class to override and extend the styling. However, The default classes for clear_button and clear_tag_button are different, meaning that if I attempt to override the class of one, the other will inadvertently also be overwritten. Here are what the docs currently look like:

Element Default daisyui classes Default tailwind classes Class override option Class extend option
clear_button cursor-pointer hidden cursor-pointer hidden clear_button_class clear_button_extra_class
clear_tag_button cursor-pointer cursor-pointer clear_button_class clear_button_extra_class

I would expect clear_tag_button to use clear_tag_button_class and clear_tag_button_extra_class instead, but I get an error if I try to use those.

Munksgaard commented 4 months ago

It seems like this choice was made on purpose, but I'm not sure I understand why: https://github.com/maxmarcon/live_select/commit/a05401732c2b3caa6877af98ff9b888edf4a2e1f

Should it not be possible to override the styling for clear_button and clear_tag_button separately?

maxmarcon commented 4 months ago

That's because the button to clear the tag is only used in tags mode (multiple selection), whereas the button to clear the selection is only used in single mode (single selection).

So there's no chance of them colliding if your LiveSeelct doesn't dynamically switch between tags to single mode.

Munksgaard commented 4 months ago

Aha, I see. The reason why I am trying to override both buttons at the same time, is because I've created a component as suggested here and wanted to apply all my custom branded styling in one place.

If I understand you correctly, if I want the clear_button (for single selection) and the clear_tag_button (for multiple selection) styling to be different, I'll have to create two separate components?

maxmarcon commented 4 months ago

Yes, at the moment this is what you should do. A component for multiple selection and one for single selection.

I can see however how this choice of reusing the same name for different styling options is a bit lazy and can lead to confusion. We should change this.

I'll change the name of this issue accordingly, thanks

maxmarcon commented 4 months ago

Feel free to write PR for this if you feel like it @Munksgaard !