Updated javascript_initialization to take in options to define custom action name

[yzdong]

Hey Winston! Modified javascript_initialization so that we can pass it an optional value to specify which action name it should use.

The use case is that one controller action could be rendering two different views, but since they are called from the same controller action, javascript_intialization will generate the same javascript.

With options, you can provide a action_name in a view template and still use javascript_initalization at the application layout level to generate two different sets of javascript.

Possible modifications would be to allow it to take in a custom controller_class as well.

Let us know what you think!

[winston]

Hi @yzdong! Thanks for the PR. May I understand how you would use this?

Typically, javascript_initialization is included in the footer. How would you pass in the options then?

Do you call javascript_initialization instead on every page, or something different?

Am also thinking then if it makes sense to include another level after page_action_name which will do an init for the main view name. Hmm..

[yzdong]

I'm calling javascript_initialization custom_name: @template_name, where @template_name is provided in the custom view template as:
@template_name = capture do 'custom_view' end

Mm yah, that could be an option too. Not quite sure how you would get the view name thought; is that available in the controller object?

[winston]

Not quite sure how you would get the view name thought; is that available in the controller object?

Just checked. Not really. Have to do more work to get it out.

I'm calling javascript_initialization custom_name: @template_name

Are you calling this in the layouts/application.html.erb? Potentially @template_name can be nil in views where you are not assigning @template_name right?

Might want to use content_for :template_name, 'custom_view' instead of capture do end..
But IMO, it's still not a very clean API. Hmm.. Lends to confusion and complexity.

Am stll wondering if you really require that granularity of breaking out JS by views.
Does your JS break if they just belong to action_name?

[yzdong]

Mm yah, I am calling javascript_initialization in layouts/application.html.erb. Guarding against an empty @template_name does make sense, but if there is no @template_name then it defaults back to the default controller action.

Heh yeah unfortunately with our current setup the JS does break if it's just action_name...

I guess the tradeoff is in flexibility v. complexity... The default setup works with a standard Railsy CRUD setup but not quite if are different paths from one controller aciton.

[winston]

This is what I think would be better in terms of "API":

In the helper javascript_initialization, I would add:

if content_for?(js_init_method)
  if(#{application_name}.#{page_controller_class}.init_#{js_init_method}) { #{application_name}.#{page_controller_class}.init_#{js_init_method}(); } 
end 

after the page_action_class call:

if(#{application_name}.#{page_controller_class}.init_#{page_action_class}) { #{application_name}.#{page_controller_class}.init_#{page_action_class}(); } 

Then to use it, all you have to do in your view is to add:

content_for :js_init_method { 'custom_view' } 

Does this make sense? wdyt?

[yzdong]

Oh, so javascript_initialization will grab the method name through js_init_method. Shouldn't it be a if-else though, since we only want either js_init_method or page_action_class, and not both?

Other than that, it makes sense to name the parameter through rails_utils rather than exposing it as an anonymous argument.

[winston]

and not both

I would actually keep both. So the standard is:

1. run init controller
2. run init controller#action
3. run custom (only if you have set js_init_method in your view).

[yzdong]

I see, yah I guess it makes sense in reducing code duplication if you share code that's common to init controller#action.

I see you've worked on refactoring some stuff-- I'll pull the newest master, write some tests up for this branch, and update the pull request.

[yzdong]

Added the changes, let me know your thoughts!

[JuanitoFatas]

Something went wrong with the indentation, could you check please, thanks!

[yzdong]

Hi Juanito,

I've fixed the indentation issues and changed the if statement, let me know if it works!

[JuanitoFatas]

@yzdong Thanks for the pull request but this still need some minor changes.

I merged this and please see c6d7b2d for what has changed. 😊