RFC: Custom CSS classes #36
Replies: 9 comments 28 replies
-
I really like the ability to call the combobox_tag with a For the ability to specify the css classes in an initializer I am not 100% convinced. CSS is a front end job, A front end developper should be able to customise this part. Setting the css classes in the initializer is not something that would be natural for a FE dev and when developping any changes to the configuration will need a sever restart to be visible as initializers are only loaded once at boot. It is probably not something that would need to be changed often but during the development part I can easily assume that several tweeks will be required to achieve a pixel perfect results. I would like to suggest a slightly different approach Variant definitionIn the initializer we define the the variants, here the # config/initialiers/hotwire_combobox.rb
HotwireCombobox.setup do |config|
config.variants [:filter]
end Generated htmlThe main fieldset would be something like that with the added class <fieldset class="hw-combobox hw-combobox-filter" data-async-id="state-box" data-controller="hw-combobox" ...> Variant stylingThen we can style using standard css .hw-combobox.hw-combobox-filter {
/* custom classes */
} Searchable classesWhen inspecting the html if we see this GeneratorTo perfect the DX maybe a generator could create a skeleton for the CSS variants. Or this could be written in comment in the initializer file |
Beta Was this translation helpful? Give feedback.
-
The initializer config is the way to go even though making changes requires a restart since this will be a rare style-guide / UX pattern change. This is the Rails way to configure dependencies and it's always possible that a future version of Rails will auto-reload initializers in development (we've seen similar improvements over the years). My one suggestion is to nest these configs to allow for future variant definitions that go beyond styling. Something like: HotwireCombobox.setup do |config|
config.add_variant :filter,
css_classes: {
input: "custom-class another-custom-class",
... I'm working on a couple feature proposals here that could each benefit from a tiny bit of non-CSS customization. |
Beta Was this translation helpful? Give feedback.
-
Thanks @adrienpoly @jlw
That's interesting. I'm sure this is the experience for many FE devs but it's not something I would've guessed. Initializers are a basic Rails feature so if it's not something some devs are used to manipulating I think it'd be good to introduce them to it through this gem.
I hear ya. This would be annoying to deal with. But I think that's on the framework to sort out. I saw Brad's post on X about putting config in controllers instead of initializers. Briefly considered that for this. But I just don't think this is a controller concern — and I don't want this to be the sole gem doing things out of the ordinary in people's projects. Again, would rather not fight the framework on this and wait for Rails to improve the developer experience re: initializers.
This is very similar to what I had originally conceived for the gem. I wrote about it in #22 (comment), which I should've linked for context. The thing that made me change my mind is @jlw's team's use case where they want to use DaisyUI to style the component. With this approach, they'd have to break down DaisyUI classes into tailwind primitives and apply them with @jlw,
Could you share some more about those proposals? I think the word It is weird that there's nothing in the API that lets the unfamiliar reader know they're dealing with CSS classes though. Could maybe do HotwireCombobox.setup do |config|
config.add_css_variant :filter,
input: "..."
# etc
config.add_css_variant :search,
input: "..."
# etc
end |
Beta Was this translation helpful? Give feedback.
-
@jlw could you expand on what wasn't working for you. Here is a test I did to use |
Beta Was this translation helpful? Give feedback.
-
I hope I'm not too late to chime in. I do see the simplicity of having the classes in the initializer, but the truth is (IMHO) that simplicity is not always best and I'll try to paint a picture. I'll preface with the fact that I am a big fan of TailwindCSS and as a big fan I rarely use Another preface, as the maintainer of Avo, as I saw this scenario, is that some folks might not be able to use Another edge case would be when you have a variant configured, but there's this one unique place where you want one of combobox's elements to look/behave different. Becuase of these reasons, I would like to propose a different API that can compliment the variants one. Inline classes. <%= combobox_tag "filter", @filter_options, variants: [:outline], classes: {
input: "",
handle: "",
}
%> You, behind the scenes can merge the variants and classes hashes and apply them to the current element. I can't tell you how many times people thanked me for offering this kind of API where you can configure something ahead of time and enable inline configuration as well. |
Beta Was this translation helpful? Give feedback.
-
Per #36 (reply in thread) I'm having thoughts about the current proposal being too flexible. Here's another idea I'm exploring: <%= combobox_tag :foo, @options do |combobox| %>
<% combobox.input_attrs class: "", foobar: "", data: { bar: "" }, aria: { baz: "" } %>
<% combobox.listbox_attrs class: "", ... %>
<% combobox.option_attrs class: "", ... %>
<% ... %>
<% end %> All attrs would be merged with the library defaults, with the defaults taking preference. So you wouldn't be able to overwrite anything important. I've never built such an interface and I don't think I've seen things like it in the wild. Although it's reminiscent of the ViewComponent slots stuff. But I really like that styling can be done in the view (no need for initializers or server restarts) and that you're manipulating each element much more directly — no passing classes at the top level expecting them to be magically propagated. It's also cleaner IMO than passing everything inline as each element is clearly delimited. Finally, you're able to add any atrrs you like to any element in the combobox. Thoughts are welcome! |
Beta Was this translation helpful? Give feedback.
-
PR here: #80 Feedback is appreciated. Gonna let it sit for a bit. I'm unsure of the amount of flexibility this introduces. Could be the source of many footguns. Thoughts? |
Beta Was this translation helpful? Give feedback.
-
Reading this thread and the documentation, I cannot find the best way to make this compatible with daisyui. I just got started using the gem, but it looks like horseshoes in my daisyui setup. 😂 Should I create variants in the configuration and apply classes to it? |
Beta Was this translation helpful? Give feedback.
-
I've been thinking about which API would enable us to have custom CSS classes for each element while avoiding passing tons of arguments to each instance of
combobox_tag
. This is a subjective preference of mine. I think having too many args often makes code hard to follow.I came up with an implementation where you're able to define variants in an initializer and then use them by passing a single param at the call site. Looks like this:
Used like this in a view template:
We could define the current default classes as a default variant. That means any classes defined in the variant would be used instead of the default classes, not in addition to. Users can include the default classes as part of their custom variants if they wish.
I'm unsure whether we should require every element when defining a variant. Seems fine to leave any omitted elements unstyled as long as we have thorough documentation for which elements we support.
Would love to hear what others think. Particularly @jlw. Also open to other proposals.
Beta Was this translation helpful? Give feedback.
All reactions