Menu button pattern and 4 examples: Set aria-expanded to false when menus are closed

[andreancardona]

Closes #697 and #2834 with the following changes:

• Revise menu button pattern to specify that aria-expanded is set to false when the menu is not displayed.
• Revise JS and documentation for 3 menu button examples and the toolbar example to set aria-expanded false when their menus are closed.
• Adjusts corresponding regression tests to ensure the state of expanded is true when the menus are open and false when the menus are closed.

Preview links

• Menu Button Pattern
• Actions Menu Button Example Using aria-activedescendant
• Actions Menu Button Example Using element.focus()
• Navigation Menu Button Example
• Toolbar Example

Review checklist

Reviewers: To learn what needs to be covered by each review, Follow the link for the type of review to which you are assigned.

• Editorial review by @mcking65
• Functional review by @mcking65
• N/A: Visual design review by reviewer_id
• N/A: Accessibility review by reviewer_id
• Code review by @jongund
• Test review by @a11ydoer

---

WAI Preview Link (Last built on Mon, 11 Dec 2023 15:40:06 GMT).

[mcking65]

@andreancardona

I don't see any javascript changes. The menubuttons do not have aria-expanded set to false when the menus are not visible.

[css-meeting-bot]

The ARIA Authoring Practices (APG) Task Force just discussed PR 2839 - set aria-expanded false when menus are closed.

The full IRC log of that discussion

<jugglinmike> Topic: PR 2839 - set aria-expanded false when menus are closed
<jugglinmike> github: https://github.com//pull/2839
<jugglinmike> arigilmore: I worked on this with Andrea
<jugglinmike> arigilmore: Oh, wait, no. Andrea worked on this alone
<jugglinmike> Matt_King: This is updating JavaScript, documentation, and tests
<jugglinmike> Matt_King: I don't see any JavaScript changes here
<jugglinmike> arigilmore: I'll let Andrea know and see if she needs any help working on it
<jugglinmike> Matt_King: Probably the more complicated part of this fix will be writing a test
<jugglinmike> Matt_King: It at least won't be starting from scratch--there is already a test for setting aria-expanded to "true"
<jugglinmike> Jem: We eventually need a reviewer...
<jugglinmike> Matt_King: Let's wait until we have the patch closer to ready

[a11ydoer]

Regarding the test review, below example tests are missing for menu-button

• menu-button/examples/menu-button-actions-active-descendant.html:
  • menu-up-arrow
  • menu-down-arrow
  • menu-home
  • menu-character
• menu-button/examples/menu-button-links.html:
  • menu-enter

[mcking65]

@andreancardona

It appears we still need additional changes.

content/patterns/toolbar/examples/js/FontMenuButton.js is still removing aria-expanded=false.

The tests are still testing to ensure false is not present when the menu is closed.For example, see line 88 in
test/tests/menu-button_actions-active-descendant.js

Corresponding changes are needed in:
test/tests/menu-button_actions.js
test/tests/menu-button_links.js
test/tests/toolbar_toolbar.js

[css-meeting-bot]

The ARIA Authoring Practices (APG) Task Force just discussed PR 2839 - set aria-expanded false when menus are closed.

The full IRC log of that discussion

<jugglinmike> Topic: PR 2839 - set aria-expanded false when menus are closed
<jugglinmike> github: https://github.com//pull/2839
<jugglinmike> Matt_King: I did a bunch of work on this last night. I found two areas of concern
<jugglinmike> Matt_King: First, the documentation is changed for toolbar, but not the actual script
<jugglinmike> Jem: I'll take a look at the example you shared
<jugglinmike> s/Jem:/Andrea_Cardona:/
<jugglinmike> Matt_King: Second is the tests
<jongund> q+
<jugglinmike> Matt_King: The test message is exactly wrong. It's saying that it's testing to make sure the value is "false"
<jugglinmike> Matt_King: The tests need to be updated, but don't forget to update the test message as well
<jugglinmike> Jem: We're missing a bunch of keyboard tests
<jugglinmike> Matt_King: We should address that in a separate pull request. In this pull request, we should focus on the change for aria-expanded

[andreancardona]

@jongund hi Jon! I was hoping you might be able to help me with the current failing tests. I'm not sure how to debug / fix

Thank you!

[jongund]

@andreancardona

The following 2 tests should replace the current toolbar-menubutton-aria-expanded test case.

1. There is a new separate test for the toolbar-menubutton-aria-expanded-false case.
2. The toolbar-menubutton-aria-expanded case is a slightly modified version of what you currently have, it removes the test for aria-expanded attribute does not exist and then tests for 'true' when the menu is opened after the click function.

NOTE: You may also want to adjust the test ID from "toolbar-menubutton-aria-expanded" to "toolbar-menubutton-aria-expanded-true" in content/patterns/toolbar/examples/js/FontMenu.js and test/tests/toolbar_toolbar.js for the case when aria-expanded is true to be consistent with the false ID (e.g. toolbar-menubutton-aria-expanded-false).

Let me know if this helps,
Jon

ariaTest(
  'Font family button has aria-expanded=false',
  exampleFile,
  'toolbar-menubutton-aria-expanded-false',
  async (t) => {
    await assertAttributeValues(
      t,
      ex.fontFamilyButtonSelector,
      'aria-expanded',
      'false'
    );
  }
);

ariaTest(
  'Font family button has aria-expanded=true',
  exampleFile,
  'toolbar-menubutton-aria-expanded',
  async (t) => {
    await (
      await t.context.session.findElement(By.css(ex.fontFamilyButtonSelector))
    ).click();

    await assertAttributeValues(
      t,
      ex.fontFamilyButtonSelector,
      'aria-expanded',
      'true'
    );
  }
);

[andreancardona]

when aria-expanded is true to be consist

@jongund just implemented those changes - thank you so much!

Current failing test does not have to do with this PR :/

[jongund]

Thanks for making the updates Andrea, the changes look good to me.

[mcking65]

Editorial and functional reviews are complete and look good.

[mcking65]

Thank you @andreancardona for all your work on this update!

Thank you also to @jongund for the excellent support to help get this over the line.