This project is archived and is in readonly mode.
Allow to register javascript/stylesheet_expansions to existing symbols
Reported by Sven Fuchs | February 15th, 2009 @ 12:12 AM | in 3.x
register_javascript_expansion and register_stylesheet_expansion currently just merge the given expansions to the respective class variables, thus overwriting previously registered symbols.
The "register" naming of these methods kinda suggests that this is possible IMO. Also there's no other accessor (anymore) that would allow subsequent registrations from multiple plugins.
The attached patch makes it so that you can register multiple javascripts/stylesheets in subsequent calls and they get merged:
def test_custom_javascript_expansions_merge_with_existing_symbols
["head", "body", "tail"].each do |script|
ActionView::Helpers::AssetTagHelper::register_javascript_expansion :monkey => [script]
end
assert_dom_equal %(<script src="/javascripts/first.js" type="text/javascript"></script>\n<script src="/javascripts/head.js" type="text/javascript"></script>\n<script src="/javascripts/body.js" type="text/javascript"></script>\n<script src="/javascripts/tail.js" type="text/javascript"></script>\n<script src="/javascripts/last.js" type="text/javascript"></script>), javascript_include_tag('first', :monkey, 'last')
end
Comments and changes to this ticket
-
DHH February 22nd, 2009 @ 03:09 PM
- Milestone set to “2.x”
-
CancelProfileIsBroken August 5th, 2009 @ 01:49 PM
- Tag changed from “2.3, actionview, expansions, javascript, patch, stylesheets” to “2.3, actionview, bugmash, expansions, javascript, patch, stylesheets”
-
Dan Pickett August 8th, 2009 @ 10:31 PM
verified that this applies cleanly to 2-3-stable
-1 on this, though imo - this could get really confusing and unwieldy if you keep appending to the same expansion - the constraint of the one expansion per call keeps things clean
-
-
Simon Jefford August 9th, 2009 @ 01:02 AM
Verified but -1 on this. I'm struggling to think of a use for this.
-
Jeremy Kemper August 9th, 2009 @ 01:12 AM
- State changed from “new” to “open”
Use case: plugin wants to add a js to head.
This is technically a backward-incompatible change, though, so not appropriate for 2-3-stable.
-
Elad Meidar August 9th, 2009 @ 01:22 AM
-1 We would you want subsequent calls to register assets ? i assume as a developer, that i would like to see only one place where each plugin registered assets expansions keys.
also, patch does not apply on master.
-
-
Dmitry Ratnikov August 9th, 2009 @ 04:09 AM
Jeremy, do you happen to have an example why would plugins need to share keys? Plugin authors should be able to make their asset namespace unique by using the plugin name (e.g. :acts_as_monkey => [ 'tail', 'body', 'head' ]).
The patch does not apply cleanly to master either.
-
Jeremy Kemper August 9th, 2009 @ 08:58 PM
- Tag changed from “2.3, actionview, bugmash, expansions, javascript, patch, stylesheets” to “2.3, actionview, expansions, javascript, patch, stylesheets”
- State changed from “open” to “wontfix”
I don't have an example. Hence wontfix :)
-
Sven Fuchs August 10th, 2009 @ 08:22 AM
- Assigned user set to “Jeremy Kemper”
- State changed from “wontfix” to “open”
Tss .. :)
Examples are probably coming from the Engines world. Think of two inhouse engines that are supposed to hook into a common admin backend. The admin backend layout is somewhere else (maybe in another engine) and contains the javascript/stylesheet assets helper calls. Maybe the two engines are a blog and a forum engine and they both ship js/css assets that are relevant for the admin backend.
When you install the forum engine it should be able to transparently register its js/css assets. You should not need to change the admin layout for that. The obvious solution to this is to just add onto an existing expansion without clearing it.
Can this be solved more unobtrusively with an additional option? like :really_just_register_without_killing_other_peoples_stuff => true? or just :append => true
Jeremy, am I supposed to reopen the ticket now? I'll just do that :)
-
-
Sven Fuchs October 19th, 2010 @ 12:16 PM
- Importance changed from “” to “”
bump
We still need this :) And it still makes so much sense. Even more with the advert of super-powerful engines in rails 3.
-
Sven Fuchs November 21st, 2010 @ 01:43 PM
bump :)
I've pushed a patch to 3-0-stable here: https://github.com/svenfuchs/rails/commit/b712476ac3f20d1e976ea431c... (I currently can't seem to upload/attach patches to lighthouse tickets, but I guess a fork commit will do as well?)
Folks, the usecase for this is rather straight forward and obvious for people who use engines to split up applications.
Maybe think of a bunch of engines that provide a blog, tickets, newsletter, ... whatever. Now they'll all contribute routes, controllers, views, assets to a common administration backend and the shared admin layout itself is provided by an engine.
Engines should now be able to register their assets (e.g. admin/blog.css, admin/tickets.css, ...) to a shared asset expansion which then can be used by the admin layout. Depending on what engines are installed the layout will include the relevant assets.
Believe me, it's the right way to do this :)
Obviously one also could monkey-add another helper method, but "registering" already means, well, registering and not overwriting. So this custom helper method maybe would be :really_register_stylesheet_expansion?
This change won't break anything for people who use asset expansions in a single place in their application. But it allows others to add on top of them.
-
Deleted User November 22nd, 2010 @ 01:29 PM
- Tag changed from “2.3, actionview, expansions, javascript, patch, stylesheets” to “2.3, 3.x, actionview, expansions, javascript, patch, stylesheets”
+1
:engine_admin would be the perfect expansion example for engines which solve different, distinct problems, but all have a common admin-interface.
I was really surprised that this didn't work.
-
Clemens Kofler November 25th, 2010 @ 08:09 AM
Wouldn't this make sense in a style of before/after filters, view paths, middleware etc. in the sense that you can prepend, append and insert before/after some other JS? My thinking is that in JavaScript most of the time load order is important (e.g. jQuery before anything that uses jQuery eagerly).
-
Sven Fuchs November 25th, 2010 @ 08:48 AM
Hey Clemens, well, so far people didn't even see any usecase here :)
On the one hand one could argue that engines shouldn't need to know that much about each other. So they could register their own dependencies (like jquery).
On the other hand engines already register initializers in the same way you mention.
This stuff works fine for us right now (it's actually been extracted from adva-cms2), so I'd wait for people coming up with the need for a more powerful mechanism. Btw, these could easily be extra helper methods (like :prepend_registration or something ...)
-
Josh Kalderimis February 13th, 2011 @ 11:49 AM
- State changed from “open” to “resolved”
Hi Sven,
Merging of expansion is in both 3-0-stable and master, so I am closing this issue as resolved.
Thanks,
Josh
-
Sven Fuchs February 13th, 2011 @ 12:52 PM
Hey Josh,
yeah, thanks!
Apparently Aaron has applied the patch meanwhile https://github.com/rails/rails/blob/55b13c532f8c1dbbe34695f356c62fe...
Sorry for not paying more attention here :)
-
Josh Kalderimis February 13th, 2011 @ 01:13 PM
Hey Sven,
No need to apologize, and its my pleasure, thanks for pushing and pushing for this change.
Have a great weekend.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
CancelProfileIsBroken
DHH
Jeremy Kemper
Josh Kalderimis
Piotr Sarnacki
Sven Fuchs