Skip to content

Commit

Permalink
Allow custom filenames to be passed for JS and CS
Browse files Browse the repository at this point in the history
We're adding an admin interface to our application and want to use
retain application.js & application.scss for the public interface.

This will allow us to pass admin.js and admin.scss in as args so we
can retain only loading the required components for each so the page
load time is fast.

There has been some added complexity in testing this as the #javascript_include_tag
and #stylesheet_link_tag methods being passed a filename that isn't present
in the assets directory will raise an error due to the file not being found
when it's during attempted compilation.

We have a few options here:

1. stub out the methods and return an empty string so there is no attempt to
compile the file. In this situation we would just want to check that the
methods receive the correct filename.
2. create the files dynamically prior to running the test, check the output
on the rendered page and then delete the files after the test has run.
3. skip the test entirely.

We've opted for option 1 as probably less complex and faster than option 2
and preferable to not testing at all.

Unfortunately we've had to use "allow_any_instance_of" and
"expect_any_instance_of" to stub the calls to check that the JS and CSS
filenames are being passed into the #javascript_include_tag and #stylesheet_link_tag
methods.

These are implemeneted in the ActionView::Helpers::AssetTagHelper helper
which is included in ActionView::Base which is particularly gnarly to
attempt to stub a single instance of.

While this isn't ideal, it's probably preferable to having to stub out a
series of objects for what is essentially a very simple test which checks
the correct filename is passed in.
  • Loading branch information
davidgisbey committed Apr 25, 2024
1 parent 93da90a commit 5ba7778
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 2 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
useful summary for people upgrading their application, not a replication
of the commit log.

## Unreleased

- Update layout_for_admin component to take custom asset filenames ([PR #3993](https://github.com/alphagov/govuk_publishing_components/pull/3993))

## 38.1.0

* Fix rendering of unnumbered papers ([PR #3988](https://github.com/alphagov/govuk_publishing_components/pull/3988))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<%
add_gem_component_stylesheet("layout-for-admin")
js_filename ||= "application"
css_filename ||= "application"

product_name ||= "Publishing"
%>
Expand All @@ -13,7 +15,7 @@
<meta name="viewport" content="width=device-width, initial-scale=1">
<%= csrf_meta_tags %>
<%= favicon_link_tag "govuk_publishing_components/favicon-#{environment}.png" %>
<%= stylesheet_link_tag "application", media: "all" %>
<%= stylesheet_link_tag css_filename, media: "all" %>
<%= javascript_include_tag "govuk_publishing_components/vendor/modernizr" %>
<%= yield :head %>
</head>
Expand All @@ -22,6 +24,6 @@
document.body.className += ' js-enabled' + ('noModule' in HTMLScriptElement.prototype ? ' govuk-frontend-supported' : '');
<% end -%>
<%= yield %>
<%= javascript_include_tag "application" %>
<%= javascript_include_tag js_filename %>
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,21 @@ examples:
browser_title: 'A page title'
block: |
<!-- You probably want to use the header, main & footer components here -->
with_custom_js_filename:
description: An alternative JS filename can be used in place of the default `application.js` if required (note that this cannot easily be demonstrated here).
data:
environment: production
product_name: Publishing
browser_title: 'A page title'
js_filename: "application"
block: |
<!-- You probably want to use the header, main & footer components here -->
with_custom_css_filename:
description: An alternative JS filename can be used in place of the default `application.scss` if required (note that this cannot easily be demonstrated here).
data:
environment: production
product_name: Publishing
browser_title: 'A page title'
css_filename: "application"
block: |
<!-- You probably want to use the header, main & footer components here -->
21 changes: 21 additions & 0 deletions spec/components/layout_for_admin_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,25 @@ def component_name

assert_select 'meta[name="robots"][content="noindex,nofollow,noimageindex"]', visible: false
end

it "can receive a custom js filename" do
allow_any_instance_of(ActionView::Base).to receive(:javascript_include_tag).with("govuk_publishing_components/vendor/modernizr")
expect_any_instance_of(ActionView::Base).to receive(:javascript_include_tag).with("admin").once

render_component(
browser_title: "Hello, admin page",
environment: "production",
js_filename: "admin",
)
end

it "can receive a custom css filename" do
expect_any_instance_of(ActionView::Base).to receive(:stylesheet_link_tag).with("admin", media: "all")

render_component(
browser_title: "Hello, admin page",
environment: "production",
css_filename: "admin",
)
end
end

0 comments on commit 5ba7778

Please sign in to comment.