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

2753 error panel needs extending to accept two errors for one input #2817

5 changes: 5 additions & 0 deletions src/components/error/_macro-options.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
| Name | Type | Required | Description |
| ---------- | ------------------------- | -------- | ------------------------------------------------------------------ |
| id | string | true | The HTML id attribute that the error summary link can anchor to |
| text | string or array`<string>` | true | The text describing the error |
| attributes | object | false | HTML attributes (for example, data attributes) to add to the field |
rmccar marked this conversation as resolved.
Show resolved Hide resolved
21 changes: 16 additions & 5 deletions src/components/error/_macro.njk
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,30 @@

rmccar marked this conversation as resolved.
Show resolved Hide resolved
{% macro onsError(params) %}
{% set content %}
<p
class="ons-panel__error"
<ul class="ons-list ons-list--bare">
{% if params.text is not string %}
{% for item in params.text %}
<li class="ons-panel__error ons-list__item"
{% if params.attributes %}{% for attribute, value in (params.attributes.items() if params.attributes is mapping and params.attributes.items else params.attributes) %}{{ attribute }}{% if value %}="{{ value }}"{% endif %} {% endfor %}{% endif %}
>
<strong>{{ item }}</strong>
</li>
{% endfor %}
{% else %}
<li class="ons-panel__error ons-list__item"
{% if params.attributes %}{% for attribute, value in (params.attributes.items() if params.attributes is mapping and params.attributes.items else params.attributes) %}{{ attribute }}{% if value %}="{{ value }}"{% endif %} {% endfor %}{% endif %}
>
<strong>{{ params.text }}</strong>
</p>
</li>
rmccar marked this conversation as resolved.
Show resolved Hide resolved
{% endif %}
</ul>
{{ caller() }}
{% endset %}

{% call onsPanel({
"type": "error",
"id": params.id
}) %}
{{ content | safe }}
{{ content | safe }}
{% endcall %}
{% endmacro %}
{% endmacro %}
22 changes: 17 additions & 5 deletions src/components/error/_macro.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,23 @@ describe('macro: error', () => {
),
);

expect(
$('.ons-panel__error')
.text()
.trim(),
).toBe('Example error text.');
expect($('.ons-panel__error').text().trim()).toBe('Example error text.');
});

it('has the provided `text` if an array was provided', () => {
rmccar marked this conversation as resolved.
Show resolved Hide resolved
const $ = cheerio.load(
renderComponent(
'error',
{
text: ['Example error text.', 'Example error text 2.'],
id: 'example-error',
},
FAKE_NESTED_CONTENT,
),
);

expect($('.ons-panel__error').text().trim()).toContain('Example error text.');
expect($('.ons-panel__error').text().trim()).toContain('Example error text 2.');
});

it('applies the provided `attributes` to the error content paragraph', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@

{% from "components/date-input/_macro.njk" import onsDateInput %}
{% from "components/panel/_macro.njk" import onsPanel %}
{% from "components/list/_macro.njk" import onsList %}

{% call
onsPanel({
title: "There is a problem with this page",
type: "error"
})
%}

{{
onsList({
"element": "ol",
"itemsList": [
{
"text": "Enter a number between 1 to 12",
"url": "#month-error",
"variants": "inPageLink"
},
{
"text": "Enter a date that is after 1 January 2019",
"url": "#year-error",
"variants": "inPageLink"
}
rmccar marked this conversation as resolved.
Show resolved Hide resolved
]
})
}}
{% endcall %}
{{
onsDateInput({
"id": "period-from-date",
"legendOrLabel": "Period from:",
"description": "For example, 31 3 2019",
"day": {
"label": {
"text": "Day"
},
"name": "day",
"value": "31"
},
"month": {
"label": {
"text": "Month"
},
"name": "month",
"value": "13"
},
"year": {
"label": {
"text": "Year"
},
"name": "year",
"value": "2018"
}
})
}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To show the functionality I think we only need one date input and just have it ask for date of birth or something would make it simpler as the extra date input isn't really adding anything. Or could just use a single regular input with 2 problems with the answer might be even better as this isn't only going to be used on components with more than one input

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example now no longer makes sense because you have a period from but no period to.
I think an example based on this would be good as its a real eQ example of what would happen in a real questionnaire.

Screenshot 2023-09-28 at 13 47 00


{{
onsDateInput({
"id": "period-to-date",
"legendOrLabel": "Period to:",
"description": "For example, 31 3 2020",
"day": {
"label": {
"text": "Day"
},
"name": "day",
"value": "31"
},
"month": {
"label": {
"text": "Month"
},
"name": "month",
"value": "13",
"error": true
},
"year": {
"label": {
"text": "Year"
},
"name": "year",
"value": "2018",
"error": true
},
"error": {
"id": "date-input-error",
"text": [
"Enter a number between 1 to 12",
"Enter a date that is after 1 January 2019"
]
}
})
}}
Loading