Skip to content

Commit

Permalink
Use accordion components vs low-level markup
Browse files Browse the repository at this point in the history
Trello: https://trello.com/c/umBDQZUc

In this commit [1], @chrislo said:

> I've had to use the "raw" HTML version of the accordian component[2]
rather than the one in the publishing component library[3] since the
latter doesn't support passing a block of arbitrary content into each
section.

However, it seems to work if you pass HTML as `content.html` attribute
for an accordion item - I hope I'm not missing something!

So I've used this approach in both
`app/views/devise/invitations/new.html.erb` &
`app/views/batch_invitation_permissions/new.html.erb`. I've also
extracted `UsersHelper#items_for_permission_checkboxes` and used a
`user` parameter with a default value to handle the slight difference
between the behaviour needed in the two pages.

Although there's still a little duplication between the two pages, I
think the use of the library accordion component and the shared helper
method has probably removed enough for now. I think extracting any more
would make the template harder to understand.

[1]: 539a70b
[2] https://design-system.service.gov.uk/components/accordion/
[3] https://components.publishing.service.gov.uk/component-guide/accordion
  • Loading branch information
floehopper committed Oct 6, 2023
1 parent b03a67d commit fc2059f
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 50 deletions.
12 changes: 12 additions & 0 deletions app/helpers/users_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,16 @@ def options_for_organisation_select(selected: nil)
end
end
end

def items_for_permission_checkboxes(application:, user: nil)
permissions_for(application).map do |permission|
{
id: "user_application_#{application.id}_supported_permission_#{permission.id}",
name: "user[supported_permission_ids][]",
label: formatted_permission_name(application.name, permission.name),
value: permission.id,
checked: user&.has_permission?(permission),
}
end
end
end
44 changes: 18 additions & 26 deletions app/views/batch_invitation_permissions/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,22 @@
%>

<%= form_for @batch_invitation, url: :batch_invitation_permissions, method: :post do |f| %>
<div class="govuk-accordion" data-module="govuk-accordion">
<% policy_scope(:user_permission_manageable_application).reject(&:retired?).each_with_index do |application, idx| %>
<div class="govuk-accordion__section">
<div class="govuk-accordion__section-header">
<h2 class="govuk-accordion__section-heading">
<span class="govuk-accordion__section-button" id="accordion-heading-<%= idx %>">
<%= application.name %>
</span>
</h2>
</div>
<div class="govuk-accordion__section-content" aria-labelledby="accordion-heading-<%= idx %>">
<%= render "govuk_publishing_components/components/checkboxes", {
name: "permissions_for_#{application.id}",
heading: "Permissions for #{application.name}",
items: permissions_for(application).map do |permission|
{ label: formatted_permission_name(application.name, permission.name),
value: permission.id,
id: "user_application_#{application.id}_supported_permission_#{permission.id}",
name: "user[supported_permission_ids][]" }
end
} %>
</div>
</div>
<% end %>
</div>
<%= render "govuk_publishing_components/components/button", { text: "Create users and send emails" } %>
<%= render "govuk_publishing_components/components/accordion", {
items: policy_scope(:user_permission_manageable_application).reject(&:retired?).map do |application|
{
heading: {
text: application.name
},
content: {
html: render("govuk_publishing_components/components/checkboxes", {

This comment has been minimized.

Copy link
@chrislo

chrislo Oct 9, 2023

Contributor

Oooh, I didn't spot you could do this - thanks @floehopper 👍

name: "permissions_for_#{application.id}",
heading: "Permissions for #{application.name}",
items: items_for_permission_checkboxes(application:),
})
},
}
end
} %>

<%= render "govuk_publishing_components/components/button", { text: "Create users and send emails" } %>
<% end %>
38 changes: 14 additions & 24 deletions app/views/devise/invitations/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -81,32 +81,22 @@
} %>
<% end %>

<div class="govuk-accordion" data-module="govuk-accordion">
<% policy_scope(:user_permission_manageable_application).reject(&:retired?).each_with_index do |application, idx| %>
<div class="govuk-accordion__section">
<div class="govuk-accordion__section-header">
<h2 class="govuk-accordion__section-heading">
<span class="govuk-accordion__section-button" id="accordion-heading-<%= idx %>">
<%= application.name %>
</span>
</h2>
</div>
<div class="govuk-accordion__section-content" aria-labelledby="accordion-heading-<%= idx %>">
<%= render "govuk_publishing_components/components/checkboxes", {
<%= render "govuk_publishing_components/components/accordion", {
items: policy_scope(:user_permission_manageable_application).reject(&:retired?).map do |application|
{
heading: {
text: application.name
},
content: {
html: render("govuk_publishing_components/components/checkboxes", {
name: "permissions_for_#{application.id}",
heading: "Permissions for #{application.name}",
items: permissions_for(application).map do |permission|
{ label: formatted_permission_name(application.name, permission.name),
value: permission.id,
id: "user_application_#{application.id}_supported_permission_#{permission.id}",
name: "user[supported_permission_ids][]",
checked: f.object.has_permission?(permission) }
end
} %>
</div>
</div>
<% end %>
</div>
items: items_for_permission_checkboxes(application:, user: f.object),
})
},
}
end
} %>

<div class="govuk-button-group">
<%= render "govuk_publishing_components/components/button", {
Expand Down

0 comments on commit fc2059f

Please sign in to comment.