Global Styles: Fix incorrect usage of ItemGroup in the Background image panel

[im3dabasia]

Part of: #67425

What?

Revised <ItemGroup> to use role="none" and prevent it from rendering an unnecessary role="list" when it is not semantically required.

Why?

Avoids adding unwanted list roles where a semantic list is not necessary, ensuring accessibility standards are maintained without enforcing unnecessary constraints.

How?

Updated <ItemGroup> to include role="none" and an optional as prop for flexibility. By default ItemGroup renders a as a list which was generating the wrong markup. Which is not good for accessibility

Testing Instructions

1. Navigate to Appearance > Editor
2. Navigate to Global Styles > Background.
3. Inspect rendered Button labeled 'Add background image' and confirm they no longer include role="list" when unnecessary.
4. Validate that UI behavior and appearance remain unaffected.

Before	After

|

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: im3dabasia <[email protected]>
Co-authored-by: afercia <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[im3dabasia]

Hey @afercia,

Whenever you have a moment, could you please review my PR? It addresses another instance of incorrect usage of the ItemGroup component in the Background Image panel.

I’d really appreciate your feedback!

[afercia]

When the background image is set, ItemGroup is rendered as a button element. The role=none would override the native button role, which isn't okay.

[im3dabasia]

Maybe we can solve this with imgUrl ? "none" : "button". I'll try it out and update you.

[afercia]

Or maybe based on the as prop. However, this would be just a temporary workaround.
This component is still used incorrectly and against its documentation. We can implement a temporary fix but the long term goal is to not allow this kind of incorrect usage.

[afercia]

@im3dabasia thanks for your PR. As mentioned in #67425 (comment) when the background image is set, this ItemGroup becomes a button element. Setting role=none on the button would override the native button role and assistive technology would not communicate this as a button.
One option could be setting the role conditionally but it wouldn't be great having a button element with role=button.
I'd rather remove the ItemGroup entirely and explore a design change.

[im3dabasia]

when the background image is set, this ItemGroup becomes a button element.

Thank you for your feedback @afercia .

I agree that this is happening, but the button has role="list", which I believe is incorrect. While redundant, it would be better to use role="button" rather than having a button element with role="list".

As a temporary fix, I’d like to propose two possible solutions. Let me know which one you think works best:

1. Add role prop value conditionally

<ItemGroup
  role={ as === 'span' ? 'none' : 'button' }
  as={as}
  className={className}
  {...toggleProps}
/>

This would add role="none" by default when a span is rendered and otherwise apply role="button".

3. Add role prop only when as === 'span'

<ItemGroup
  {...(as === 'span' ? { role: 'none' } : {})}
  as={as}
  className={className}
  {...toggleProps}
/>

This approach adds the role prop only when a span is rendered; otherwise, it allows the default behavior to prevail (i.e., a button with role="list").

Personally, I believe the first approach is better as it improves accessibility overall for this issue.

Looking forward to here your thoughts @afercia

[afercia]

a button with role="list"

We should avoid that so the first approach is better. However, this can only be acceptable as a temporary solution.
Please add an inline // TODO: comment to explain why there is a temporary fix for the role. It would be good to use the same 'todo' comment requested also for #68633