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

Add error param to input #1150

Closed
penx opened this issue Jan 21, 2019 · 4 comments
Closed

Add error param to input #1150

penx opened this issue Jan 21, 2019 · 4 comments
Labels
feature request User requests a new feature 🕔 hours A well understood issue which we expect to take less than a day to resolve.

Comments

@penx
Copy link

penx commented Jan 21, 2019

At the moment, date-input passes a CSS class to input when it wants to mark a field as having an error:

classes: "govuk-date-input__input " + (item.classes if item.classes),

name: day
classes: govuk-input--width-2 govuk-input--error

This implies that the user of the date-input has inferred knowledge of the inner workings (CSS class name) of input.

I propose that an 'error' param is added to input so that, when using the input template, the user (be it an application using the input template or another component) doesn't need to have inferred knowledge of the class names. The classes param would still exist so this approach would also still work, but the preferred approach would be to use the error param.

This would mean adding an error param to input as follows:

before:

  <input class="govuk-input {%- if params.classes %} {{ params.classes }}{% endif %} {%- if params.errorMessage %} govuk-input--error{% endif %}" id="{{ params.id }}" name="{{ params.name }}" type="{{ params.type | default('text') }}"

after:

  <input class="govuk-input {%- if params.classes %} {{ params.classes }}{% endif %} {%- if params.errorMessage | params.error %} govuk-input--error{% endif %}" id="{{ params.id }}" name="{{ params.name }}" type="{{ params.type | default('text') }}"

And changing the date-input examples as follows:

before:

        name: day
        classes: govuk-input--width-2 govuk-input--error

after:

        name: day
        classes: govuk-input--width-2
        error: true

This may seem like a trivial issue for users of the nunjucks templates - but when working on ports (e.g. using CSSinJS or CSS modules) it is useful for one component/template to not have to have knowledge of the inner workings of another.

@NickColley
Copy link
Contributor

I believe this is the same as this issue we closed: #460

There has been some recent discussion internally about this since the GOV.UK Publishing team is very keen on avoiding using classes for this same reason.

@penx
Copy link
Author

penx commented Jan 21, 2019

Yep looks the same. Can #460 be reopened?

@hannalaakso hannalaakso added awaiting triage Needs triaging by team feature request User requests a new feature labels Jan 21, 2019
@kellylee-gds kellylee-gds added 🕔 hours A well understood issue which we expect to take less than a day to resolve. Priority: Low and removed awaiting triage Needs triaging by team labels Jan 23, 2019
@aliuk2012
Copy link
Contributor

Closing this as it is part of #460

@penx
Copy link
Author

penx commented Dec 1, 2021

Can this be reopened as it doesn't seem to have been addressed in #460?

(as per my comment, for this to be resolved, the fixtures should not contain CSS classes)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request User requests a new feature 🕔 hours A well understood issue which we expect to take less than a day to resolve.
Projects
None yet
Development

No branches or pull requests

5 participants