PostVisibility component: invalid HTML and better accessibility

[afercia]

When setting the post visibility, a "flying menu/panel" opens with the visibility settings:

Some issues noticed so far:

Invalid HTML: label elements can't contain div elements

As per the WordPress accessibility coding standards, labels should be explicit: that means not wrapping the element and using for/id attributes. Similar issues were already discussed and addressed for other components, so I'm avoiding to repeat the good reasons behind this requirement.

The password protected field has no label. Placeholders should not be used as replacement for labels... see also https://core.trac.wordpress.org/ticket/40331

The toggle button should use an aria-expanded attribute to give feedback about the PostVisibility panel opened state.

When closing the panel, focus should be moved back to the toggle button.

Pressing Escape should close the panel.

The radio buttons should be wrapped within a <fieldset> element and the Post Visibility text should be the fieldset legend: not everything must be a div, please use semantic elements for text when appropriate.

The radio buttons descriptions should be associated to the radio buttons using aria-describedby and an ID.

Not sure about Italic text: personally, I found it hard to read especially for text longer then one line. This is more a design decision though.

Not sure about the JS confirm when choosing "Private".

Aside, as mentioned in another issue, the button text (e.g. "Public") should reflect the available action e.g. "Set visibility" and not the option current state. This applies also to the Publish button that shows "Immediately" or the publish date: those are the state of the option, not the action associated with the button.

[aduth]

Some related notes in #1204 . Consolidating approaches into a common component could help guarantee consistency once existing issues have been resolved. A note there about DOM structure including reference to React-Portal as an option for "root-mounted" elements. This would be more for when the popover (a div) is rendered within a label, for example.

Not sure about the JS confirm when choosing "Private".

Can you elaborate on this point for why it might be a poor choice?

[afercia]

Can you elaborate on this point for why it might be a poor choice?

Well I'm not sure about how the new publish flow works. In the current publish box, when you select "Private" there's no JS confirm. If nothing has changed, I'm not sure there's the need for a confirmation in the first place. I may be missing something, though.

[afercia]

Also noting the toggle button doesn't need event.preventDefault();: it's a button, all it needs is a type="button" attribute so there's no default action to prevent.

[afercia]

Pressing Escape should close the panel.

I'm wondering if a Higher Order Component à ls react-click-outside could be handy to wrap any component that needs to be closed {or any other action?} when pressing Escape.

[aduth]

I'm wondering if a Higher Order Component à ls react-click-outside could be handy to wrap any component that needs to be closed {or any other action?} when pressing Escape.

Yep, I'd thought this could be included as part of abstracting popovers to a common Popover component, but if there's value in this behavior beyond just popovers, a higher-order component is certainly worth considering as a further abstraction.

[afercia]

Usability issue (also related to the JS confirm):

• when setting "Public" or "Password Protected", the panel stays open: users have to click outside of the panel to close it and that's good IMHO because there are no assumptions: it needs an explicit user action
• when setting "Private" instead, a JS confirm appears and pressing OK closes the pane. This is inconsistent with the other settings behavior and probably needs some re-thinking