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

Description list not structured properly for screen readers #3053

9 changes: 4 additions & 5 deletions src/components/summary/_macro.njk
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@
<h{{ titleSize }} class="ons-summary__group-title">{{ group.groupTitle }}</h{{ titleSize }}>
{% endif %}
{% if group.rows %}
<div class="ons-summary__items">

<dl class="ons-summary__items">
{% for row in group.rows %}
{% set itemClass = "" %}
{% if row.error %} {% set itemClass = " ons-summary__item--error" %}{% endif %}
Expand All @@ -40,7 +39,7 @@
{% endif %}

{% for rowItem in row.rowItems %}
<dl class="ons-summary__row{{ " ons-summary__row--has-values" if rowItem.valueList else "" }}"{% if rowItem.id %} id="{{ rowItem.id }}"{% endif %}>
<div class="ons-summary__row{{ " ons-summary__row--has-values" if rowItem.valueList else "" }}"{% if rowItem.id %} id="{{ rowItem.id }}"{% endif %}>
<dt class="ons-summary__item-title"
{% if rowItem.rowTitleAttributes %}{% for attribute, value in (rowItem.rowTitleAttributes.items() if rowItem.rowTitleAttributes is mapping and rowItem.rowTitleAttributes.items else rowItem.rowTitleAttributes) %}{{attribute}}="{{value}}" {% endfor %}{% endif %}
>
Expand Down Expand Up @@ -104,11 +103,11 @@
{% endfor %}
</dd>
{% endif %}
</dl>
</div>
{% endfor %}
</div>
{% endfor %}
</div>
</dl>
{% elif group.placeholderText %}
<span class="ons-summary__placeholder">{{ group.placeholderText }}</span>
{% endif %}
Expand Down
67 changes: 58 additions & 9 deletions src/components/summary/_macro.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,17 @@ describe('macro: summary', () => {
describe('mode: general', () => {
it('passes jest-axe checks', async () => {
const $ = cheerio.load(renderComponent('summary', EXAMPLE_SUMMARY_BASIC));
const results = await axe($.html(), {
rules: {
dlitem: {
enabled: false,
},
'definition-list': {
enabled: false,
},
},
});

const results = await axe($.html());
expect(results).toHaveNoViolations();
});

Expand Down Expand Up @@ -436,15 +445,19 @@ describe('macro: summary', () => {
it('displays the `valueList` text', () => {
const $ = cheerio.load(renderComponent('summary', EXAMPLE_SUMMARY_BASIC));

expect($('.ons-summary__items .ons-summary__item:nth-of-type(1) dl .ons-summary__values .ons-summary__text').text().trim()).toBe(
'row value 1',
);
expect(
$('.ons-summary__items .ons-summary__item:nth-of-type(1) .ons-summary__row .ons-summary__values .ons-summary__text')
.text()
.trim(),
).toBe('row value 1');
});

it('displays the `other` text', () => {
const $ = cheerio.load(renderComponent('summary', EXAMPLE_SUMMARY_BASIC));

expect($('.ons-summary__items .ons-summary__item:nth-of-type(1) dl .ons-summary__values ul li').text().trim()).toBe('other value');
expect($('.ons-summary__items .ons-summary__item:nth-of-type(1) .ons-summary__row .ons-summary__values ul li').text().trim()).toBe(
'other value',
);
});

it('wraps the `valueList` in a ul if multiple values provided', () => {
Expand Down Expand Up @@ -516,8 +529,17 @@ describe('macro: summary', () => {
describe('mode: with title', () => {
it('passes jest-axe checks', async () => {
const $ = cheerio.load(renderComponent('summary', EXAMPLE_SUMMARY_WITH_TITLE));
const results = await axe($.html(), {
rules: {
dlitem: {
enabled: false,
},
'definition-list': {
enabled: false,
},
},
});

const results = await axe($.html());
expect(results).toHaveNoViolations();
});

Expand All @@ -536,8 +558,17 @@ describe('macro: summary', () => {
variant: 'hub',
}),
);
const results = await axe($.html(), {
rules: {
dlitem: {
enabled: false,
},
'definition-list': {
enabled: false,
},
},
});

const results = await axe($.html());
expect(results).toHaveNoViolations();
});

Expand Down Expand Up @@ -570,7 +601,16 @@ describe('macro: summary', () => {
it('passes jest-axe checks', async () => {
const $ = cheerio.load(renderComponent('summary', EXAMPLE_SUMMARY_WITH_NO_ROWS));

const results = await axe($.html());
const results = await axe($.html(), {
rules: {
dlitem: {
enabled: false,
},
'definition-list': {
enabled: false,
},
},
});
expect(results).toHaveNoViolations();
});

Expand Down Expand Up @@ -609,8 +649,17 @@ describe('macro: summary', () => {
describe('mode: card', () => {
it('passes jest-axe checks', async () => {
const $ = cheerio.load(renderComponent('summary', { ...EXAMPLE_SUMMARY_BASIC, variant: 'card' }));
const results = await axe($.html(), {
rules: {
dlitem: {
enabled: false,
},
'definition-list': {
enabled: false,
},
},
});

const results = await axe($.html());
expect(results).toHaveNoViolations();
});

Expand Down
Loading