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.

Sign up for GitHub

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

fix(ASSETS-17274): Visual list is not marked up as list. #275

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix(ASSETS-17274): Visual list is not marked up as list. #275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
edd5b42
change help menu markdown to ul with li
ionelc-ensemble Jan 20, 2023
9cb4919
Merge branch 'master' into issue/ASSETS-17274-help-list-items-markup
ionelc-ensemble Jan 20, 2023
5d5cd49
change apps list role=listbox to role=list and items role=option to r…
ionelc-ensemble Jan 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion coral-component-list/src/scripts/SelectList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ const SelectList = Decorator(class extends BaseComponent(HTMLElement) {

// adds the role to support accessibility
if (!this.hasAttribute('role')) {
this.setAttribute('role', 'listbox');
this.setAttribute('role', 'list');
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change should not be made to the SelectList control. The WAI-ARIA listbox role is a widget role, that will force a screen reader into forms mode, allowing the user to navigate using the arrow keys. A list is a static role defining content structure. Think about this as the difference between a <select> element and a ul or ol in HTML.

}

if (!this.hasAttribute('aria-label')) {
Expand Down
2 changes: 1 addition & 1 deletion coral-component-list/src/scripts/SelectListItem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ const SelectListItem = Decorator(class extends BaseComponent(HTMLElement) {
this.classList.add(CLASSNAME);

if (!this.hasAttribute('role')) {
this.setAttribute('role', 'option');
this.setAttribute('role', 'listitem');
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change should not be made to the SelectListItem control. The WAI-ARIA option role is a widget role, that will keep a screen reader in forms mode, allowing the user to navigate using the arrow keys. A listitem is a static role defining content structure. Think about this as the difference between an <option> element and a li in HTML.

}

// Support cloneNode
Expand Down
2 changes: 1 addition & 1 deletion coral-component-list/src/tests/test.SelectList.Item.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('SelectList.Item', function () {
describe('Instantiation', function () {
function testDefaultInstance(el) {
expect(el.classList.contains('_coral-Menu-item')).to.be.true;
expect(el.getAttribute('role')).to.equal('option');
expect(el.getAttribute('role')).to.equal('listitem');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Test for option is correct.

}

it('should be possible using new', function () {
Expand Down
2 changes: 1 addition & 1 deletion coral-component-list/src/tests/test.SelectList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('SelectList', function () {
describe('Instantiation', function () {
function testDefaultInstance(el) {
expect(el.classList.contains('_coral-Menu')).to.be.true;
expect(el.getAttribute('role')).equal('listbox');
expect(el.getAttribute('role')).equal('list');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Test for listbox is correct.

}

it('should be possible using new', function () {
Expand Down
4 changes: 3 additions & 1 deletion coral-component-shell/src/scripts/ShellHelp.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ class ShellHelp extends BaseComponent(HTMLElement) {

const selector = `#${this.id} > a[is="coral-shell-help-item"], coral-shell-help-separator`;
Array.prototype.forEach.call(this.querySelectorAll(selector), (item) => {
this._elements.items.appendChild(item);
var listItem = document.createElement('li');
listItem.appendChild(item);
this._elements.items.appendChild(listItem);
});
}

Expand Down
1 change: 1 addition & 0 deletions coral-component-shell/src/styles/help/index.styl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ $shell-loading-spacing = 16px;

._coral-Shell-help-items {
padding: 3px 0;
margin: 0;
}

._coral-Shell-help-result-item {
Expand Down
2 changes: 1 addition & 1 deletion coral-component-shell/src/templates/help.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<div handle="contentWrapper">
<label class="u-coral-screenReaderOnly" id="{{labelId}}">{{data.i18n.get('Search for Help')}}</label>
<coral-search class="_coral-Shell-help-search" handle="search" placeholder="{{data.i18n.get('Search for Help')}}" labelledby="{{labelId}}"></coral-search>
<div class="_coral-Shell-help-items" handle="items"></div>
<ul class="_coral-Shell-help-items" handle="items"></ul>
<coral-anchorlist class="_coral-Shell-help-results" handle="results" hidden></coral-anchorlist>
<div class="_coral-Shell-help-resultMessage" handle="resultMessage" role="status" hidden></div>
<div class="_coral-Shell-help-loading" handle="loading" role="status" hidden>
Expand Down