Add CodePen button to menubar-editor and avoid external SVG

[zcorpan]

The approach taken here with Unicode symbols in CSS generated content is different from what menubar-navigation.html does (which uses inline SVG, without any ARIA attributes). It's further possible to use SVG-in-CSS by using data: URLs, as menubar-navigation.css did for the separator (but this patch changes it to a CSS linear-gradient to be consistent with menubar-editor.css).

What are the accessibility implications of using Unicode symbols in CSS generated content? Is it good, acceptable, or bad?

Preview editor menubar in compare branch in raw githack

---

Preview | Diff

[jongund]

@zcorpan
This example needs to be updated to use the APG coding practices and support high contrast modes of operating systems. Do you want me to just make all the changes in a different branch, rather than making piecemeal changes to the example?

[zcorpan]

@jongund that seems separate to this change, so I would prefer a separate branch. Thanks!

[zcorpan]

The regression test had a failure here which I could reproduce locally, but haven't yet investigated why it happens.

[jongund]

@zcorpan
When this happens to me I just create a new branch from main, update the files I changed into the new branch.
This seems to fix the problem most of the time.
Seem to do with getting to out-of-date with main, but not sure.

[carmacleod]

raw.githack doesn't seem to be working at the moment, but the following link should be ok for preview:
https://raw.githack.com/w3c/aria-practices/bocoup/menubar-editor-codepen/examples/menubar/menubar-editor.html

[a11ydoer]

@zcorpan
I don't see any unicode in css you mentioned this issue. I think you are asking about CSS pseudo element of "after" Is that correct? My understadning is that CSS pseudo element is not accessible by screen readers. Are you hoping for APG group to come up with different coding for "expand"(down arrow) icon for the accessibility?

.disclosure-nav button::after {
  content: "";
  border-bottom: 1px solid #000;
  border-right: 1px solid #000;
  height: 0.5em;
  margin-left: 0.75em;
  width: 0.5em;
  transform: rotate(45deg);

[a11ydoer]

Here is unicode accessiblity info by WCAG Working group.

"Description
The objective of this technique is to show how to provide a visible, text alternative for an Unicode character that conveys information.

Some versions of assistive technologies will announce CSS generated content, as well as specific Unicode characters. However, because the announcement may be inaccurate aria-hidden="true" is used so it will be ignored by assistive technologies. An on-screen text alternative is added."

[a11ydoer]

Now I see unicode in Editor menu bar example from File changes.

@charset "utf-8";

.menubar-editor [role="menubar"] > li > [role="menuitem"]::after {
content: "▼";
padding-left: 0.25rem;
font-size: 11px;
vertical-align: middle;

[zcorpan]

Right.

What was there before was an image in ::after generated content, which doesn't seem more accessible than the character "▼". But maybe neither of these techniques are ideal?

The accessible name could be set with aria-label to something else, or maybe it could be removed with aria-hidden="true" if the other ARIA attributes already provide enough information and the triangle is only there to help visual users. I'm not sure what is best here.

[jongund]

Changes:

1. Update the code to use the APG JS coding practices:
   • Use class instead of prototype
   • Event handlers should be named "onXXX'
   • Use event.key, instead of event.keyCode
   • Use pointer events instead of mouse events
   • See the navigation menu bar example as a example/template
2. Update Unicode character for a checked the menuitemradio visually to be a solid circle similar to the SVG graphic in the original example.
3. Put the character entities (e.g. ●, ) used for indicating the checked state for menuitemradio and menuitemcheckbox in span elements with aria-hidden="true". The span should initially be empty and can just be in the source code and use CSS content selectors to add the check state character, this is a ARIA best practices to synchronize the visual state with an aria attribute. This will also require updating the documentation and adding a regression test for the presence of the aria-hidden attribute.

[zcorpan]

@jongund thanks, addressed now.

[jongund]

Howard the changes look very good, but the menu "separator" disappears in high contrast modes, so suggest using the same technique as used in the menubar-navigation.html example:

.menubar-navigation [role="separator"] {
  padding-top: 3px;
  background-image: url("data:image/svg+xml,%3C%3Fxml version='1.0' encoding='utf-8'%3F%3E%3Csvg xmlns='http://www.w3.org/2000/svg' width='12' height='12' viewBox='0 0 12 12'%3E%3Cline x1='0' y1='6' x2='12' y2='6' style='stroke:black;stroke-width:1' /%3E%3C/svg%3E%0A");
  background-position: center;
  background-repeat: repeat-x;
}

Also the roles, properties and states table needs to be updated to include the use of aria-hidden and then add the test to the regression file.

Thanks for your work on this example!

[howard-e]

Howard the changes look very good ...
Thanks for your work on this example!

@jongund this was all Simon (@zcorpan) :), but I'll be taking a look at your feedback as well.

[howard-e]

@jongund I see here that @zcorpan describes the use of aria-hidden on a span within the menuitem element. And the same has been done for menuitemcheckbox and menuitemradio. Just checking if you took note of this or is it that you would rather it be in a row for itself?

[jongund]

aria-hidden needs it's own table row in the table.
Example code:

<tr data-test-id="menu-aria-hidden">
    <td></td>
    <th scope="row"><code>aria-hidden="true"</code></th>
    <td><code>span</code></td>
    <td>Removes the character entities used for open and closed icons from the accessibility tree to prevent them from being included in the accessible name of the sort buttons.</td>
</tr>

This will also need a regression test in test\tests\menubar_menubar-editor.js, something like:

...
const ex =  {
...
spanSelector: '#ex1 [aria-hidden]`,
....
}
...
ariaTest(
  'Visual character entity for the open or closed state is hidden from AT',
  exampleFile,
  'menu-aria-hidden',
  async (t) => {
    await assertAttributeValues(t, ex.spanSelector, 'aria-hidden', 'true');
  }
);

[howard-e]

the menu "separator" disappears in high contrast modes, so suggest using the same technique as used in the menubar-navigation.html example

Made the suggested changes so please review again when you can @jongund.

Also the roles, properties and states table needs to be updated to include the use of aria-hidden

Please see #1876 (comment) for my own clarification.

and then add the test to the regression file.

It seems a previous commit was done to update the regression tests for aria-hidden: a983109.

[jongund]

Howard the changes look very good ...
Thanks for your work on this example!

@jongund this was all Simon (@zcorpan) :), but I'll be taking a look at your feedback as well.

This is good now.

[jongund]

I put some comments in the conversation related to aria-hidden documentation and regression testing.

[mcking65]

@jongund

I have one question. The only documentation we have on HCM is:

To support operating system high contrast settings, focus is highlighted by adding and removing a border around the menu item with focus.

This is substantially less than other examples. Should there be more? I'm not certain which text, if any, from other examples, e.g., switch or radio, where we have recently updated HCM documentation, might be applicable here.

If all your feedback has been addressed, please indicate so by changing your review status to approve.

[mcking65]

I can't figure out why regression 1 is failing when it runs against the PR. It passes against the push.

[mcking65]

I've completed editorial changes and review. This is an approving editorial review.