Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal for customizable classes merging #1831

Open
borama opened this issue Feb 23, 2024 · 1 comment
Open

Proposal for customizable classes merging #1831

borama opened this issue Feb 23, 2024 · 1 comment

Comments

@borama
Copy link

borama commented Feb 23, 2024

Hello! I would like to propose an optional change in the way Simple Form merges the CSS classes defined in its configuration file with those passed while rendering form inputs. I came to this proposal after thinking for a long time how to support Tailwind in Simple Form in a clean way. I also read the other relevant issues here, mainly #1723, which seems inconclusive though. I even wrote a post about the problem back in the day but now I tend to consider the solution given in it mediocre, at best, because it circumvents the way Simple Form works.

The common way to use Simple Form, as I understand it, is to define the "base" classes for various inputs, their wrappers and helper elements in the configuration file. When we need to modify a specific form's input style, we can pass additional classes via input_html etc. parameters of the form.input method. Simple Form then merges the two sources into the final class HTML attribute, effectively concatenating all the classes together.

While this works well for traditional hand-written CSS, it often falls short for Tailwind or other utility class systems. The specificity of the utility classes is always the same so the CSS cascading order is determined only by the order in which the classes are specified in the CSS file generated by Tailwind, i.e. outside of developer's control.

An example of the problem

Let me give a small example: in the configuration file we set the labels to use the medium font (using the Tailwind's font-medium utility class):

# config/initializers/simple_form.rb
config.wrappers :default do |b|
  ...
  b.use :label, class: "font-medium"
  ...
end

but suppose we want a specific field's label to be bold instead:

<%= simple_form_for @user do |f| %>
  <%= f.input :username, label_html: { class: 'font-bold' } %>
<% end %>

Simple Form concatenates the two classes (with others) together, producing the following HTML for the label:

<label class="font-medium string optional font-bold" for="user_username">Username</label>

While the font-bold class is present in the markup, the font-medium still wins the CSS cascade because it is specified later in the generated Tailwind CSS file and thus the label is not rendered bold.

Possible workarounds

One way to deal with this is using the !important modifier to raise the cascading order of the overriding class:

<%= simple_form_for @user do |f| %>
  <%= f.input :username, label_html: { class: '!font-bold' } %>
<% end %>

The exclamation mark in the class name makes Tailwind generate the class definition with the !important flag set. This works well but has the downside of further cluttering the output CSS file with !important variants of various classes.

Another workaround, that I proposed in my blog post, keeps the generated CSS cleaner but at the expense of using the configuration file only for the tags structure, styling them by default in a custom form builder and manually removing the default classes that would need to be made !important to override them. Not nice either…

The proposed solution

I believe that the ideal solution would be if Simple Form was able to render only the overriding class in the cases when both the default and overriding classes target the same CSS property (e.g. the font-weight in the example above). Of course, I am not the first to think about it that way and there is the Tailwind Merge gem that does precisely this: it merges arbitrary Tailwind classes together, resolving any conflicts along the way (it itself is a port of a JS library). As a developer, I would not have to think about !importants or default classes removal, it would all just work transparently. BTW, we use Tailwind Merge in our project in production for over a year now and it works really well for us.

I am proposing to modify Simple Form in a way that would allow to optionally customize classes merging. Currently, it seems to take place on this line in the merge_wrapper_options method. Perhaps we could add a new configuration option such as config.custom_class_merger that would allow to specify a ruby class that would take care of classes merging. It would default to an internal Simple Form ruby class that would behave in the same way as today, i.e. it would concatenate the classes. But, a Tailwind user might add a custom class that would use the Tailwind Merge gem instead. Thinking about this, this would even open a possibility to optionally reconfigure Simple Form in a way that it would always replace the default classes with the overriding ones, which is a behavior that some of the users occasionally wished for (#1307).

So, what do you think? I believe this would help using Simple Form with Tailwind in a much cleaner way. I can try to make a PR if you'd be OK with this and am eager to hear about your thoughts in general. Thanks!

@pas256
Copy link

pas256 commented Jun 17, 2024

I'd love to see this happen! Do you need any help?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants