search

alphagov / govuk-frontend

GOV.UK Frontend contains the code you need to start building a user interface for government platforms and services.
https://frontend.design-system.service.gov.uk/
MIT License
1.18k stars 325 forks source link

Provide a way for maintainers of ports to check that they're using equivalent HTML markup #1830

Closed 36degrees closed 4 years ago

36degrees commented 4 years ago

What

Provide a way for maintainers of ports to ensure that, for given scenarios, their port generates markup equivalent to that of GOV.UK Frontend.

Why

Whilst anyone can easily re-use the stylesheets and JavaScript from GOV.UK Frontend, most of our users have to maintain their own abstractions of the HTML markup used for a component.

It's important for that markup to be equivalent to the markup used in GOV.UK Frontend so that it renders the same and has the same accessibility. This is especially true when those abstractions are published as packages to be re-used by others.

Further detail

Possible implementations might include:

Questions:

matthew-shaw commented 4 years ago

This would be really useful for a Jinja port I've recently started: https://github.com/LandRegistry/govuk-frontend-jinja

The code in the repo above is generated by an automated script that creates Jinja equivalent macros for each of the Nunjucks ones. This is currently run within each frontend app itself at build time. I'd rather this was an installable package from PyPi to help manage version upgrades, remove some additional complexity from the app, and have a robust set of tests to prove compatibility. Ideally, I want to render the HTML output using a single set of inputs, for both the original Nunjucks and Jinja ported macros, then diff and fail tests if they're not identical.

As you can see, not got very far with this yet so feedback/contributions very welcome!

andymantell commented 4 years ago

In govuk-react-jsx I render the nunjucks templates using the yaml example data in each component, and then do the same for the react components. I then use https://github.com/Prettyhtml/prettyhtml to smooth over minor rendering differences and https://github.com/markedjs/html-differ to diff the results. It's not the prettiest code but you can see it here:

https://github.com/surevine/govuk-react-jsx/blob/master/tests/utils/govuk-frontend-diff.js

which is then called from each component's tests:

https://github.com/surevine/govuk-react-jsx/blob/master/src/govuk/components/accordion/accordion.test.js

I then get test output like this (in this case, I have removed the role="banner" from the header to demonstrate)

image

This works well for the most part. The only issue I have encountered is cases where the example yaml data doesn't fully exercise the component. There's also a few superficial differences in the output but I managed to configure the html differ to ignore these.

Having a set of pre-rendered fixtures would certainly simplify my test suite and I could concentrate on just the diffing. Or indeed a test suite where I pass in my html and everything is handled for me.

I might have a go at abstracting out the work I did on govuk-react-jsx and see if it can help @matthew-shaw out above - as a proof of concept almost.

36degrees commented 4 years ago

@matthew-shaw have you seen https://github.com/alphagov/govuk-frontend-jinja? (maintained by a separate team at GDS, not the Design System team)

@andymantell I'm not sure you tagged the right person there!

We think the first step will be to create a proposal that we can share with the community – once we've done that we'll make sure you get the chance to feed back. These comments will be really helpful in shaping that proposal, so thank you both!

andymantell commented 4 years ago

@36degrees The govuk-frontend-jinja repo at https://github.com/alphagov/govuk-frontend-jinja repository uses a port of a script I wrote back at HM Land Registry to automatically port the Nunjucks across to Jinja using some regular expressions and other tomfoolery. I believe they've taken it a bit further than I had originally and fixed some issues I hadn't managed to. But it's similar conceptually.

Before I left HMLR though I was growing increasingly frustrated with the automated porting script and had basically concluded that it wasn't a great way to do things - there are so many subtle differences between Nunjucks and Jinja that the script became harder to maintain than just manually porting between the two! @matthew-shaw and I have chatted about this at length and he is working on the successor to this which is a set of manual ports supported by a test suite exactly as you described in this ticket. (i.e. the same model I use for govuk-react-jsx)

I think if I have a crack at packaging up my test suite it might be an interesting exercise to see how easily it can be applied to @matthew-shaw's work

Not proposing you use my code particularly, but it might shed some light on any potential issues with such a solution.

lfdebrux commented 4 years ago

@andymantell interesting, we actually found that Nunjucks and Jinja templates are similar enough that we haven't really had to worry about maintaining https://github.com/alphagov/govuk-frontend-jinja (which of course we couldn't have done without you, so thanks again!).

That said, that could be mainly because we haven't been closely following the master branch of govuk-frontend, but we've also been helped by a comprehensive test suite. Much as you and @matthew-shaw described, it's based on the YAML examples; except we copied and pasted them into our codebase :/

lfdebrux commented 4 years ago

Also, I think that @penx who created the issue #1095 already had a way of extracting and packaging the govuk-frontend exampes

https://github.com/penx/govuk-frontend-template-spec

andymantell commented 4 years ago

Oh nice. govuk-frontend-template-spec looks interesting, I'll take a look at it.

@lfdebrux Regarding the Jinja porting script - I found myself adding more and more edge cases to the conversion as each govuk-frontend release came out and it just didn't feel like an clean and enjoyable way to achieve it. I manually replicate the govuk-frontend changes into govuk-react-jsx and it's really easy just to step through the changelog when releases happen and implement them again in JSX (backed up by the tests). Doing it this way has felt much tidier (to me at least).

I reckon @matthew-shaw should pursue this solution (and I'll pitch in and help where I can) and maybe we could all have a chat at some point at compare where we've ended up. It would be good to get one published to PyPi.

I've probably derailed this thread massively. Sorry! :-)

DanScottNI commented 4 years ago

We (Department of Finance Northern Ireland) have a c# port in development of the nunjucks macros.

A way to test that the output is largely the same as the nunjucks macros would be very useful.

Definitely going to look into govuk-frontend-template-spec.

gunndabad commented 4 years ago

@DanScottNI is your C# port open source?

andymantell commented 4 years ago

I've been working on extracting the tests from govuk-react-jsx into their own cli tool this week. It's here at https://github.com/surevine/govuk-frontend-diff if anyone wants to give it a whirl.

It expects you to implement a tiny http server responding to /component/<component-name> and /template routes. You run the tool against this server and it passes the example data and expects html responses in return. It then diffs them against the reference Nunjucks implementation and passes or fails accordingly.

There's a pull request open at https://github.com/LandRegistry/govuk-frontend-jinja/pull/6 for a demo of how to integrate with it. Happy to help out with others if repositories are open source.

matthew-shaw commented 4 years ago

As @andymantell says, the govuk-frontend-jinja package is now published on PyPi. I've also deployed a little demo Flask app to Heroku with a full component specimen of every example from the GOV.UK Design System. This is now also being used internally within HM Land Registry reference/template implementations we call "skeleton apps" and I'm working on getting our service teams to adopt it.

This has already been a very worthwhile exercise. Using Andy's test suite, we now have 190 tests for every single component in every single permutation from the design system. In doing that, we've found some bugs with our original port that have now been fixed, so we already have a provably higher quality output than before. This means we can be certain of 100% markup parity and compliance with the latest, and every subsequent, GOV.UK Frontend release. This has also enabled me to remove a bunch of complexity from our skeleton frontend application, and simplify that into an open source component that the cross government community can make use of too.

The published package is currently at release 0.2.1 and I intend to leave it in a beta stage until at least the next release of GOV.UK Frontend. Once I can prove that doing an update to the latest GOV.UK release is quick and accurate I'll bump it to version 1.0.0 and mark it as production ready. In all likelihood very little will change, and having that comprehensive test suite will ensure its quality.

penx commented 4 years ago

Hi all, I made govuk-frontend-template-spec, just to let you know I've subbed to this thread and happy to help where I can. I'm not working on govuk projects at the moment, but If anyone wants to collaborate on it to make it a decent stop gap solution until there's an official solution then please ping me.

DanScottNI commented 4 years ago

@DanScottNI is your C# port open source?

Not yet. Still very early stages of development, and we're still in the early stages of open sourcing things.

gunndabad commented 4 years ago

@DanScottNI is your C# port open source?

Not yet. Still very early stages of development, and we're still in the early stages of open sourcing things.

I have an ASP.NET Core Tag Helper port of most of these components at https://github.com/gunndabad/govuk-frontend-aspnetcore that may be of interest. Still early days so (very) light on docs.

vanitabarrett commented 4 years ago

We're going to start including fixture files in the published GOV.UK Frontend package.

If you maintain your own macros, templates or partials you can use the fixtures to check that your components output the same HTML that GOV.UK Frontend uses, for example:

We'd like your feedback on this proposed change. You can:

Please provide feedback by 5pm on Tuesday 4 August.

Problem

Whilst you can easily re-use the stylesheets and JavaScript from GOV.UK Frontend, it's likely that you have to maintain the HTML markup used for each component yourself.

At the minute there's no automated way for you to tell whether your HTML is the same as the HTML that GOV.UK Frontend uses.

You may already be using the examples in the component YAML files as test fixtures, but:

Proposal

We'll make the examples provided for each component more comprehensive, so that every component option is represented by at least one example.

We'll then publish the examples in a fixtures.json file for each component, which will contain an array of objects representing each example with the following properties:

For example, in the published package you'd find govuk/components/button/fixtures.json with examples that look like this:

[
  {
    "name": "default",
    "options": {
      "text": "Save and continue"
    },
    "html": "<button class=\"govuk-button\" data-module=\"govuk-button\">\n  Save and continue\n</button>"
  },
  // […]
]

You can use these fixture files in your own tests to check that your HTML is the same as the HTML that GOV.UK Frontend uses.

For example, in pseudo-code:

fixtures = load_json_file("govuk/components/button/fixtures.json")
for fixture in fixtures
    expected_ouput = fixture.html
    actual_output = render_my_button(fixture.options)
    expect_equal(actual_output, expected_output)
end

You may need to normalise the expected and actual HTML to allow for differences in whitespace or indentation.

You can try a pre-release of GOV.UK Frontend that includes a fixtures.json file for the button component only by running:

npm install alphagov/govuk-frontend#634eeca76ee6f4399cba54cc13a11bd38f5c450a
andymantell commented 4 years ago

@vanitabarrett Are the current YAML files going to remain, or do the new fixtures.json files replace them?

Reason I ask is that I use the examples in the YAML files to build the storybook demos on govuk-react-jsx. If they're disappearing I'll do something else instead... (I.e. I'm not saying you have to keep them, just thinking ahead).

As for the fixtures themselves, it sounds perfect.

penx commented 4 years ago

@36degrees @vanitabarrett having a read through, that sounds spot on. I'll try do a React PoC at some point.

Some things to think about:

e.g. fixtures.json

{
  "button": "./govuk/components/button/fixtures.json",
  "etc.": "...etc"
}
vanitabarrett commented 4 years ago

Thank you for the comments @penx @andymantell

Some parts of the proposal were perhaps not as clear as they could be:

@penx Our aim is to make these fixtures as generic as possible. In the same way that maintainers may need to normalise the HTML to allow for differences, we expect any differences in framework behaviour to be handled by maintainers. If there’s anything we can do to make the process smoother, let us know!

The examples in GOVUK Frontend should only pass html where we specifically want to check the HTML works correctly. If you want to skip these, you could skip any tests that pass html to a component.

penx commented 4 years ago

The examples in GOVUK Frontend should only pass html where we specifically want to check the HTML works correctly

👌 if this is consistently followed then this works for me, thanks!

frankieroberto commented 4 years ago

I like this idea, and will experiment with the button example in our port.

A few questions:

Minor suggestion:

vanitabarrett commented 4 years ago

Thanks for your comment @frankieroberto !

We will generate fixtures for all components in GOVUK Frontend, so the fieldset and label will have fixtures. The fixtures are generated from the YAML files, so anything with a YAML file will get a corresponding fixtures file.

Any components that support caller should also have another option, for example: passing HTML instead. We would suggest taking that approach instead.

If you want to avoid npm, the fixtures will be on Github in a standard place for each component at: package/govuk/components/[component-name]/fixtures.json. I believe the package directory also contains a package.json which has the current version. Is that the kind of thing you were imagining?

Can you give a little more detail or an example of the backward-compatibility you mentioned?

frankieroberto commented 4 years ago

@vanitabarrett thanks for your response.

We will generate fixtures for all components in GOVUK Frontend, so the fieldset and label will have fixtures. The fixtures are generated from the YAML files, so anything with a YAML file will get a corresponding fixtures file.

Great. Would these be under the existing examples key in the YAML, or are you proposing a separate key for the test fixtures? (This doesn’t affect people using the JSON file, but it'd be good to understand what the expectations are for contributors of new components)

Any components that support caller should also have another option, for example: passing HTML instead. We would suggest taking that approach instead.

That makes sense, but the govukFieldset component currently doesn't have an html option. The caller is technically optional, but without it you can only have a fieldset with a label, not any content.

You could either add an html option to the component, or another idea would be to have a special argument name for the caller, eg __caller__.

If you want to avoid npm, the fixtures will be on Github in a standard place for each component at: package/govuk/components/[component-name]/fixtures.json. I believe the package directory also contains a package.json which has the current version. Is that the kind of thing you were imagining?

Sounds good.

Can you give a little more detail or an example of the backward-compatibility you mentioned?

If the JSON file looked like this:

{
  "examples": [ 
    {
      "name": "default",
      "options": {
        "text": "Save and continue"
      },
      "html": "<button class=\"govuk-button\" data-module=\"govuk-button\">\n  Save and continue\n</button>"
    }
  ]
}

rather than this:

[ 
  {
    "name": "default",
    "options": {
      "text": "Save and continue"
    },
    "html": "<button class=\"govuk-button\" data-module=\"govuk-button\">\n  Save and continue\n</button>"
  }
]

then that allows you to add additional top-level keys to the JSON file, without breaking any existing code which is simply iterating through the examples array:

{
  "name": "Button",
  "url": "https://design-system.service.gov.uk/components/button/",
  "version": "3.8.0",
  "deprecated": false,
  "examples": [ 
    {
      "name": "default",
      "options": {
        "text": "Save and continue"
      },
      "html": "<button class=\"govuk-button\" data-module=\"govuk-button\">\n  Save and continue\n</button>"
    }
  ]
}

(It's hard to predict whether this’ll be needed or not, but may as well allow it as a possibility. It’s also recommended by the GDS API guidance, I just discovered)

vanitabarrett commented 4 years ago

Thanks for clarifying @frankieroberto, I think your suggestions sound sensible! :+1:

Great. Would these be under the existing examples key in the YAML, or are you proposing a separate key for the test fixtures? (This doesn’t affect people using the JSON file, but it'd be good to understand what the expectations are for contributors of new components)

Yes, the fixtures files will be generated from the examples in the YAML files. We’ll need to do a bit of work to ensure the current examples are comprehensive enough before generating the first set of fixtures. I think we’ll probably review the existing documentation for contributors to see if anything needs tweaking there too.

vanitabarrett commented 4 years ago

This has been included in v3.9.0 of GOVUK Frontend 🎉