From 983ffefda7656273bf0f348a2cd6384c5143c74c Mon Sep 17 00:00:00 2001 From: Scott Vandehey Date: Tue, 12 Jul 2022 14:19:26 -0700 Subject: [PATCH 1/9] Add `is_sectioning` check to Card and Overview This PR updates the template for the Card component and Overview object to check if `tag_name` is a sectioning element like `article` or `section`, and then change the default for `header_tag_element` and `footer_tag_element` to either `header`/`footer` or `div`. This avoids an issue when `header` elements are rendered in a `div`, it causes useless "banner" landmarks to display in the VO Rotor. --- src/components/card/card.twig | 27 +++++++++++++++------------ src/objects/overview/overview.twig | 13 +++++++++---- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/components/card/card.twig b/src/components/card/card.twig index fb8ed64ee..fde0bba87 100644 --- a/src/components/card/card.twig +++ b/src/components/card/card.twig @@ -1,23 +1,26 @@ -{% set tag_name = tag_name|default('article') %} -{% set header_tag_name = header_tag_name|default('header') %} -{% set footer_tag_name = footer_tag_name|default('footer') %} +{% set _tag_name = tag_name|default('article') %} +{% set is_sectioning = _tag_name in ['article', 'section'] %} +{% set default_header_tag = is_sectioning ? 'header' : 'div' %} +{% set default_footer_tag = is_sectioning ? 'footer' : 'div' %} +{% set _header_tag_name = header_tag_name|default(default_header_tag) %} +{% set _footer_tag_name = footer_tag_name|default(default_footer_tag) %} -{% set heading_level = heading_level|default(2) %} +{% set _heading_level = heading_level|default(2) %} {% set _heading_block %}{% block heading %}{% endblock %}{% endset %} {% set _cover_block %}{% block cover %}{% endblock %}{% endset %} {% set _content_block %}{% block content %}{% endblock %}{% endset %} {% set _footer_block %}{% block footer %}{% endblock %}{% endset %} -<{{ tag_name }} class=" +<{{ _tag_name }} class=" c-card {% if href %}c-card--with-link{% endif %} {% if class %}{{ class }}{% endif %}" {% if heading_id and _heading_block is not empty %}aria-labelledby="{{heading_id}}"{% endif %}> {% if _heading_block is not empty %} - <{{ header_tag_name }} class="c-card__header"> - + <{{ _header_tag_name }} class="c-card__header"> + {% if href %} {{ _heading_block }} @@ -25,8 +28,8 @@ {% else %} {{ _heading_block }} {% endif %} - - + + {% endif %} {% if _cover_block is not empty %} @@ -42,9 +45,9 @@ {% endif %} {% if _footer_block is not empty %} - <{{ footer_tag_name }} class="c-card__footer"> + <{{ _footer_tag_name }} class="c-card__footer"> {{ _footer_block }} - + {% endif %} - + diff --git a/src/objects/overview/overview.twig b/src/objects/overview/overview.twig index 17426ccdd..0fddfb3fb 100644 --- a/src/objects/overview/overview.twig +++ b/src/objects/overview/overview.twig @@ -1,14 +1,19 @@ -<{{overview_tag|default('section')}} +{% set _overview_tag = overview_tag|default('section') %} +{% set is_sectioning = _overview_tag in ['article', 'section'] %} +{% set default_header_tag = is_sectioning ? 'header' : 'div' %} +{% set _header_tag_name = header_tag_name|default(default_header_tag) %} + +<{{ _overview_tag }} class="o-overview" {% if labelledby_id %}aria-labelledby="{{labelledby_id}}"{% endif %} > -
+ <{{ _header_tag_name }} class="o-overview__header"> {% block header %}{% endblock %} -
+
{% block actions %}{% endblock %}
{% block content %}{% endblock %}
- + From d4996b4c1b5524fa8f739326c9fdd29e010eaea1 Mon Sep 17 00:00:00 2001 From: Scott Vandehey Date: Tue, 12 Jul 2022 14:48:21 -0700 Subject: [PATCH 2/9] add tests --- src/components/card/card.test.ts | 57 +++++++++++++++++++++++++++ src/components/card/demo/div.twig | 22 +++++++++++ src/objects/overview/demo/div.twig | 11 ++++++ src/objects/overview/overview.test.ts | 56 ++++++++++++++++++++++++++ 4 files changed, 146 insertions(+) create mode 100644 src/components/card/card.test.ts create mode 100644 src/components/card/demo/div.twig create mode 100644 src/objects/overview/demo/div.twig create mode 100644 src/objects/overview/overview.test.ts diff --git a/src/components/card/card.test.ts b/src/components/card/card.test.ts new file mode 100644 index 000000000..14da83868 --- /dev/null +++ b/src/components/card/card.test.ts @@ -0,0 +1,57 @@ +import path from 'path'; + +import type { ElementHandle } from 'pleasantest'; +import { getAccessibilityTree, withBrowser } from 'pleasantest'; + +import { loadTwigTemplate } from '../../../test-utils.js'; + +/** Helper to load the Twig template file */ +const template = loadTwigTemplate(path.join(__dirname, './demo/single.twig')); +const divTemplate = loadTwigTemplate(path.join(__dirname, './demo/div.twig')); + +describe('Card component', () => { + test( + 'should use header/footer with article', + withBrowser(async ({ utils, page }) => { + await utils.injectHTML( + await template({ + show_heading: true, + show_footer: true, + }) + ); + + const body = await page.evaluateHandle( + () => document.body + ); + expect(await getAccessibilityTree(body)).toMatchInlineSnapshot(` + article + banner + heading "Lorem ipsum dolor sit amet" (level=2) + text "Lorem ipsum dolor sit amet" + contentinfo + text "Jul 12, 2022" + `); + }) + ); + + test( + 'should not use header/footer with div', + withBrowser(async ({ utils, page }) => { + await utils.injectHTML( + await divTemplate({ + show_heading: true, + show_footer: true, + }) + ); + + const body = await page.evaluateHandle( + () => document.body + ); + expect(await getAccessibilityTree(body)).toMatchInlineSnapshot(` + heading "Lorem ipsum dolor sit amet" (level=2) + text "Lorem ipsum dolor sit amet" + text "Jul 12, 2022" + `); + }) + ); +}); diff --git a/src/components/card/demo/div.twig b/src/components/card/demo/div.twig new file mode 100644 index 000000000..4b8b1413f --- /dev/null +++ b/src/components/card/demo/div.twig @@ -0,0 +1,22 @@ +{% embed '@cloudfour/components/card/card.twig' with { tag_name: 'div' } %} + {% block heading %} + {%- if show_heading -%} + Lorem ipsum dolor sit amet + {%- endif -%} + {% endblock %} + {% block cover %} + {%- if show_cover -%} + + {%- endif -%} + {% endblock %} + {% block content %} + {%- if show_content -%} +

Consectetur adipiscing elit. Fusce tempor ut ex nec scelerisque. Quisque dui tortor, tempus et tempor in, rhoncus eu massa. Vestibulum dolor erat, vestibulum eget velit eu, dignissim hendrerit tortor.

+ {%- endif -%} + {% endblock %} + {% block footer %} + {%- if show_footer -%} +

{{'now'|date('M j, Y')}}

+ {%- endif -%} + {% endblock %} +{% endembed %} diff --git a/src/objects/overview/demo/div.twig b/src/objects/overview/demo/div.twig new file mode 100644 index 000000000..484b25d47 --- /dev/null +++ b/src/objects/overview/demo/div.twig @@ -0,0 +1,11 @@ +{% embed '@cloudfour/objects/overview/overview.twig' with { overview_tag: 'div' } %} + {% block header %} + Header + {% endblock %} + {% block actions %} + Actions + {% endblock %} + {% block content %} + Content + {% endblock %} +{% endembed %} diff --git a/src/objects/overview/overview.test.ts b/src/objects/overview/overview.test.ts new file mode 100644 index 000000000..80ce62fde --- /dev/null +++ b/src/objects/overview/overview.test.ts @@ -0,0 +1,56 @@ +import path from 'path'; + +import type { ElementHandle } from 'pleasantest'; +import { getAccessibilityTree, withBrowser } from 'pleasantest'; + +import { loadTwigTemplate } from '../../../test-utils.js'; + +/** Helper to load the Twig template file */ +const template = loadTwigTemplate(path.join(__dirname, './demo/basic.twig')); +const divTemplate = loadTwigTemplate(path.join(__dirname, './demo/div.twig')); + +describe('Overview object', () => { + test( + 'should use header with section', + withBrowser(async ({ utils, page }) => { + await utils.injectHTML( + await template({ + show_heading: true, + show_footer: true, + }) + ); + + const body = await page.evaluateHandle( + () => document.body + ); + expect(await getAccessibilityTree(body)).toMatchInlineSnapshot(` + region + banner + text "Header" + text "Actions" + text "Content" + `); + }) + ); + + test( + 'should not use header with div', + withBrowser(async ({ utils, page }) => { + await utils.injectHTML( + await divTemplate({ + show_heading: true, + show_footer: true, + }) + ); + + const body = await page.evaluateHandle( + () => document.body + ); + expect(await getAccessibilityTree(body)).toMatchInlineSnapshot(` + text "Header" + text "Actions" + text "Content" + `); + }) + ); +}); From de56be3551f90cf718bfeaa4d13ad1f63a74c525 Mon Sep 17 00:00:00 2001 From: Scott Vandehey Date: Tue, 12 Jul 2022 14:52:07 -0700 Subject: [PATCH 3/9] Create sharp-years-think.md --- .changeset/sharp-years-think.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/sharp-years-think.md diff --git a/.changeset/sharp-years-think.md b/.changeset/sharp-years-think.md new file mode 100644 index 000000000..1b370089c --- /dev/null +++ b/.changeset/sharp-years-think.md @@ -0,0 +1,5 @@ +--- +"@cloudfour/patterns": patch +--- + +Update Card and Overview to only use `header` and `footer` elements if the containing element is an `article` or `section`. From d6e615ee8b44a81ab182844229371b9207187199 Mon Sep 17 00:00:00 2001 From: Scott Vandehey Date: Tue, 12 Jul 2022 15:59:36 -0700 Subject: [PATCH 4/9] mark properties as private --- src/components/card/card.twig | 10 +++++----- src/objects/overview/overview.twig | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/components/card/card.twig b/src/components/card/card.twig index fde0bba87..003fd06d3 100644 --- a/src/components/card/card.twig +++ b/src/components/card/card.twig @@ -1,9 +1,9 @@ {% set _tag_name = tag_name|default('article') %} -{% set is_sectioning = _tag_name in ['article', 'section'] %} -{% set default_header_tag = is_sectioning ? 'header' : 'div' %} -{% set default_footer_tag = is_sectioning ? 'footer' : 'div' %} -{% set _header_tag_name = header_tag_name|default(default_header_tag) %} -{% set _footer_tag_name = footer_tag_name|default(default_footer_tag) %} +{% set _is_sectioning = _tag_name in ['article', 'section'] %} +{% set _default_header_tag = _is_sectioning ? 'header' : 'div' %} +{% set _default_footer_tag = _is_sectioning ? 'footer' : 'div' %} +{% set _header_tag_name = header_tag_name|default(_default_header_tag) %} +{% set _footer_tag_name = footer_tag_name|default(_default_footer_tag) %} {% set _heading_level = heading_level|default(2) %} diff --git a/src/objects/overview/overview.twig b/src/objects/overview/overview.twig index 0fddfb3fb..fc75854a8 100644 --- a/src/objects/overview/overview.twig +++ b/src/objects/overview/overview.twig @@ -1,7 +1,7 @@ {% set _overview_tag = overview_tag|default('section') %} -{% set is_sectioning = _overview_tag in ['article', 'section'] %} -{% set default_header_tag = is_sectioning ? 'header' : 'div' %} -{% set _header_tag_name = header_tag_name|default(default_header_tag) %} +{% set _is_sectioning = _overview_tag in ['article', 'section'] %} +{% set _default_header_tag = _is_sectioning ? 'header' : 'div' %} +{% set _header_tag_name = header_tag_name|default(_default_header_tag) %} <{{ _overview_tag }} class="o-overview" From fef623e43733514c9a17d13f33fe914b70a28984 Mon Sep 17 00:00:00 2001 From: Scott Vandehey Date: Tue, 12 Jul 2022 16:05:13 -0700 Subject: [PATCH 5/9] add comment about test files --- src/components/card/demo/div.twig | 1 + src/objects/overview/demo/div.twig | 1 + 2 files changed, 2 insertions(+) diff --git a/src/components/card/demo/div.twig b/src/components/card/demo/div.twig index 4b8b1413f..2ce2b4a65 100644 --- a/src/components/card/demo/div.twig +++ b/src/components/card/demo/div.twig @@ -1,3 +1,4 @@ +{# Used for tests #} {% embed '@cloudfour/components/card/card.twig' with { tag_name: 'div' } %} {% block heading %} {%- if show_heading -%} diff --git a/src/objects/overview/demo/div.twig b/src/objects/overview/demo/div.twig index 484b25d47..f5ad0bdb0 100644 --- a/src/objects/overview/demo/div.twig +++ b/src/objects/overview/demo/div.twig @@ -1,3 +1,4 @@ +{# Used for tests #} {% embed '@cloudfour/objects/overview/overview.twig' with { overview_tag: 'div' } %} {% block header %} Header From 0d0f3e4c2c594fb0c19f3e70ab4e7526d0bbc8be Mon Sep 17 00:00:00 2001 From: Scott Vandehey Date: Tue, 12 Jul 2022 16:08:19 -0700 Subject: [PATCH 6/9] Update src/components/card/card.test.ts Co-authored-by: Paul Hebert --- src/components/card/card.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/components/card/card.test.ts b/src/components/card/card.test.ts index 14da83868..a447bc180 100644 --- a/src/components/card/card.test.ts +++ b/src/components/card/card.test.ts @@ -23,13 +23,11 @@ describe('Card component', () => { const body = await page.evaluateHandle( () => document.body ); - expect(await getAccessibilityTree(body)).toMatchInlineSnapshot(` + expect(await getAccessibilityTree(body, {includeText: false})).toMatchInlineSnapshot(` article banner heading "Lorem ipsum dolor sit amet" (level=2) - text "Lorem ipsum dolor sit amet" contentinfo - text "Jul 12, 2022" `); }) ); From 1941aaff3e2dca5e081708ce7cfdf7f55d864d95 Mon Sep 17 00:00:00 2001 From: Scott Vandehey Date: Tue, 12 Jul 2022 16:08:29 -0700 Subject: [PATCH 7/9] Update src/components/card/card.test.ts Co-authored-by: Paul Hebert --- src/components/card/card.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/components/card/card.test.ts b/src/components/card/card.test.ts index a447bc180..d189cecd4 100644 --- a/src/components/card/card.test.ts +++ b/src/components/card/card.test.ts @@ -45,10 +45,8 @@ describe('Card component', () => { const body = await page.evaluateHandle( () => document.body ); - expect(await getAccessibilityTree(body)).toMatchInlineSnapshot(` + expect(await getAccessibilityTree(body, {includeText: false})).toMatchInlineSnapshot(` heading "Lorem ipsum dolor sit amet" (level=2) - text "Lorem ipsum dolor sit amet" - text "Jul 12, 2022" `); }) ); From d3edd76a993c02ae860e9f8a59da028e22514be9 Mon Sep 17 00:00:00 2001 From: Scott Vandehey Date: Tue, 12 Jul 2022 16:11:04 -0700 Subject: [PATCH 8/9] add comments --- src/components/card/card.twig | 5 +++++ src/objects/overview/overview.twig | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/components/card/card.twig b/src/components/card/card.twig index 003fd06d3..ec5720c70 100644 --- a/src/components/card/card.twig +++ b/src/components/card/card.twig @@ -1,4 +1,9 @@ {% set _tag_name = tag_name|default('article') %} +{# + Using `header` inside a `div` causes pointless "banner" landmarks in + the VoiceOver rotor. As a result, we set the default header/footer + element to `div` if the `tag_name` is anything but `article` or `section`. +#} {% set _is_sectioning = _tag_name in ['article', 'section'] %} {% set _default_header_tag = _is_sectioning ? 'header' : 'div' %} {% set _default_footer_tag = _is_sectioning ? 'footer' : 'div' %} diff --git a/src/objects/overview/overview.twig b/src/objects/overview/overview.twig index fc75854a8..6ff10e621 100644 --- a/src/objects/overview/overview.twig +++ b/src/objects/overview/overview.twig @@ -1,4 +1,9 @@ {% set _overview_tag = overview_tag|default('section') %} +{# + Using `header` inside a `div` causes pointless "banner" landmarks in + the VoiceOver rotor. As a result, we set the default header element + to `div` if the `overview_tag` is anything but `article` or `section`. +#} {% set _is_sectioning = _overview_tag in ['article', 'section'] %} {% set _default_header_tag = _is_sectioning ? 'header' : 'div' %} {% set _header_tag_name = header_tag_name|default(_default_header_tag) %} From 7b02ae211b483fc14045228029b8840a3260bc87 Mon Sep 17 00:00:00 2001 From: Scott Vandehey Date: Tue, 12 Jul 2022 16:13:21 -0700 Subject: [PATCH 9/9] lint --- src/components/card/card.test.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/components/card/card.test.ts b/src/components/card/card.test.ts index d189cecd4..9eb4fa7d4 100644 --- a/src/components/card/card.test.ts +++ b/src/components/card/card.test.ts @@ -23,12 +23,13 @@ describe('Card component', () => { const body = await page.evaluateHandle( () => document.body ); - expect(await getAccessibilityTree(body, {includeText: false})).toMatchInlineSnapshot(` - article - banner - heading "Lorem ipsum dolor sit amet" (level=2) - contentinfo - `); + expect(await getAccessibilityTree(body, { includeText: false })) + .toMatchInlineSnapshot(` + article + banner + heading "Lorem ipsum dolor sit amet" (level=2) + contentinfo + `); }) ); @@ -45,9 +46,9 @@ describe('Card component', () => { const body = await page.evaluateHandle( () => document.body ); - expect(await getAccessibilityTree(body, {includeText: false})).toMatchInlineSnapshot(` - heading "Lorem ipsum dolor sit amet" (level=2) - `); + expect( + await getAccessibilityTree(body, { includeText: false }) + ).toMatchInlineSnapshot(`heading "Lorem ipsum dolor sit amet" (level=2)`); }) ); });