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

Accept full and partial component paths in resolver #140

Merged
merged 1 commit into from Feb 22, 2016

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Feb 22, 2016

For components to be able to call other components (composeable components) they need to use a component path that includes .raw (this is not a standard Rails format, but a convention
we use).

Example

Consider the following component:

<div class="govuk-govspeak-html-publication">
  <%= partial: 'govuk_component/govspeak', locals: govspeak_locals %>
</div>

In this example, the application can make a request for the govspeak-html-publication component (govuk_component/govspeak-html-publication.raw.html.erb), and then a subsequent request for govspeak, both will work. Slimmer converts the component name into a filename.

When testing in static we don't use slimmer, so the test attempts to load govuk_component/govspeak.html.erb rather than govspeak.raw.html.erb version. That files doesn't exist and the tests fail.

Changing the nested component to one that works with tests:

<div class="govuk-govspeak-html-publication">
  <%= render file: 'govuk_component/govspeak.raw', locals: govspeak_locals %>
</div>

This works locally, but when requested by an application through slimmer, slimmer would take govspeak.raw and request govspeak.raw.raw.html.erb.

The simplest approach is to allow apps to request components using the complete template filename, the raw variant, or just its name. This avoids bespoke template resolver logic.

There is a longer term plan to move component loading to its own API, rather than riding on the Rails template resolver magic.

cc @dsingleton @benlovell

@@ -56,7 +56,10 @@ def fetch(template_url)
end

def template_url(template_path)
[static_host, "templates", "#{template_path}.raw.html.erb"].join('/')
# accept full or partial component template_path from application

This comment has been minimized.

@benlovell

benlovell Feb 22, 2016
Contributor

I'm not sure this comment is necessary, since the detail in the commit that modified is good.

@@ -26,4 +29,15 @@
assert_match /<test-govuk-component data-template="govuk_component-name">/, templates.first.args[0]
end
end

private

This comment has been minimized.

@benlovell

benlovell Feb 22, 2016
Contributor

I don't think access modifiers are necessary in tests. If it's to control scoping for other methods that would clash, that's a larger issue that should be addressed.

@benlovell
Copy link
Contributor

@benlovell benlovell commented Feb 22, 2016

Approach makes sense to me. 👍

For components to be able to call other components from within
(composeable components) they need to use a component path that
includes `.raw` (this is not a standard Rails format, but a convention
we use).
@fofr fofr force-pushed the more-flexible-component-loading branch from e596d39 to 2d139e2 Feb 22, 2016
@dsingleton
Copy link
Contributor

@dsingleton dsingleton commented Feb 22, 2016

👍 from me. It's a little messy, as noted, but the mess is behind the 'API' of Slimmer/Static.

Definitely agree it's a :plus: 1️⃣ for moving to an explicit component call approach (eg, <%= render_component(name, locals) %> in the near future.

benlovell added a commit that referenced this pull request Feb 22, 2016
Accept full and partial component paths in resolver
@benlovell benlovell merged commit 5a8dd90 into master Feb 22, 2016
1 check passed
1 check passed
default "Build #128 succeeded on Jenkins"
Details
@benlovell benlovell deleted the more-flexible-component-loading branch Feb 22, 2016
fofr added a commit that referenced this pull request Feb 22, 2016
* Allow applications to request components using full or partial
component paths, eg "name", "name.raw" and "name.raw.html.erb". This
allows components to be nested within other components.

#140
@fofr fofr mentioned this pull request Feb 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.