rails / tailwindcss-rails

MIT License
1.4k stars 173 forks source link

Generate styles for the sessions#new view template #382

Closed yshmarov closed 2 months ago

yshmarov commented 2 months ago

The new sessions generator has an html template that needs to be styled

flavorjones commented 2 months ago

@yshmarov Great idea, are you interested in submitting a PR?

yshmarov commented 2 months ago

The generator could looks like this:

# app/views/sessions/new.html.erb
<div class="mx-auto md:w-2/3 w-full">
  <h1 class="font-bold text-4xl">Sign in</h1>

  <% if alert.present? %>
    <p class="py-2 px-3 bg-red-50 mb-5 text-red-500 font-medium rounded-lg inline-block" id="alert"><%= alert %></p>
  <% end %>

  <%= form_with url: session_url, class: "contents" do |form| %>
    <div class="my-5">
      <%= form.email_field :email_address, required: true, autofocus: true, autocomplete: "username", placeholder: "Enter your email address", value: params[:email_address], class: "block shadow rounded-md border border-gray-400 outline-none px-3 py-2 mt-2 w-full" %>
    </div>

    <div class="my-5">
      <%= form.password_field :password, required: true, autocomplete: "current-password", placeholder: "Enter your password", maxlength: 72, class: "block shadow rounded-md border border-gray-400 outline-none px-3 py-2 mt-2 w-full" %>
    </div>

    <div class="inline">
      <%= form.submit "Sign in", class: "rounded-lg py-3 px-5 bg-blue-600 text-white inline-block font-medium cursor-pointer" %>
    </div>
  <% end %>
</div>

Result:

Screenshot 2024-07-21 at 10 53 24

For reference, posts#index with the scaffold generator looks like this:

Screenshot 2024-07-21 at 10 52 10

I'm not quite sure how it would look in a PR, so that it overrides the default file...

flavorjones commented 2 months ago

@yshmarov Thanks for submitting the styling. I can take care of the rest, and will make sure to credit you as co-author. It may be a few days, I'm on holiday at the moment. Thank you!

flavorjones commented 2 months ago

OK, I dug into this today. A few things need to happen in order for this gem to be able to override the new session view template ...

The naive change to the upstream generator to parameterize the template engine might look like:

diff --git a/railties/lib/rails/generators/rails/sessions/sessions_generator.rb b/railties/lib/rails/generators/rails/sessions/sessions_generator.rb
index 21ed4204..29520a06 100644
--- a/railties/lib/rails/generators/rails/sessions/sessions_generator.rb
+++ b/railties/lib/rails/generators/rails/sessions/sessions_generator.rb
@@ -3,6 +3,10 @@
 module Rails
   module Generators
     class SessionsGenerator < Base # :nodoc:
+      hook_for :template_engine, as: :sessions do |template_engine|
+        invoke template_engine
+      end
+
       def create_session_files
         template "models/session.rb", File.join("app/models/session.rb")
         template "models/user.rb", File.join("app/models/user.rb")

This would allow this gem to implement Tailwindcss::Generators::SessionsGenerator in lib/generators/tailwindcss/sessions/sessions_generator.rb.

However, when we do this, the generator will look locally for all of the model and controller templates as well, which we should not be copying into this gem just to override the view:

https://github.com/rails/rails/blob/c01ee683aa700b109f704a5b6611107fe2dc5476/railties/lib/rails/generators/rails/sessions/sessions_generator.rb#L7-L12

I think this requires breaking up the sessions generator into multiple generators, similar to how the scaffold generation is broken up into Rails::Generators::ScaffoldGenerator and Erb::Generators::ScaffoldGenerator, which is a conversation we should have upstream.

Given the sessions generator is intended to be bare-bones, it's not clear to me that it's intended to be overridden by gems like this; and if it is, we may need to wait for it to firm up a bit before making a change like I'm describing. It's possible that @dhh has some opinions that might guide next steps.

dhh commented 2 months ago

Yeah, I'm a bit of two minds about this. The generator as it exists in the rails repo is intended to be unstyled. But I am actually starting to think that some degree of styling for these built-in generators could be nice. So maybe adding the hooks like proposed above would be fine.

Could also just let it sit and cook for a little longer. And see if we can't come up with an approach to default styling that feels proportionate.

flavorjones commented 2 months ago

@dhh The necessary changes are pretty small, PR is at https://github.com/rails/rails/pull/52442

flavorjones commented 2 months ago

Draft pr is at #384

flavorjones commented 2 months ago

Just a note that the upstream Rails PR was merged, so I'm moving forward with #384.