BP-Nouveau: Keyboard focus lost in air with Messages bulk actions UI

[mercime]

The current bulk actions UI hides the options for the bulk actions off-screen using .bp-hide class which causes keyboard focus to jump to the top ala #7711 after tabbing through the checkbox:

.bp-hide {
	height: 0;
	left: -999em;
	overflow: hidden;
	position: absolute;
	top: -999em;
}

We could restructure the new Messages bulk actions UI to be similar in functionality to Gmail's custom components which are accessible for visual users, screen readers users, and keyboard users.

• Instead of using <input type="checkbox" ...> we could use the "div for interactive elements that do something" - the semantic <button> element.
• Add aria-haspopup="true" aria-expanded="false" bp-tooltip="Select messages bulk action" aria-label="Select messages bulk action" or better yet, remove the aria-label and bp-tooltip and show text in button instead. Use JavaScript to change state to aria-expanded="true" when clicked on
• For the options available, use div with role="menu" aria-haspopup="true" to wrap all options, each option is wrapped with role="menu-item" and use JS to indicate which is selected.

[mercime]

In r11906:

Nouveau: Fix keyboard focus for hidden form controls.

This prevents loss of focus for keyboard users as they tab through the form
controls where initial state is hidden.

Fixes #7711, #7720. See #7714.

[mercime]

In attached patch, kept most of the original structure and made the following changes to fix the focus jump along with some other enhancements:

• Removed the bp-screen-reader-text class from the label of the checkbox for some words
• Added .bp-hide and .bp-show classes for .bulk-actions-wrap div in buddypress-messages.js
• Removed the nesting of the checkbox within the label element for both the bulk action and the message threads
• Added tooltip to the bulk-actions button which has a check icon
• Moved the li.user-messages-bulk-actions to after the li.user-messages-search so that the bulk actions will be closer to the message thread on desktop and especially on mobile view. See attached messages-bulk-action-fix-mobile.gif and messages-bulk-action-fix-desktop.gif attached.
• Added style support for above-mentioned.

For review/comments.

[DJPaul]

In the patch:

• <label for="user_messages_select_all"><span class="bp-screen-reader-text"><?php esc_html_e( 'Select ', 'buddypress' ); ?></span><?php esc_html_e( 'All Messages', 'buddypress' ); ?></label>
  • The string can't be split. This is meaningfully untranslatable.

I trust you with the rest of the markup changes.

[mercime]

Thanks for the review @djpaul. Attached 7714.2.patch removes the span with the bp-screen-reader-text class.

[DJPaul]

Thanks - commit when you're happy. I trust you with the rest of the markup changes, but if you want those reviewed, I can help corral someone into looking.

[hnla]

I would like to review please before commits are made, I too trust to the changes and the keyboard focus issue is good catch and important but would like to check the changes over.

[hnla]

@mercime yep like the additional enhancements & in addition perhaps would quite like to remove the initial border wrapping the hidden select/action popup when shown, this is for two reasons #1 it removes the slight page jump to accommodate the element inline ( border adds height ) and #2 having the element(s) show without border gives a more text like visual style that fits better with your text addition 'All Messages', also on hover of the select we do have a border appear to help define this as a select element visually ( which could be increased in strength/color)

.buddypress-wrap .subnav-filters .user-messages-bulk-actions .bulk-actions-wrap {
    border: 1px solid #eee;
}

Other than that I do have a major concern over the input elements being used without a form parent and where they don't need strictly to be inputs ( as you mention earlier) if we are only passing data to scripts ( a bad thing but...) not sure what we can do about this though at this stage.

Ignoring the above input concern lets bash this in, the focus is important the enhancements good and if we can pull out the border style above I'm happy :)

[mercime]

Refreshed patch

[mercime]

Refreshed patch 7714.3.patch removes the following improvements from 7714.2.patch:

• _nouveau_messages.scss and bp_filters.scss with generated CSS changes r11920 by hnla
• messages.js in r11916 by hnla

Committing 7714.3.patch in a few minutes

[mercime]

In 11951:

BP Nouveau: Improve HTML Markup of Messages bulk actions.

-Remove nesting of input controls within the label tags for a cleaner markup.
-Move the list element containing the checkbox and bulk actions closer to the
message list especially for mobile view.
-Reveal the label tag for the checkbox so that sighted users may see what the checkbox is for.
Style and JS support for this change have already been committed in r11916 and
r11920.

Fixes #7714.