Fix format inconsistencies #830
Conversation
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-review-pr-830
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-review-pr-830
alex-ju
force-pushed
the
fix-format-inconsistencies
branch
from
751fdee
to
074bfa0
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-review-pr-830
CHANGELOG.md
Outdated
| ([PR 824](https://github.com/alphagov/govuk-frontend/pull/824)) | ||
| ([PR #824](https://github.com/alphagov/govuk-frontend/pull/824)) | ||
|
|
||
| - Fix YAML and Nunjucks format inconsistencies |
There was a problem hiding this comment.
Is this a fix? It doesnt look like any of the api has changed? I'm not sure
There was a problem hiding this comment.
Mmm.. no API changes, indeed. More of an internal, isn't it?
alex-ju
force-pushed
the
fix-format-inconsistencies
branch
from
074bfa0
to
7be5f0f
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-review-pr-830
| }) | ||
| }} | ||
| html: 'It can take up to 8 weeks <a href="#">to register</a> a lasting power of attorney if there are no mistakes in the application.' | ||
| }) }} |
There was a problem hiding this comment.
This should have double quotes as well
There was a problem hiding this comment.
That'd then require escaping the double quoted href attribute – personally I'd prefer to use single quotes rather than escaping in those scenarios, but that's a loosely held preference 🙂
There was a problem hiding this comment.
I think in this specific circumstance single quotes is reasonable given we want to keep the HTML the same as elsewhere.
| }, | ||
| "html": 'This is a new service – your <a href="#" class="govuk-link">feedback</a> will help us to improve it.' | ||
| html: 'This is a new service – your <a href="#" class="govuk-link">feedback</a> will help us to improve it.' |
There was a problem hiding this comment.
This should have double quotes
app/views/partials/showExamples.njk
Outdated
| @@ -13,13 +13,13 @@ | |||
| {% set previewLink = '/components/' + componentName + "/preview" %} | |||
| {% set previewText = 'Preview the ' + componentName + ' component' %} | |||
| {% else %} | |||
| {% set itemName = componentName + '--' + item.name %} | |||
| {% set itemName = (componentName + ' ' + item.name) | replace("-", " ") %} | |||
There was a problem hiding this comment.
Slightly outside the bounds of this PR but it would be really good to have a comment here to say why item.name has dashes which are then taken out here
There was a problem hiding this comment.
Could we go through and remove the dashes from the example names instead?
There was a problem hiding this comment.
This would also then allow for dashes to still be used where appropriate – e.g. 'Label with govuk-label--xl modifier' which would otherwise become 'Label with govuk label xl modifier' with this.
There was a problem hiding this comment.
The dashes from the example names are used to map the express routes.
app.get('/components/:component/:example*?/preview', function (req, res, next) {
But if we're fine with having urls with %20 we can get rid of the dashes and use spaces instead.
There was a problem hiding this comment.
Updated so the example names are defined in YAML without dashes and are not affected by formatting (we can have dashes - Input with width-10 class, uppercases - Footer GOV.UK or any characters) while preserving nice urls.
alex-ju
force-pushed
the
fix-format-inconsistencies
branch
from
7be5f0f
to
69eceb0
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-review-pr-830
alex-ju
force-pushed
the
fix-format-inconsistencies
branch
from
69eceb0
to
d9f1cd8
Compare
This:
Trello card