Combobox examples with listbox popup: Add properties and states to listbox control button #1336

jongund · 2020-02-16T21:52:42Z

Fixes #1333 and #1331:

Updates button to include aria attributes identified in issue Combobox examples: Mark up open button so it reflects combobox state #1333 and issue Combobox Examples: In high contrast mode the drop down arrow disappears when the field is selected #1331
Adds regression test for the newly added attributes.

Preview link

View the combobox examples in the compare branch with RawGitHack

Review checklist

Editorial review by Matt (@mcking65)
Functional review by Matt (@mcking65)
Visual design review by Siri
Accessibility review by Matt (@mcking65)
Code review by Sarah (@smhigley) or Simon (@zcorpan)
Test review by Sarah (@smhigley) or Simon (@zcorpan)

…es listbox

mcking65

Thanks for this @jongund!! I made some suggestions for the button labels.

examples/combobox/combobox-autocomplete-both.html

mcking65 · 2020-02-17T03:15:44Z

examples/combobox/combobox-autocomplete-list.html

@@ -53,7 +53,7 @@ <h2 id="ex_label">Example</h2>
          <div class="group">
            <input id="cb1-input" class="cb_edit" type="text" role="combobox" aria-autocomplete="list"
              aria-expanded="false" aria-controls="cb1-listbox">
-            <button id="cb1-button" tabindex="-1" aria-label="Open">
+            <button id="cb1-button" tabindex="-1"  aria-label="State" aria-expanded="false" aria-controls="cb1-listbox">


Same recommendation as above:

Suggested change

<button id="cb1-button" tabindex="-1" aria-label="State" aria-expanded="false" aria-controls="cb1-listbox">

<button id="cb1-button" tabindex="-1" aria-label="States" aria-expanded="false" aria-controls="cb1-listbox">

examples/combobox/combobox-autocomplete-none.html

mcking65

Looks very good from editorial perspective; as the button is not actually part of the combobox, I agree that the table about it should be last. I made 3 comments regarding the need to document use of aria-label.

examples/combobox/combobox-autocomplete-both.html

examples/combobox/combobox-autocomplete-list.html

examples/combobox/combobox-autocomplete-none.html

jongund · 2020-02-17T15:56:40Z

@mcking65

I updated the pull request with your suggestions and also added a row for tabindex=-``. I also added a section in the keyboard support section saying that the button` is being removed from tab sequence of the page, but is still important for assistive technologies that use touch events to open the listbox.

jongund · 2020-02-17T17:30:44Z

@mcking65

I updated this pull request to also address issue #1331 high contrast mode.

zcorpan · 2020-03-03T15:34:10Z

Visual design review: the relevant change is about Windows high contrast, reviewed in #1331 (comment)

zcorpan · 2020-03-03T16:32:36Z

Maybe #1343 should be merged first, and this be rebased on top and remove the t.plan(1)s.

zcorpan

Formatting nits in a first pass of code review

examples/combobox/combobox-autocomplete-list.html

test/tests/combobox_autocomplete-both.js

test/tests/combobox_autocomplete-none.js

examples/combobox/combobox-autocomplete-none.html

zcorpan · 2020-03-03T20:39:09Z

Maybe #1343 should be merged first, and this be rebased on top and remove the t.plan(1)s.

Never mind; I think that PR should add a lint check, which means other PRs like this one don't need to be blocked on it.

zcorpan · 2020-03-03T20:46:23Z

npm run regression-report gives

combobox/combobox-autocomplete-both.html:
    combobox-id
combobox/combobox-autocomplete-list.html:
    combobox-id
combobox/combobox-autocomplete-none.html:
    combobox-id

I think those failures were introduced by #1276. I can report a new issue about this.

zcorpan

Code review and test review done. This looks OK except for formatting nits above.

I can apply them and merge.

zcorpan · 2020-03-04T13:56:41Z

Actually I can't merge, I'm not authorized to push to protected branches.

Combobox examples with listbox popup: Add expanded and controls to popup control button and make visible in high contrast (pull #1336) Fixes issues #1333 and #1331: * added new name, aria-expanded, and aria-controls to popup control button. * Added regression test for new aria properties on popup control button. * Added documentation for attributes on popup control button. * updated css to support Windows high contrast mode. Co-authored-by: Matt King <[email protected]> Co-authored-by: Simon Pieters <[email protected]>

Combobox examples with listbox popup: Add expanded and controls to popup control button and make visible in high contrast (pull w3c#1336) Fixes issues w3c#1333 and w3c#1331: * added new name, aria-expanded, and aria-controls to popup control button. * Added regression test for new aria properties on popup control button. * Added documentation for attributes on popup control button. * updated css to support Windows high contrast mode. Co-authored-by: Matt King <[email protected]> Co-authored-by: Simon Pieters <[email protected]>

…pup control button and make visible in high contrast (pull w3c#1336) Fixes issues w3c#1333 and w3c#1331: * added new name, aria-expanded, and aria-controls to popup control button. * Added regression test for new aria properties on popup control button. * Added documentation for attributes on popup control button. * updated css to support Windows high contrast mode. Co-authored-by: Matt King <[email protected]> Co-authored-by: Simon Pieters <[email protected]>

jongund added 2 commits February 16, 2020 15:21

added aria properties and correct label to button that opens anc clos…

75777c9

…es listbox

updated regression test for new aria properties on button

a40c168

jongund mentioned this pull request Feb 16, 2020

Combobox examples: Mark up open button so it reflects combobox state #1333

Closed

mcking65 changed the title ~~Issue1333 add aria to button~~ Combobox examples with listbox popup: Add properties and states to listbox control button Feb 17, 2020

mcking65 requested changes Feb 17, 2020

View reviewed changes

examples/combobox/combobox-autocomplete-both.html Show resolved Hide resolved

examples/combobox/combobox-autocomplete-list.html Show resolved Hide resolved

examples/combobox/combobox-autocomplete-none.html Show resolved Hide resolved

updated combobox examples based Matt's feedback

c862f31

jongund added 3 commits February 17, 2020 10:02

updated documentation for button

a4d4ce0

fixed unclosed section bug

c0dfc7f

updated css for high contrast support

431de15

jongund and others added 2 commits February 17, 2020 11:57

updated css

89a7833

Minor editorial revision to tabindex documentation for the button

8b213b5

mcking65 approved these changes Feb 28, 2020

View reviewed changes

zcorpan self-assigned this Feb 28, 2020

zcorpan mentioned this pull request Mar 3, 2020

Combobox Examples: In high contrast mode the drop down arrow disappears when the field is selected #1331

Closed

zcorpan reviewed Mar 3, 2020

View reviewed changes

zcorpan mentioned this pull request Mar 3, 2020

combobox examples rows "combobox-id" are missing tests #1347

Closed

zcorpan approved these changes Mar 3, 2020

View reviewed changes

Apply suggestions from code review

1bc542e

Merge branch 'master' into issue1333-add-aria-to-button

78d88a6

mcking65 linked an issue Mar 8, 2020 that may be closed by this pull request

Combobox Examples: In high contrast mode the drop down arrow disappears when the field is selected #1331

Closed

mcking65 merged commit 941af00 into master Mar 9, 2020

mcking65 deleted the issue1333-add-aria-to-button branch March 9, 2020 02:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Combobox examples with listbox popup: Add properties and states to listbox control button #1336

Combobox examples with listbox popup: Add properties and states to listbox control button #1336

jongund commented Feb 16, 2020 •

edited by a11ydoer

Loading

mcking65 left a comment

mcking65 Feb 17, 2020

jongund Feb 17, 2020

mcking65 left a comment

jongund commented Feb 17, 2020

jongund commented Feb 17, 2020

zcorpan commented Mar 3, 2020

zcorpan commented Mar 3, 2020

zcorpan left a comment

zcorpan commented Mar 3, 2020

zcorpan commented Mar 3, 2020

zcorpan left a comment

zcorpan commented Mar 4, 2020

	<button id="cb1-button" tabindex="-1" aria-label="State" aria-expanded="false" aria-controls="cb1-listbox">
	<button id="cb1-button" tabindex="-1" aria-label="States" aria-expanded="false" aria-controls="cb1-listbox">

Combobox examples with listbox popup: Add properties and states to listbox control button #1336

Combobox examples with listbox popup: Add properties and states to listbox control button #1336

Conversation

jongund commented Feb 16, 2020 • edited by a11ydoer Loading

Preview link

Review checklist

mcking65 left a comment

Choose a reason for hiding this comment

mcking65 Feb 17, 2020

Choose a reason for hiding this comment

jongund Feb 17, 2020

Choose a reason for hiding this comment

mcking65 left a comment

Choose a reason for hiding this comment

jongund commented Feb 17, 2020

jongund commented Feb 17, 2020

zcorpan commented Mar 3, 2020

zcorpan commented Mar 3, 2020

zcorpan left a comment

Choose a reason for hiding this comment

zcorpan commented Mar 3, 2020

zcorpan commented Mar 3, 2020

zcorpan left a comment

Choose a reason for hiding this comment

zcorpan commented Mar 4, 2020

jongund commented Feb 16, 2020 •

edited by a11ydoer

Loading