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

Split ComponentDoc into ComponentDoc and ComponentDocResolver #34

Merged
merged 2 commits into from
Aug 18, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -3,16 +3,16 @@ class ComponentGuideController < GovukPublishingComponents::ApplicationControlle
append_view_path File.join(Rails.root, "app", "views", "components")

def index
@component_docs = ComponentDoc.all
@component_docs = component_documentation_resolver.all
end

def show
@component_doc = ComponentDoc.get(params[:component])
@component_doc = component_documentation_resolver.get(params[:component])
@guide_breadcrumbs = [index_breadcrumb, component_breadcrumb(@component_doc)]
end

def fixture
@component_doc = ComponentDoc.get(params[:component])
@component_doc = component_documentation_resolver.get(params[:component])
@component_fixture = @component_doc.fixtures.find { |f| f.id == params[:fixture] }
@guide_breadcrumbs = [
index_breadcrumb,
@@ -25,7 +25,7 @@ def fixture

def preview
@component_fixtures = []
@component_doc = ComponentDoc.get(params[:component])
@component_doc = component_documentation_resolver.get(params[:component])
@preview = true

if params[:fixture].present?
@@ -37,6 +37,10 @@ def preview

private

def component_documentation_resolver
@component_documentation_resolver ||= ComponentDocResolver.new
end

def index_breadcrumb
{
title: GovukPublishingComponents::Config.component_guide_title,
54 changes: 19 additions & 35 deletions app/models/govuk_publishing_components/component_doc.rb
Original file line number Diff line number Diff line change
@@ -1,25 +1,19 @@
module GovukPublishingComponents
ComponentDoc = Struct.new(:id, :name, :description, :body, :accessibility_criteria, :fixtures) do
def self.get(id)
component = fetch_component_doc(id)
self.build(component)
end

def self.all
fetch_component_docs.map { |component| build(component) }
end

def self.build(component)
fixtures = component[:fixtures].map { |id, data|
GovukPublishingComponents::ComponentFixture.new(id.to_s, data)
}

self.new(component[:id],
component[:name],
component[:description],
component[:body],
component[:accessibility_criteria],
fixtures)
class ComponentDoc

Choose a reason for hiding this comment

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

Why have we moved away from a Struct? It looks like we've just wound up with more boilerplate code. To make it immutable? We could use something like immutable-struct?

attr_reader :id,
:name,
:description,
:body,
:accessibility_criteria,
:fixtures

def initialize(id, name, description, body, accessibility_criteria, fixtures)
@id = id
@name = name
@description = description
@body = body
@accessibility_criteria = accessibility_criteria
@fixtures = fixtures
end

def fixture
@@ -30,20 +24,6 @@ def other_fixtures
fixtures.slice(1..-1)
end

def self.fetch_component_docs
doc_files = Rails.root.join("app", "views", "components", "docs", "*.yml")
Dir[doc_files].sort.map { |file| parse_documentation(file) }
end

def self.fetch_component_doc(id)
file = Rails.root.join("app", "views", "components", "docs", "#{id}.yml")
parse_documentation(file)
end

def self.parse_documentation(file)
{ id: File.basename(file, ".yml") }.merge(YAML::load_file(file)).with_indifferent_access
end

def html_body
govspeak_to_html(body) if body.present?
end
@@ -52,6 +32,10 @@ def html_accessibility_criteria
govspeak_to_html(accessibility_criteria) if accessibility_criteria.present?
end

def partial_path
"components/#{id}"
end

private

def govspeak_to_html(govspeak)
41 changes: 41 additions & 0 deletions app/models/govuk_publishing_components/component_doc_resolver.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
module GovukPublishingComponents
class ComponentDocResolver
def get(id)
component = fetch_component_doc(id)
build(component)
end

def all
fetch_component_docs.map { |component| build(component) }
end

private

def build(component)
fixtures = component[:fixtures].map { |id, data|
ComponentFixture.new(id.to_s, data)
}

ComponentDoc.new(component[:id],
component[:name],
component[:description],
component[:body],
component[:accessibility_criteria],
fixtures)
end

def fetch_component_docs
doc_files = Rails.root.join("app", "views", "components", "docs", "*.yml")
Dir[doc_files].sort.map { |file| parse_documentation(file) }
end

def fetch_component_doc(id)

Choose a reason for hiding this comment

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

So every time we try to access documentation we have to perform multiple file operations? Feels a little bit icky; could potentially memoise this stuff at a static level? Although given that this is all just internal code used by developers, maybe we don't have to worry about it too much.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to make it faster, and memoising makes sense. As this is predominantly a tool for developing components we need to balance that with the problem of too much caching hiding local changes.

Choose a reason for hiding this comment

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

Makes sense to me. Not sure we'd notice much of a performance change, anyway.

file = Rails.root.join("app", "views", "components", "docs", "#{id}.yml")
parse_documentation(file)
end

def parse_documentation(file)
{ id: File.basename(file, ".yml") }.merge(YAML::load_file(file)).with_indifferent_access
end
end
end
10 changes: 9 additions & 1 deletion app/models/govuk_publishing_components/component_fixture.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
module GovukPublishingComponents
ComponentFixture = Struct.new(:id, :data) do
class ComponentFixture
attr_reader :id,
:data

def initialize(id, data)
@id = id
@data = data
end

def name
id.humanize
end
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<div class="component-call code-block" contenteditable>
<pre class="language-ruby"><code>&lt;%= render 'components/<%= component_doc.id %>'<% if fixture.data? %>, <%= fixture.pretty_data %><% end %> %&gt;</code></pre>
<pre class="language-ruby"><code>&lt;%= render '<%= @component_doc.partial_path %>'<% if fixture.data? %>, <%= fixture.pretty_data %><% end %> %&gt;</code></pre>
</div>
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<div class="component-guide-preview">
<%= render "components/#{@component_doc[:id]}", fixture.data %>
<%= render @component_doc.partial_path, fixture.data %>
</div>
Original file line number Diff line number Diff line change
@@ -3,6 +3,6 @@
<% if @component_fixtures.length > 1 %>
<h2 class="preview-title"><a href="<%= component_fixture_path(@component_doc.id, fixture.id) %>"><%= fixture.name %></a></h2>
<% end %>
<%= render "components/#{@component_doc[:id]}", fixture.data %>
<%= render @component_doc.partial_path, fixture.data %>
</div>
<% end %>