Add View Components - Githubissues

Description of issue

Whitehall has large complex views. The established pattern is to break down the pages into many partials. Each of these partials have different functions and vary in levels of complexity. We then test these views through a combination of feature specs and view controller specs.

These specs are slow when it comes to performance. They're also often somewhat laborious to write as you are unable to isolate the section of the view that you are working on and subsequently interested in testing. As many views are large and complex, you often have to handle interactions with other applications. For example you may well have to stub out some calls to Publishing API despite only being interested in the fact that when you add some fields to the page, they render and have the correct values.

Often there is quite a lot of logic that sits within partials in the form of conditionals, or is pulled in from the controller or helpers.

An example of this can be seen in the way we construct images for editions

https://github.com/alphagov/whitehall/blob/ecb22a6a76ccbfbd83de00b65bd8e6cccdf8c39e/app/views/admin/editions/_image_template.html.erb

Within this template we conditionally render 5 different inputs or pieces of content based on the object's state.

We also often see the use of variables within views And many locals being passed into partials.

In order to properly test all the combinations of conditionals and locals passed in, we're required to write an exhaustive amount of controller view specs, which often are essentially functioning as unit tests, but with none of the performance.

In reality, a lot of logic in the views is not properly tested.

As we're currently starting to port Whitehall to use the GOV.UK Design System , now is a good time to establish a new pattern for ensuring we're confident that when complex logic is required in rendering views, it's properly tested and efficient.

View Components

View Components are a solution to these issues and are used by quite a range of well known organisations such as Github.

There’s loads of good information on them here https://viewcomponent.org/, but essentially they’re an extension of the presenter pattern and allows you to consolidate the logic needed for a partial/template into a single class. Unlike a presenter this class is directly coupled to a html.erb file. They allow you to encapsulate and test view logic easily and effectively.

They’re around 100x faster than traditional controller specs and live performance is around 10x faster than partials as they are precompiled at boot rather than runtime.

Unlike a partial, they have an explicit interface which uses the initialise method which helps reduce bugs that can slip in when partials are reused across many contexts with different objects.

They’re particularly useful for creating reusable components or when a view has quite a bit of embedded ruby with varied outputs based on state as they’re easy to test.

View components use Capybara matchers so they’re easy for developers to pick up and use as it’s a familiar syntax.

When should you use them?

I think there’s 3 uncontroversial use cases for view components in our codebase

When we create a reusable component that is used across many pages - for example, error summary handling
When we create a partial which requires many locals - for example, selecting a date and time
When there is a lot of logic embedded in a view with many outputs based on the objects state - for example, rendering image fields

I’ve spiked out moving all 3 of the above into a View Component.

https://github.com/alphagov/whitehall/pull/6947 - Moves the error summary partial to a View Component https://github.com/alphagov/whitehall/pull/6948 - Moves the datetime_fields partial to a View Component https://github.com/alphagov/whitehall/pull/6949 - Updates the images_template partial to render a View Component

Using view components

File structure

VIew components sit at the top level of the directory. Much like the rest of the app, we can create additional folders for interfaces. Largely, for all the work we're doing transitioning Whitehall they should live inside app/components/admin/*

How to use them

You can use:

govuk-docker-run rails g component admin/ComponentName

govuk-docker-run rails g component admin/component_name

to generate a new component. This will create the following files for you:

If we look at the DatetimeComponent as an example i ran govuk-docker-run rails g component admin/datetime

Looking at these screenshots of the current implementation we can see that a lot of logic is passed into the partials via locals. Additional variables are created. Then the variables are used within the various fields

There's a lot going on on in the partial.

the field name being passed in is used to construct the name and id of the datetime fields
. the value of the select is being set to the value of the relevant datetime on the object (for example, if the fieldname is first_published_at it calls edition.first_published_at.year for the year select.
If there is no value on the object passed in for the corresponding field name then it checks to see if a default date has passed in
It checks to see if a start year and end year have been passed in and if so passes them through to the select_year method to ensure the correct options are generated for the year input

I've converted this to a view component and tried to pull as much logic out of the view as possible

What used to be passed into the partial as locals, are now passed into the component on initialisation

Attributes that are called directly in the view are added as attribute readers. Additional logic that is passed into the view is added via private methods. Any values that are called more than once if the view are memoised.

Even with as much logic pulled out the view as possible it still ends up being a fairly busy component.

Which hopefully makes it clear why heavily unit testing something like this is pretty important.

Here i've tested that all the required functionality is working as expected.

It tests that:

it constructs the input names and ids based on the class if a prefix isn't passed in and the field name
You can pass in a prefix which overrides using the objects class to build the fields prefix. . For example, if you pass in a consultation you can pass in edition to set the fields names to edition[attribute]. This is required due to all the inheritance that is going on with editions and attachments
That the datetime inputs use the datetime on the object if one is present to populate their value. If not the default time if one is passed in.
That when you input a start year or end year, the correct options are created in the year dropdown

If you get a chance to have a look through the PRs and have any questions or feedback it would be much appreciated!

Thanks for putting this together and for all the details about them.

As per our brief chat, there's probably 3 key things to work out.

1) How can we objectively describe what should be a view component and what shouldn't

We want to minimise subjectivity and maximise objectivity. It looks like:

When we create a reusable component that is used across many pages - for example, error summary handling When we create a partial which requires many locals - for example, selecting a date and time When there is a lot of logic embedded in a view with many outputs based on the objects state - for example, rendering image fields

Is a great start to this.

I guess some potentially subjective things are:

if you need a partial that is reusable across a multiple controllers views (i.e. something that might have sat in a shared views folder)
you would never use one as the main view of an action (so they'd only be used in the equivalent of partials)

And potentially unclear are:

What point should a partial be a view component? (I'm not convinced on the above about number of variables, I'd imagine it's more about doing logic with the variables)
Should they be coupled to a particular view?
When should something be a publishing component (i.e. a local one as per autocomplete)?

2) What, if any, organisation convention should they have

This issue is more pertinent the wider we intend to use them. If we expect there to be < 30 it seems fine to have them just in components with a flat naming structure. If we expect that every view might have 4 or 5 we'll easily end up with hundreds.

A key factor of this seems to be if we're going to have components coupled to views. I'd imagine in that case we'd want a structure that maps similar to views like components/admin/attachments

Also I think in the prototyping we've put them in components/admin - this is inconsistent with local publishing components which don't have an admin prefix - they shouldn't really need an admin prefix unless they're coupled to a view.

3) relationship with publishing components

We already have use of publishing components in Whitehall, with a handful of local ones and the use of the ones in the gem.

A question is how will our components integrate with the gem and the component guide tool (which is currently broken in Whitehall but that's a different story)

Some things I wonder are:

would any of our view components become local components? if so, how do we decide? if not are we happy to not have a local component guide system
would anyone create local components? would we port the ones we have to view components?
if we decide we need a component to be available to other apps - i.e. port to the gem - are we happy to just rewrite it in the gem style?

4) Are we going to follow component isolation principles?

Sorry, we didn't discuss this before but it came to my mind putting this together. It's a bit of a leading question as I totally think we should.

There's a description about it in: https://github.com/alphagov/govuk_publishing_components/blob/main/docs/component_principles.md as well as other principles we may want to consider - since it's good prior art to build on (though not all will be applicable).

Hey Kevin,

Thanks for all the feedback. Daniel, Dilowar, Ollie and me had a really useful discussion this morning just after you popped into your meeting which I think has helped to consolidate the answer to a bunch of these questions.

I think it makes sense to answer these questions at the same time, because the difference between how we would use View Components and when to use a Publishing Component are intertwined.

I think there's 3 discreet needs that a combination of Publishing Components and View Components can meet. Essentially they are:

Publishing Components gem

App agnostic components, anything that we build that is app agnostic and meet the GOV.UK Design System standards. These should apply the Component Principles and be added to the Publishing Components gem.

Local Publishing Components

These are app specific components. For example, we need a datetime input component which uses selects while we're doing a like for like port. This wouldn't meet GOV.UK Design System standards so other apps shouldn't use it. Eventually down the line the Date Component in the gem could be extended to handle time and we could use that instead (but that can be worried about post like for like port).

We came to the conclusion that it makes sense to use a Publishing Component over a View Component when the need that the component meets is view agnostic. Much like a Publishing Component it should be able to be applied to any page that requires it. It should live within the local components folder (i.e. app/views/components) with Miller Columns, etc. and should be documented and tested. In terms of testing, I think we should look into how Publishing Components are tested in the gem and replicate that. Currently, we're not really testing these except in the feature specs.

In the example PRs that would mean that the DateTimeComponent and the ErrorSummaryComponent, would not use ViewComponents, and instead would become local Publishing Components.

View Components

For the most part, these should be non-reusable components and be linked to a specific view. The exception to this might be when a fairly niche bit of code is reused across more than 1 page. To illustrate this. Imagine we added a review previous edition page, and all it did was present the edition show pages sections. We may well want to reuse the code that presents the sections.

etc.

In this situation the Component is not view agnostic and cannot be rationalised as such, so it would make sense to use a View Component that would be shared across multiple views. However, this would be the exception to the rule.

Their purpose is extract complex logic in views into manageable chunks and improve test coverage within views. Unlike the above two examples, these will not stick to the Component Principles. They will often include business logic that was previously in the views and will directly test the output of the HTML.

As they're linked to views, they will largely mimic the file structure of views. For example, if it's a component that is used on the edit edition page. The file structure would be the following:

components/editions/edit/image_component.rb
components/editions/edit/image_component.html.erb
test/components/editions/edit/image_component_test.rb

This way it's easy to understand where the component is used and we won't have a crowded top level folder.

Any components that are reused within the same view folder (edition/*) would live in a shared folder. So if the image_component was reused in the show page. It would live in

components/editions/shared/image_component.rb
components/editions/shared/image_component.html.erb
test/components/editions/shared/image_component_test.rb

If a need arose to add an image to another controller and it shared the same functionality as the edit edition page (for example, adding an image to a role), then it should be abstracted into a reusable Publishing Component.

Could/should View Components be used to build Publishing Components?

Could yes, should no. We came to the conclusion that for now having a separation of concerns and implementation between the two would be useful to enforce when each should be used.

Down the line it may make sense to have two discreet uses for View Components and document what is required for each, but for now it's simpler not to.

Awesome thanks for that @davidgisbey, great progress 👍

So I'm going to try summarise that back to see if I've understood fully:

Whitehall ViewComponent is not a replacement for govuk_publishingcomponents (both local and gem), we will continue to use and create local components for functionality that is not coupled to a view (aside, I assume in future we need to set-up ActionView tests for these, something like https://github.com/alphagov/government-frontend/blob/main/test/views/content_items/attachments.html.erb_test.rb ?)_
we intend to use ViewComponents in controller specific circumstances, thus we expect them to be namespaced based on their Controller / Action. For example. Admin::Editions::Edit::ImageComponent - effectively as a replacement for complex partials.
if a component is shared between actions of a controller it goes under a shared namespace e.g. Admin::Editions::Shared (aside can we drop the shared and just put them in Admin::Editions it doesn't feel like this would be unmanageable if you're already namespaced to controller)?

It sounds like we're pretty close to asserting what ViewComponents mean to us, from our various chats. Here's my attempt to articulate it for clarity, it's to try work out if we're all on the same page so do please say if bits don't fit in:

View components are intended for controller specific view needs, particularly to provide an easier way to test views, for components used in multiple places we have govuk_publishing_components. We consider view components to be a part of the view layer of the application so should only contain logic that relates to converting the input arguments into HTML, if more complex business logic is needed you can use Helper functions or add it to methods on the object passed to the component. Tests for view components should be all based on HTML output, If you need to test something more that it's likely not something that belongs in a ViewComponent.

A further question:

do we need to consider where associated CSS / JS might be stored? I'm guessing we might have very little of it since we'd hope it's mostly publishing component specific. I suppose we could carry on using the views/<action>/<controller> pattern?

I think that's a really good summation. I agree with all of that.

I think there would be very little JS and CSS for this like you say. A View Component may well use a Publishing Component within it, but i wouldn't expect one to have its own JS files.

If CSS is needed within the component though, i'd expect it to live within the view file here

I wonder if there is JS specific to a view whether it should live here view component or not. For example, shared view logic across the new and edit edition views (all the conditional rendering of guidance etc. should live here)

One thing that i did just think of is the

Admin::Editions is going to get quite busy. A lot of the View Components are going to be used in the new and edit views so would live at the top level. We might need to have a think about that.

Great stuff, it looks like we're in business then.

There's a couple of tasks we should do while introducing this: 1. Leave a record of our decision to introduce them and 2. provide documentation on how to use them.

To meet these, we should create a docs/adrs directory and add the first Architecture Decision Record for Whitehall (needn't be complex, can mostly use the information from https://github.com/alphagov/whitehall/issues/6954#issuecomment-1293465983 as that seems to have the domain decisions). As an ADR is a point in time doc I don't think that's the best choice to document how they're used (naming, directories, js etc) so I think that should be something in the docs/ directory and maybe have a app/components/README.md that points to that file for documentation on Whitehall view component conventions.

These definitely don't have to be tasks for just you @davidgisbey we can work out how to split them among the team.

Admin::Editions is going to get quite busy. A lot of the View Components are going to be used in the new and edit views so would live at the top level. We might need to have a think about that.

Sure - does seem likely as there are 74 partials in there now (49 without _legacy). are any of them effectively children of components within it? I wonder if there might be some hierarchy there. I somewhat wonder if there could be a form view component that then has children for it's fields and that might neaten a bunch

If CSS is needed within the component though, i'd expect it to live within the view file here I wonder if there is JS specific to a view whether it should live here view component or not. For example, shared view logic across the new and edit edition views (all the conditional rendering of guidance etc. should live here)

Ok sure, yeah I've seen in viewcomponent.org the example of CSS and JS next to the component though I'm not sure if that'll go for asset pipeline.

I imagine the simplest thing to suggest is that if there is CSS or JS specific to the component it should be in a folder of assets/stylesheets/views/admin/edition/<component_name>.<css|js>

Just wanted to check is it thumbs up, more discussion or thumbs down on:

Add view tests for publishing components?
Drop the Shared namespace?

alphagov / whitehall

Add View Components #6954