Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ui5-li] Unable to set accessibleRole attribute for resulting ui5-icon when using icon property #4682

Closed
1 of 4 tasks
dkris opened this issue Feb 2, 2022 · 12 comments
Closed
1 of 4 tasks

Comments

@dkris
Copy link

dkris commented Feb 2, 2022

Bug Description

Hi Team,

When icon property for StandardListItem is set, there is no option to set the accessibleRole (#4548) property for the resulting ui5-icon.

When we specify action-settings as the value for the icon parameter, the resulting svg image for this list item has a default img role set to it.

<ui5-icon part="icon" class="ui5-li-icon" name="action-settings" ui5-icon="">
<svg class="ui5-icon-root" viewBox="0 0 512 512" focusable="false" preserveAspectRatio="xMidYMid meet" xmlns="http://www.w3.org/2000/svg" tabindex="-1" role="img" aria-label="Settings">...</svg>
</ui5-icon>

When a screen reader is used to navigate the application, the settings list item is announced twice as "Settings settings".

Expected Behavior

accessibleRole should be a config option on the ui5-li, that can be passed down to ui5-icon when the icon property for StandardListItem is set. Labels should be announced only once when using screen reader to navigate within the application.

Steps to Reproduce

  1. Go to https://codesandbox.io/s/wonderful-burnell-m62vm?file=/index.html
  2. Using screen reader, navigate through the screen

Context

  • UI5 Web Components version: 1.1.0
  • OS/Platform: All
  • Browser: All
  • Affected component: ui5-li

Priority

  • Low
  • Medium
  • High
  • Very High

The priority indicates the severity of the issue. To set the appropriate priority consider the following criteria:

  • Breaks entire application or system - High or Very High
  • Accessibility issue - Medium or High
  • Functional issue - Medium or High
  • Visual issue - Low or Medium

Note: The priority might be re-evaluated by the issue processor.

Stakeholder Info (if applicable)

  • Organization: SF
  • Bussiness impact: Medium
@Shtilianova
Copy link

Hello @dkris,
This is behavior is correct and it's happened because the icon name and the text are the same. For this reason the "settings" is announced twice. If you change the text to "apple" for example apple it will announce "settings apple".
Regards, Diana

@ilhan007
Copy link
Member

ilhan007 commented Feb 2, 2022

Hello Didi, actually this is a feature request, the author requests an API to define/control the accessible name of the ui5-icon that is rendered inside StandardListItem.

@ilhan007 ilhan007 reopened this Feb 2, 2022
@ilhan007
Copy link
Member

ilhan007 commented Feb 2, 2022

Hello @dkris from our offline discussions I know that you need to control/define the accessibleRole of the internally rendered ui5-icon, rendered inside the StandardListItem. Is that all? Please comment if I miss something.
Meanwhile, I will forward your request to the team.

Hello @SAP/ui5-webcomponents-topic-p it seems that our SF colleagues, that recently requested an API for the ui5-icon,
are using the StandardListItem, where the icon is not accessible directly and they need to control the accessibleRole of the ui5-icon, rendered inside the StandardListItem. We either need to expose a slot, or property. similar to the FCL, ShellBar, allowing to set aria attrs to the internals.

@dobrinyonkov
Copy link
Contributor

dobrinyonkov commented Feb 11, 2022

Hello,

the purpose of the icon here is only decorative. Should have role="presentation" and aria-hidden="true" by default.

@dkris
Copy link
Author

dkris commented Feb 22, 2022

Hello @dkris from our offline discussions I know that you need to control/define the accessibleRole of the internally rendered ui5-icon, rendered inside the StandardListItem. Is that all? Please comment if I miss something. Meanwhile, I will forward your request to the team.

Hello @SAP/ui5-webcomponents-topic-p it seems that our SF colleagues, that recently requested an API for the ui5-icon, are using the StandardListItem, where the icon is not accessible directly and they need to control the accessibleRole of the ui5-icon, rendered inside the StandardListItem. We either need to expose a slot, or property. similar to the FCL, ShellBar, allowing to set aria attrs to the internals.

You're correct @ilhan007. Thank you for checking.

@ilhan007
Copy link
Member

@dkris you can see the explanation above - "the purpose of the icon here is only decorative - should have role="presentation" and aria-hidden="true" by default."

@dkris
Copy link
Author

dkris commented Feb 23, 2022

@ilhan007 Is this request going to be handled by @SAP/ui5-webcomponents-topic-p. Will we have an config option available to set the role of the icon within StandardListItem?
I noticed that you closed the issue. Does this mean it's handled?

@ilhan007
Copy link
Member

@dkris no, as far I understand this is the team's statement - the purpose of the icon in the StandardListItem is only decorative - it should have role="presentation" and aria-hidden="true". It's not meant to have different role.
@dobrinyonkov is my observation correct?

@dobrinyonkov
Copy link
Contributor

dobrinyonkov commented Feb 24, 2022

I apologize for the misunderstanding. I meant that we need to add these attributes internally. Currently the role is 'img', which should be presentation and with aria-hidden according to OpenUI5 implementaiton of the List control.

@dkris, apart from excluding the icon from the acc tree do you need the API for any other use case? Will it be enough to add the above attributes internally?

@dkris
Copy link
Author

dkris commented Mar 2, 2022

@dobrinyonkov Excluding the icon from acc tree is the only use case for this API request.

@dobrinyonkov
Copy link
Contributor

Hello, PR #4742 will add the role="presentation" and aria-hidden="true" attributes.

@terezamch
Copy link
Contributor

Hello @dkris ,

As the last mention PR is already merged and the issue is fixed, I am closing the GitHub issue.

Best regards,
Tereza

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Completed
Development

No branches or pull requests

5 participants