search

home-assistant / frontend

:lollipop: Frontend for Home Assistant
https://demo.home-assistant.io
Other
3.96k stars 2.69k forks source link

Unlabeled buttons #3267

Open joshms123 opened 5 years ago

joshms123 commented 5 years ago

Home Assistant release with the issue: 0.94.1

Last working Home Assistant release (if known): None

UI (States or Lovelace UI?): Lovelace

Browser and Operating System: Chrome, Windows 10

Description of problem:

As a visually impaired user, I have found several accessibility issues over the year or so while using home assistant. The biggest issue currently (of mine at least) is the number of unlabeled buttons in the Lovelace UI. Currently when navigating with my screen reader, I hear "unlabeled 0", "unlabeled 1", etc. With a different screen reader (NVDA), I get even less feedback. List of unlabeled buttons I know of currently:

Javascript errors shown in the web inspector (if applicable): N/A

Additional information: N/A

balloob commented 5 years ago

I have opened a pull request to address a couple of the mentioned missing labels. The pull request is #3310.

balloob commented 5 years ago

For people looking into helping with this issue, there is an accessibility dev tool to help find bad buttons: https://www.deque.com/axe/.

steverep commented 5 years ago

I am a visually impaired user as well and noticed media device buttons, as well as a few others, got labeled in a recent release. Thanks for that.

My cover device cards are not labeled yet though.

I'll point out more as I come across them.

balloob commented 5 years ago

Opened pull request #3421 to add labels to the cover and cover tilt controls.

joshms123 commented 5 years ago

Hi there!

Great work on the accessibility fixes so far, I really appreciate it!

The following buttons are unlabeled:

The following button from my original post is still unlabeled:

balloob commented 5 years ago

@joshms123 addressed in #3422

steverep commented 5 years ago

In the modal dialog when editing an entities card, there are 2 unlabeled buttons associated with each entity in the list. One of them deletes the entity and I'm not sure what the other one does.

balloob commented 5 years ago

@steverep addressed in #3430

steverep commented 5 years ago

Two more:

bramkragten commented 5 years ago

Do you have a more specific example of the close button? I don't see a close button in the adding integration and the more info dialog does have a label. The view editing dialog only has a cancel button, but that has text in it so should be ok?

steverep commented 5 years ago

Do you have a more specific example of the close button?

Sorry, I was a bit lax on that and looks like several were fixed. Here's a few that I just found:

balloob commented 5 years ago

@steverep wanted to let you know that @bramkragten is working for @NabuCasa and will focus on the Home Assistant frontend. Improving accessibility is one of this tasks so please keep the issues coming.

steverep commented 5 years ago

A few more to add to the list from my last comment:

iantrich commented 4 years ago

@balloob @bramkragten is there a way to integrate axe into our test builds to alert us before merging new unlabeled buttons?

steverep commented 4 years ago

More...

steverep commented 4 years ago
springstan commented 4 years ago

Can we somehow translate the aria-labels in js files? So far I had no luck and I was wondering if there was any way to do so.

iantrich commented 4 years ago

@springstan only if it extends the LocalizeMixin, I believe

springstan commented 4 years ago

@iantrich I thought so too, but as soon as I use localize with the aria-label the attribute disappears.

bramkragten commented 4 years ago

@springstan you should use the $ after the attribute name in Polymer so it will be an attribute, otherwise Polymer makes it a property.

          aria-label$="[[localize('ui.translation.key')]]"
nickallan1 commented 4 years ago

Hi just started using homeassistant and I'm also a screen reader user. Some unlabelled controls I've found developer tools there is a left arrow before the tabs and a right arrow after the tabs. these are visually disabled but are still active for a screen reader as the disabled attribute hasn't been set, they also need to have text labels.

Interesting, until you activate a tab control, the screen reader doesn't see its label. e.g I've got the MQTT tab, but until I activate that tab, the "MQTT" text is not visible to my screen reader. Once you've activated a tab once, the text label is then visible.

service tab when a value is in the service edit dropdown, the clear button that appears needs a label. there's also a second button that shows up there as well, not sure what it does.

screen reader being used for testing is Jaws 2020 from www.freedomscientific.com

nickallan1 commented 4 years ago

Another issue I've found is in the sidebar. when you look at this with a screen reader there are 2 controls, the first is a button that has a label "Sidebar toggle" and next is a listbox. If you use the up and down arrows in the listbox before pressing the sidebar toggle, the screen reader announces the url for each item in the list instead of the item's text. e.g my screen reader announces the following as you down arrow Link http://hassio.local:8123/logbook Link http://hassio.local:8123/history Link http://hassio.local:8123/core_configurator

If I press the sidebar toggle button and then use the down arrow in the listbox, the screen reader says Logbook Link History Link Configurator Link The issue is that the Sidebar toggle button doesn't indicate its state, so you can't tell if your in the mode where the listbox will speak correctly or not until you've started arrowing through the list items. suggestion

  1. just make the listbox work correctly all the time without needing to press the sidebar toggle button.

or

  1. use the aria-expanded attribute on the sidebar toggle button to indicate if the sidebar is visible or not. if this is the preferred solution, you probably want to actually hide the listbox element from screen readers if the toggle button is in its collapsed state. https://www.w3.org/WAI/GL/wiki/Using_aria-expanded_to_indicate_the_state_of_a_collapsible_element
bramkragten commented 4 years ago

Thanks for reporting @nickallan1! I'll look into this shortly.

scottyphillips commented 4 years ago

Hi Devs, @nickallan1 also mentioned the Lovelace Thermostat card in particular the HVAC mode icons needs to be looked into as well as it relates to a custom component of mine. (see https://github.com/scottyphillips/mitsubishi_echonet/issues/9)

I opened (and then closed) issue https://github.com/home-assistant/home-assistant/issues/30495 but in hindsight should have checked here first.

nickallan1 commented 4 years ago

thanks @bramkragten in relation to the thermostat controls in lovelace. as noted above the buttons aren't labelled. I assume there's supposed to be a slider to allow changing of temperature, its also not accessible.

bramkragten commented 4 years ago

For the thermostat slider, @thomasloven can probably help with that?

steverep commented 4 years ago

@bramkragten the last 2 check boxes above don't look to be fixed in the latest release but you checked them off. And while I'm at it...

steverep commented 4 years ago

As far as I can tell, all buttons have now been replaced with labels in the form "hass:icon-name" in the latest release.

I'm losing confidence in Home Assistant taking accessibility seriously. Is there a goal to close this issue and ensure new releases have appropriately labeled buttons or not? Providing alternative text for graphical elements is literally criterion 1.1.1 in WCAG. There are a lot more accessibility issues to report.

bramkragten commented 4 years ago

That is something one of our upstream dependencies does, I will fix that. We made some switches on upstream elements we use and this one does things differently than the one we used before.

Thanks for letting us know this so we can fix it!

bartbunting commented 3 years ago

Hi, another screenreader user here. Having read the comments here I'm still encountering many buttons reporting as "Unlabeled ". frequently it is what I can only assume is the "Back" button as it appears to perform that function. Also on the Configuration > Integrations page the button to add a new integration is still read as "Unlabeled 0". I'd be happy to try and fix some of these but have no idea where I should be looking as I don't understand how they are being created. Guessing by some library as grep doesn't find them. Would very much appreciate this being addressed.

spacegaier commented 3 years ago

With my PR https://github.com/home-assistant/frontend/pull/7587 all fab buttons (used e.g. to setup new integrations, create new automations or save changed scripts) should now provide an aria-label.

My next PR https://github.com/home-assistant/frontend/pull/7588 will propose a way to fix the "back" and "next" buttons as well.

nickallan1 commented 3 years ago

This isn't working for me. Looking at the DOM, It would seem to be because the actual button element has aria-label="". the aria-label that has the correct button text is a parent of the button. At least Jaws is looking at the aria-label="" on the button that gets focus so it calls it unlabelled0. This is the same type of issue you see in the lovelace controls as well in most cases. Note, if you turn jaws's virtual buffer off and tab through the UI, it does seem to pick up the aria-label that actually has a text value, but in the normal virtual buffer interface, most things seem to still be unlabelled. Parcial example of the setup integration button below.


<mwc-fab slot="fab" aria-label="Set up a new integration" title="Set up a new integration">
<button class="mdc-fab  " aria-label="">

        <mwc-ripple class="ripple"></mwc-ripple>

        <span class="icon-slot-container">
          <slot name="icon">
            <!----><!---->
          </slot>
        </span>
        <!----><!---->
        <!----><!---->
      </button>
        </mwc-fab>```
spacegaier commented 3 years ago

@nickallan1 Are you referring to my PRs or in general to this topic? With my changes from PR #7587, I get the following DOM. The <button> does have the aria-label filled properly.

grafik

The DOM you posted is the old, before situation.

nickallan1 commented 3 years ago

@spacegaier Is this supposed to be in the current production release? I'm running the following versions Home Assistant 0.117.2 Frontend version: 20201021.4 - latest host_os HassOS 4.15 supervisor 2020.10.1

spacegaier commented 3 years ago

@nickallan1 No, not yet. My changes are a pull request (= coding proposal) that I submitted earlier today and still needs to be reviewed and merged into the official coding version. So it will hopefully be part of the next beta versions.

bartbunting commented 3 years ago

@spacegaier I just tested and your change makes the button to add a new integration speak correctly. I am still seeing an "Unlabled 0" for what appears to be the back arrow. This appears both on the Integrations page itself and also at the top of add a new integration page. This is a great improvement though. Haven't been able to test every instance you updated but will try and test further.

spacegaier commented 3 years ago

Just to make sure: You are aware that the PR for the fabs (the "new integration" for example) and the one for the backend buttons are two separate ones? Did you test both?

bartbunting commented 3 years ago

Sorry my bad, I didn't notice that. I'm still getting my head around how to test and I think my way is likely not the best as yet. Having checked out both branches of your repo I can report that yes they both do different things. fab-aria-labels fixes the Setup new integration" button and friends. The aria-prev-next branch labels the "back" button on the config > integrations screen (and presumably in other places it's used).

However there is still a button with "Unlabeled 0" at the top of the model for adding a new integration. Guessing it's meant to be close or similar?

This is great progress thanks!

I am happy to test and report more of these issues, just need to learn a smoother workflow for testing pull requests and make better friends with the chrome devtools using jaws.

spacegaier commented 3 years ago

Yes, the close button of that particular dialog is not included in those PRs, I will create another one for all <ha-icon-button>. 102 instances to check and potentially adjust in the source.

Nardol commented 3 years ago

Hello,

I noticed another group of buttons which are not labelled: all button for Google cast devices. Also when assigning a device to a room, Orca announce a button with no label, which I cannot test because the focus return to the edit box to specify the room.

I use Orca on Debian Buster.

joshms123 commented 3 years ago

Hello there, There are still quite a few unlabeled buttons in the current home assistant release; including lovelace cards, configuration, supervisor, ETC. I would like to assist in resolving these, firstly, is there a way to easily edit the HTML where these buttons are? Secondly, is there possibly an automated testing tool that could look for these issues to assist developers in resolving these issues in their code, before its published to release? Thanks, Josh.

spacegaier commented 3 years ago

I am working on it, but it is a big task, since we have a lot of icon buttons throughout HA: https://github.com/home-assistant/frontend/pull/9230

steverep commented 2 years ago

Again, I'm going to ask if there is ever going to be a serious (or at least half-heart) attempt to ensure accessibility of Home Assistant? This issue is well over 2 years old and again I'll mention it's criterion 1.1.1 on accessibility guidelines.

I'd love to just list problem buttons I find as I have done earlier in this thread a number of times, but some of the buttons already fixed were then unlabeled again in a future release, so I don't want to waste my time.

Surely the developers could write a simple unit test to check all pull requests for unlabeled buttons before allowing a merge.

joshms123 commented 2 years ago

Hi, seems like #9230 was merged. I know there are still a few problems, but overall I have noticed quite an improvement.

steverep commented 2 years ago

Is that PR in the next release?

spacegaier commented 2 years ago

Is that PR in the next release?

That was already part of the current 2021.11 release.

I'd love to just list problem buttons I find as I have done earlier in this thread a number of times, but some of the buttons already fixed were then unlabeled again in a future release, so I don't want to waste my time.

We can only fix what we are aware of. Overall my above mentioned PR generalized the handling of all icon-buttons, so a lot more of them should now have an ARIA label assigned. But for sure there are still a few, that need to be onboarded, so reporting them makes sense.

Again, I'm going to ask if there is ever going to be a serious (or at least half-heart) attempt to ensure accessibility of Home Assistant?

We are always trying to ensure accessibility, but of course that competes as with all requests for the limited resources of the project. but just yesterday there was another UX meeting reg. accessibility, so those discussions are happening (see the "#devs_ux" Discord channel).

steverep commented 2 years ago

We can only fix what we are aware of. Overall my above mentioned PR generalized the handling of all icon-buttons, so a lot more of them should now have an ARIA label assigned. But for sure there are still a few, that need to be onboarded, so reporting them makes sense.

In that case, in the spirit of specifics and not just complaining:

There's more... I just can't think of them off the top of my head. I'll find them.

spacegaier commented 2 years ago

I created the PR #10858 for the automation page (found two more icon buttons on that page that had issues that will be fixed by the PR).

spacegaier commented 2 years ago

Second raised point (well actually the first in your two point list 😄), will be solved by PR #10859.

steverep commented 2 years ago

As promised: