Skip to content

Generate styles for the sessions#new view template #382

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

Closed
yshmarov opened this issue Jul 20, 2024 · 8 comments · Fixed by #384
Closed

Generate styles for the sessions#new view template #382

yshmarov opened this issue Jul 20, 2024 · 8 comments · Fixed by #384

Comments

@yshmarov
Copy link
Contributor

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

@flavorjones
Copy link
Member

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

@yshmarov
Copy link
Contributor Author

yshmarov commented Jul 21, 2024

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
Copy link
Member

@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
Copy link
Member

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
Copy link
Member

dhh commented Jul 28, 2024

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
Copy link
Member

@dhh The necessary changes are pretty small, PR is at rails/rails#52442

@flavorjones
Copy link
Member

Draft pr is at #384

@flavorjones
Copy link
Member

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

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

Successfully merging a pull request may close this issue.

3 participants