Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should label in Nunjucks be wrapped in a condition? #714

Closed
kr8n3r opened this issue May 22, 2018 · 8 comments
Closed

Should label in Nunjucks be wrapped in a condition? #714

kr8n3r opened this issue May 22, 2018 · 8 comments

Comments

@kr8n3r
Copy link

kr8n3r commented May 22, 2018

The following code

{{ govukInput({
   "id": "national-insurance-number",
   "name": "national-insurance-number",
 }) }}

will output

<div class="govuk-form-group">
  <label class="govuk-label" for="govuk-input-a">
    
  </label>
  <input class="govuk-input" id="govuk-input-a" name="national-insurance-number" type="text">
</div>

Empty label, that has a 5px bottom margin, which will effect any alignment

In the input component template we currently don't wrap label in a condition, like we do hint and error message.

@NickColley
Copy link
Contributor

Ideally not having a label input would result in a hard failure with a visible error.

If anything we could consider doing something like

.govuk-label:empty::after {
  content: "Warning: No label for input";
  color: red;
  font-weight: bold;
}

@joelanman
Copy link
Contributor

Isn't it valid to provide a label separately from the input macro? We provide a label macro

@kr8n3r
Copy link
Author

kr8n3r commented May 23, 2018

@NickColley label has whitespace so :empty won't work

@NickColley
Copy link
Contributor

One of the best things about out input component is that we make it hard to forget an associated label by default.

If we can do this without compromising that 👍

@joelanman
Copy link
Contributor

joelanman commented May 23, 2018

It is valid to provide a label separately, so I think it should be possible not have a label in this macro.

We could possibly require label: false from the user. If label is simply missing (=== undefined), it could print an error (we can just use the macro to do this directly, not CSS). However this is a point on our general approach - do we show a similar error in any other macro?

@NickColley
Copy link
Contributor

@joelanman label: false is great since it makes sure the user is making the decision.

Having general errors also sounds useful.

@igloosi was this raised based on an issue you noticed?

@joelanman
Copy link
Contributor

User-facing errors worries me a bit - on a dynamic site, a macro could theoretically be called that shows the end user an error that isn't for them - it's confusing and they can do nothing about it.

@NickColley
Copy link
Contributor

Sure, this could be a development only thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants