-
Notifications
You must be signed in to change notification settings - Fork 189
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
Comments
@yshmarov Great idea, are you interested in submitting a PR? |
@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! |
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 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: I think this requires breaking up the sessions generator into multiple generators, similar to how the scaffold generation is broken up into 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. |
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. |
@dhh The necessary changes are pretty small, PR is at rails/rails#52442 |
Draft pr is at #384 |
Just a note that the upstream Rails PR was merged, so I'm moving forward with #384. |
The new sessions generator has an html template that needs to be styled
The text was updated successfully, but these errors were encountered: