Modal dialog: small improvement for elementShouldBeHidden

[afercia]

Description

The modal dialog component uses some logic to determine what body children should be aria-hidden when the modal dialog is open.

gutenberg/packages/components/src/modal/aria-helper.ts

Lines 23 to 54 in 559391a

	export function modalize( modalElement?: HTMLDivElement ) {
	const elements = Array.from( document.body.children );
	const hiddenElements: Element[] = [];
	hiddenElementsByDepth.push( hiddenElements );
	for ( const element of elements ) {
	if ( element === modalElement ) {
	continue;
	}
	if ( elementShouldBeHidden( element ) ) {
	element.setAttribute( 'aria-hidden', 'true' );
	hiddenElements.push( element );
	}
	}
	}
	/**
	* Determines if the passed element should not be hidden from screen readers.
	*
	* @param element The element that should be checked.
	*
	* @return Whether the element should not be hidden from screen-readers.
	*/
	export function elementShouldBeHidden( element: Element ) {
	const role = element.getAttribute( 'role' );
	return ! (
	element.tagName === 'SCRIPT' ||
	element.hasAttribute( 'aria-hidden' ) ||
	element.hasAttribute( 'aria-live' ) ||
	( role && LIVE_REGION_ARIA_ROLES.has( role ) )
	);
	}

Some elements are correctly filtered out as they should not get the aria-hidden attribute. It looks like elements with the hidden HTML attribute could be filtered out as well, as they're already completely hidden from assistive technologies.

Currentlty, aria-hidden is added and then removed also to these elements, which seems unnecessary:

Step-by-step reproduction instructions

• Go to the post editor.
• Inspect the DOM.
• For the purpose of this test, make sure no speak messages have been sent to the aria-live regions.
• When no messages have been sent yet, observe the element with id a11y-speak-intro-text does have an HTML attribute hidden.
• Open a modal dialog e.g. the Keyboard Shortcuts one by pressing the keys Control+Option+H.
• Observe the a11y-speak-intro-text element gets an aria-hidden="true" attribute, which is unnecessary as the element is already hidden.
• There may be other elements with a hidden HTML attribute e.g. added by plugins that don't need aria-hidden="true".

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

• Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

• Yes

[vk17-starlord]

The code could be optimized by checking whether an element already has the hidden attribute, and if so, skip applying aria-hidden="true". The current behavior is redundant, as hidden already takes care of making the element inaccessible to assistive technologies.

This could be achieved by modifying the elementShouldBeHidden() function to include a condition like:

javascript

if (element.hasAttribute('hidden')) {
return false;
}

should i raise PR with this change @afercia ?

[afercia]

@vk17-starlord sure please do feel free to submit a PR.
I think the best approach would be to add one more 'OR' here, where some element with sprecific attributes are already checked.

gutenberg/packages/components/src/modal/aria-helper.ts

Lines 46 to 54 in d6a4c72

	export function elementShouldBeHidden( element: Element ) {
	const role = element.getAttribute( 'role' );
	return ! (
	element.tagName === 'SCRIPT' ||
	element.hasAttribute( 'aria-hidden' ) ||
	element.hasAttribute( 'aria-live' ) ||
	( role && LIVE_REGION_ARIA_ROLES.has( role ) )
	);
	}

[vk17-starlord]

@afercia sure working on it

[vk17-starlord]

@afercia can you check this ?