Fluentui: [DetailsList] Aria Label being set incorrectly

[Shobhit1]

Environment Information

• __Package version(s)__: Fabric React 6.114.0

Please provide a reproduction of the bug in a codepen:

Actual behavior:

For DetailsList component, ariaLabel prop is not doing anything, because aria-label only checks if the iconOnly prop is set or not and then assigns the name as aria-label or assigns undefined

Expected behavior:

Ideally, it should always give priority to ariaLabel, even if there is a iconOnly prop set to true.
Also, even if we want to give priority to name in case of iconOnly spans, the component should be setting ariaLabel and not undefined for false condition.

Priorities and help requested:

Are you willing to submit a PR to fix? Yes

Requested priority: Normal
Products/sites affected:

[Shobhit1]

I can fix it, given I know the reasoning behind setting name as the aria-label and also why would we set it to undefined if its not marked as iconOnly

[ecraig12345]

Thanks for reporting this. I'm a bit confused, do you know the original component author's reasoning behind setting the aria-label the way it's being done right now? Also, which file is that screenshot from? (edit: it's DetailsColumn.base.tsx around line 85)

[KevinTCoughlin]

@Shobhit1 is there an accessibility validation error?

Looking at the code, aria-label is used when the column renders _only_ an icon. If the column renders text, then aria-labelledby is used (the inverse of the line above).

https://github.com/OfficeDev/office-ui-fabric-react/blob/74f602bf522e0074f17dafefb9cae8b37e04cf37/packages/office-ui-fabric-react/src/components/DetailsList/DetailsColumn.base.tsx#L86

My understanding is that aria-labelledby takes precedence over _all_ other name sources.

As required by the text alternative computation, user agents give precedence to aria-labelledby over aria-label when computing the accessible name property.
https://www.w3.org/WAI/PF/aria-1.1/states_and_properties#aria-labelledby

Importantly, aria-labelledby overrides all other name sources for an element. So, for example, if an element has both an aria-labelledby and an aria-label, or an aria-labelledby and a native HTML label, the aria-labelledby label always takes precedence.
https://developers.google.com/web/fundamentals/accessibility/semantics-aria/aria-labels-and-relationships

I'm unsure of the issue here. Also, it seems like the fix as proposed would be a no-op since aria-labelledby would take precedence over aria-label.

cc: @natalieethell

[natalieethell]

I agree with Kevin's reasoning above for when we use aria-label versus aria-labelledby. @Shobhit1 Does that make sense? The logic here seems intentional.

[Shobhit1]

So, I was getting an error when I ran accessibility-checks for this component. I agree that aria-labelledby takes precedence.
But, there is no way for me to update this value using props. I was trying to fix the issue that I found by changing aria-label but since that prop doesn't really do anything, it was not feasible. I am sorry if my first issue caused any confusion to what the problem was. I never meant to compare aria-labelledby with aria-label.

[natalieethell]

Thanks for the clarification!

From my understanding, I believe that we want to maintain this aria-labelledby logic, which references the actual column name element. The W3C standards state the following:

If the label text is visible on screen, authors SHOULD use aria-labelledby and SHOULD NOT use aria-label. Use aria-label only if the interface is such that it is not possible to have a visible label on the screen.
https://www.w3.org/WAI/PF/aria-1.1/states_and_properties#aria-labelledby

As for the aria-label logic, we do use column.ariaLabel for the element rendered here, which is referenced in aria-describedby:

https://github.com/OfficeDev/office-ui-fabric-react/blob/7937c6b99ffa75b52f7b563eca1c38a56204b504/packages/office-ui-fabric-react/src/components/DetailsList/DetailsColumn.base.tsx#L246-L248

I considered what the behavior would be if we changed column.name here to column.ariaLabel || column.name.

https://github.com/OfficeDev/office-ui-fabric-react/blob/7937c6b99ffa75b52f7b563eca1c38a56204b504/packages/office-ui-fabric-react/src/components/DetailsList/DetailsColumn.base.tsx#L85

However, since we already have column.ariaLabel used in the aria-describedby, the screen reader already reads out the column name and the aria label.

@Shobhit1 what error were you getting when running the accessibility checks?

[Shobhit1]

In my case, I didn't have an icon and the name was blank too. Since there was no icon, I never passed isIconOnly prop and since I did not want a name, my name prop was ''.
This made aria-label to be undefined, even though I was passing ariaLabel prop from Coloumn. I raised this issue to find out why was that.
I fixed it by making isIconOnly to true and then passing the aria through column name.

Error Description: "Ensures buttons have discernible text"

[natalieethell]

@Shobhit1 I see. If you have a column header with no name and no icon, then there is no extra context or added visual appearance of the element, which means that a sighted user does not get any benefit over a non-sighted user. So in that case, I wouldn't see a need for an aria label. To quote the W3C standards site on aria-label,

[...] elements can be given the attribute aria-label to provide an accessible name for situations when there is no visible label due to a chosen design approach or layout but the context and visual appearance of the control make its purpose clear.
https://www.w3.org/TR/WCAG20-TECHS/ARIA14.html

@KevinTCoughlin what are your thoughts here?

[natalieethell]

Curious, what is the scenario in which you have a blank column header?

[Shobhit1]

It actually has nothing but icons (as content on the column), so ideally isIconOnly solved my problem and I am starting to think, it is the correct way to fix my particular scenario because it correctly describes the content of my column.
But, I feel that ariaLabel prop is wasted in those situations because we dont really use it. Curious to know if the name being set as aria-label for column header button, in case of icon only columns solve this for everyone?
Willing to close this issue if its becoming a drag though. Thanks for engaging in this conversation and help me understand the logic for this.

[natalieethell]

If you pass in column.ariaLabel, that will be rendered in the label element that is referenced by aria-describedby, along with some other information like column.filterAriaLabel, column.sortDescendingAriaLabel, column.sortAscendingAriaLabel, and column.groupAriaLabel. So, it is used.

If we did the following

aria-label={column.isIconOnly ? column.ariaLabel || column.name : undefined}

then we'd have the column.ariaLabel in two locations and it is read out twice by the screen reader (aria-label and aria-describedby are both read out). So, I'm not sure that it's really necessary. Does that make sense? Let me know what you think.

However, if we do really want it, I also don't see much harm in adding it. Example 2 here on developer.mozilla.org shows a scenario that uses both aria-label and aria-describedby, although they are pretty different.

And of course! Happy to discuss this. I want to make sure we make the right move here.

[KevinTCoughlin]

If you pass in column.ariaLabel, that will be rendered in the label element that is referenced by aria-describedby, along with some other information like column.filterAriaLabel, column.sortDescendingAriaLabel, column.sortAscendingAriaLabel, and column.groupAriaLabel. So, it is used.

If we did the following

aria-label={column.isIconOnly ? column.ariaLabel || column.name : undefined}

then we'd have the column.ariaLabel in two locations and it is read out twice by the screen reader (aria-label and aria-describedby are both read out). So, I'm not sure that it's really necessary. Does that make sense? Let me know what you think.

However, if we do really want it, I also don't see much harm in adding it. Example 2 here on developer.mozilla.org shows a scenario that uses both aria-label and aria-describedby, although they are pretty different.

And of course! Happy to discuss this. I want to make sure we make the right move here.

I'm in agreement with @natalieethell's analysis and glad that she unblocked you @Shobhit1 by using isIconOnly. Good discussion all.

Regarding changing the aria-label conditional which would result in a double-read.. I think we should avoid it as it will be valid, but seem like a bug for those using assistive technology.

I _do_ think we need to surface this isIconOnly prop better as part of our developer documentation. We are actively working on improving this, specifically by improving the visibility into our component props APIs.

@Shobhit1 if your accessibility validation issue is solved by the usage of isIconOnly, would you mind closing this issue?

Thank you all for the discussion. It is helping to inform our documentation strategy going forward 😃.

[Shobhit1]

Yes. I will close this issue. Thank you for all the help.

[KevinTCoughlin]

We had another report of this issue. It _is not straightforward_ to apply an aria-label to a column when rendering the IColumn's name is not desired. We should make this clearer either via documentation, refactoring code, or a run-time warning.

The diff I supplied as a workaround was:

diff --git a/packages/office-ui-fabric-react/src/components/DetailsList/examples/DetailsList.Basic.Example.tsx b/packages/office-ui-fabric-react/src/components/DetailsList/examples/DetailsList.Basic.Example.tsx
index 30b54b085..5c4d249b7 100644
--- a/packages/office-ui-fabric-react/src/components/DetailsList/examples/DetailsList.Basic.Example.tsx
+++ b/packages/office-ui-fabric-react/src/components/DetailsList/examples/DetailsList.Basic.Example.tsx
@@ -25,7 +25,8 @@ for (let i = 0; i < 200; i++) {
 const _columns: IColumn[] = [
   {
     key: 'column1',
-    name: 'Name',
+    name: 'Name used as ariaLabel',
+    isIconOnly: true,
     fieldName: 'name',
     minWidth: 100,
     maxWidth: 200,

I vote we add this to backlog to tackle at a later date.

[Shobhit1]

This is the exact workaround I did to make sure that there is some ariaLabel on the column header.

[cathymc]

Our team is also having trouble with aria-label being read by both JAWS and Narrator, most likely due to this priority issue. We are using the aria-label to announce whether the column is filtered or not filtered, so the work around of using the 'name' attribute may not work for us.

Any ideas for another work around or added more accessibility with column filters?

[dzearing]

@KevinTCoughlin should this be added? Also you might know the answer to @cathymc 's question.