Skip to content
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

Fix CLP top-bar assets not linked to asset_host when configured #2320

Merged
merged 2 commits into from Jul 22, 2016

Conversation

bladealslayer
Copy link
Member

This happens, because we use ActionController::Metal and the
setting of config.action_controller.asset_host doesn't take effect.

Alternatively, we could load a bunch of extra modules and hooks, but it
seems an overkill, because there are lots of hooked hooks, it seems:

include AbstractController::AssetPaths
include ActionController::ParamsWrapper
ActiveSupport.run_load_hooks(:action_controller, self)

This happens, because we use ActionController::Metal and the
setting of config.action_controller.asset_host doesn't take effect.

Alternatively, we could load a bunch of extra modules and hooks, but it
seems an overkill:

  include AbstractController::AssetPaths
  include ActionController::ParamsWrapper
  ActiveSupport.run_load_hooks(:action_controller, self)
<%= stylesheet_link_tag 'app-bundle' %>
<%= javascript_include_tag 'vendor-bundle' %>
<%= javascript_include_tag 'app-bundle' %>
<% asset_host = local_assigns.key?(:asset_host) ? asset_host : nil %>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if asset_host can not be found? Assets are served from rails?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the link to the asset will be /assets/something, so served from the same domain as the app.

Copy link
Contributor

@lyyder lyyder Jul 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bladealslayer bladealslayer merged commit 42b9525 into master Jul 22, 2016
@bladealslayer bladealslayer deleted the clp-assets branch July 22, 2016 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants