Add Tag component

[tijmenb]

Quick PR to discuss some patterns and approaches.

[tijmenb]

For Apply we need to add a custom CSS class. Do we skip doing that for now?

[aliuk2012]

I think this would be worth while. On Publish we are using custom classes on tags

• https://github.com/DFE-Digital/publish-teacher-training/blob/master/app/decorators/application_decorator.rb
• https://github.com/DFE-Digital/publish-teacher-training/blob/master/app/views/providers/allocations/_request_status.html.erb#L23-L25

[aliuk2012]

Could follow a similar pattern to this https://github.com/DFE-Digital/govuk-components/blob/master/app/components/govuk_component/back_link.rb#L7-L8

[tijmenb]

Should we validate the colours?

[tijmenb]

Or should we just allow passing in classes like the nunjucks macro?

[peteryates]

The angle I took with the form builder was to provide arguments for the documented features of a component. Here's a relevant example for configuring input width; in this case as a user, I'd probably want to be able to call something like Tag::TagComponent.new(text: 'Danger', colour: 'red').

[tvararu]

There are mixed feelings on this. @adamsilver once tried doing something custom with a component in the Form builder that wasn't possible because you couldn't pass in custom classes. I ended up submitting a patch upstream: x-govuk/govuk-form-builder#96

The design system does allow you to pass custom classes for most of its components, and I think it's to allow users to break encapsulation where it's necessary, such as defining new variants of a component that are not yet upstream.

I think it makes sense to allow not only passing in classes but also providing smart arguments that interpret the specification in order to make components easy to use. So though it's more hard work, I think all 3 of these have their use-cases, and should be supported:

GovukComponent::Tag.new(text: 'Danger', colour: 'red')

GovukComponent::Tag.new(text: 'Danger', colour: 'danger')

GovukComponent::Tag.new(text: 'Danger', classes: 'govuk-tag--red')

[tvararu]

If anything, the 'danger' semantic variant could be dropped, I think it's the least important.

[aliuk2012]

What makes the nunjuck macros great is that they also allow you to pass in additional html attributes and the fixability it brings. Heres some standards they use to determine the API for components. I think we should inherit this to help use keep this gem up to date when the govuk-frontend makes breaking changes.

[peteryates]

The Nunjucks macros are powerful and flexible but I'm really not a fan of passing in strings of HTML. Invariably, from experience, you just end up with HTML in a string in a block of JavaScript/Ruby in a HTML(-like) template.

[tijmenb]

This is different from the existing tests. I wonder if this makes it easier to check whether or not the HTML is correctly formed. For me this was easier to do then translating it into have_css assertions like the previous tests.

[tvararu]

This is similar to Jest inline snapshots from the JavaScript world, can confirm it is a very efficient and sane way for testing small blocks of HTML or JSON.

One difference is that jest (rspec equivalent) gives you a button (jest --updateSnapshot) to update all out of date snapshots, which makes refactoring tests en-masse very easy for banal changes.

[peteryates]

I tend to lean towards the have_css approach because it's ignorant of whitespace, indentation etc.

expect(html).to have_css('strong', class: 'govuk-tag govuk-tag--green', text: 'Alert')

On smaller, simpler components matching the HTML exactly works well and is readable, this is fine by me

[tvararu]

Yeah agree re: whitespace @peteryates. That's where having a tool do it for you comes in really handy.

(This exists but it doesn't look maintained https://github.com/yesmeck/rspec-snapshot)

[aliuk2012]

🤔 If we render the components on a page we could use jest to snapshot the component. We could go one step further and do some visual regression testing using jest-image-snapshot (developed by american express) https://www.npmjs.com/package/jest-image-snapshot

[aliuk2012]

These are the params that govuk-frontend nunjuck macro accepts https://github.com/alphagov/govuk-frontend/blob/master/src/govuk/components/tag/tag.yaml#L1-L17

[peteryates]

I've rebased the above work and modernised it a bit and made it consistent with other components, plus added to the examples page.

[adamsilver]

@peteryates you don't need to (and shouldn't) capitalise the text, the CSS styling takes care of that.

[peteryates]

@adamsilver capitalize only changes the first letter here, so red becomes Red in the output HTML. However I agree it's unnecessary, I'll remove it.