Asset pipeline support - #1
Open
saharvey wants to merge 8 commits into
Open
Conversation
abrom
reviewed
Jul 15, 2026
|
|
||
| private | ||
|
|
||
| def middleware_path(file, version) |
There was a problem hiding this comment.
would suggest adding a lookbook_ prefix given this will still bleed into the view helper method space (or, despite the ABC increase, simply inline it. Not perfect.. but less pollution to the views
| # Hook into assets:precompile so this runs automatically during deployment | ||
| if Rake::Task.task_defined?('assets:precompile') | ||
| Rake::Task['assets:precompile'].enhance do | ||
| Rake::Task['lookbook:copy_assets'].invoke |
There was a problem hiding this comment.
should this also be wrapped in the same guard as above. ie
if Rails.application.config.action_controller.respond_to?(:asset_host) &&
Rails.application.config.action_controller.asset_host.present?
| css_parser | ||
| htmlbeautifier (~> 1.3) | ||
| htmlentities (~> 4.3.4) | ||
| htmlentities (~> 4.3) |
Collaborator
Author
There was a problem hiding this comment.
Looks like this happened upstream, not sure why the lock file wasn't updated
saharvey
commented
Jul 15, 2026
| <%= render_component_tag :i, style: "height: #{size_rems}; width: #{size_rems}; #{@html_attrs[:style]}", class: "icon-stroke-#{stroke}" do %> | ||
| <svg> | ||
| <use href="/lookbook-assets/img/lucide-sprite.svg#<%= @icon_name %>"></use> | ||
| <use href="<%= sprite_path %>#<%= @icon_name %>"></use> |
Collaborator
Author
There was a problem hiding this comment.
Use lookbook_asset_path instead of hardcoding the path
| css_parser | ||
| htmlbeautifier (~> 1.3) | ||
| htmlentities (~> 4.3.4) | ||
| htmlentities (~> 4.3) |
Collaborator
Author
There was a problem hiding this comment.
Looks like this happened upstream, not sure why the lock file wasn't updated
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's this PR do?
Adds CDN support to
lookbook_asset_pathso Lookbook's pre-built CSS/JS/image assets can be served from a host app's CDN in production, instead of only via the gem's built-inRack::Staticmiddleware.
Also fixes the icon component to route its SVG sprite through that same helper instead of a hardcoded path.
Why is it needed?
Host apps that serve assets from a CDN had no way to get Lookbook's own assets onto that CDN, they were only ever served from the gem's in-process middleware, which doesn't work once a host app is actually accessed through a CDN in front of the app.
Where should the reviewer start?
Start with
app/helpers/lookbook/application_helper.rbwherelookbook_asset_pathis the core of this change (CDN URL vs. middleware fallback), and once that logic makes sense,lib/tasks/lookbook_assets.rake(how the assets actually get onto disk for deploy) and the icon component's use of the same helper both follow naturally from it.How should this be manually tested?
This is on staging 2: https://stg-02.studiosity.com/connect/admin/lookbook/inspect/button/warning
Screenshots (if appropriate)