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

ComboBox: aria label of an item cannot be set separately from preview text #6665

Closed
janormanms opened this issue Oct 11, 2018 · 6 comments
Closed

Comments

@janormanms
Copy link
Contributor

Bug Report

  • Package version(s): 6.74.0

Priorities and help requested:

Are you willing to submit a PR to fix? No

Requested priority: High (accessibility)

Products/sites affected: Outlook

Describe the issue:

Setting the ariaLabel property on IComboBoxOption does not set the aria-label on the option. It only does so if useAriaLabelAsText is set to true, which results in the aria label being used as the preview text.

Related PRs:
#4540
#4722

Actual behavior:

Aria label can only be set if you want to use it as the preview text as well

Expected behavior:

Aria label can be set, and if you want to use it as the preview text you can use the existing useAriaLabelAsText property

See ComboBox._renderOption and ComboBox._getPreviewText, essentially leave _getPreviewText as is but update the aria-label props being set on the CommandButton or CheckBox to be "item.ariaLabel ? item.ariaLabel : item.text"

@dzearing
Copy link
Member

@natalieethell were you working on this? I'm not sure if that behavior suggested is even right. May be good to chat with @jspurlin

@natalieethell
Copy link
Contributor

natalieethell commented Nov 12, 2018

Hey @dzearing I am not actively working on this. I just moved the card from “Ready for Engineering” to “In PR” because I noticed there was a PR opened for it.

@dzearing
Copy link
Member

@natalieethell Ah oops I see it now :) thanks!

@micahgodbolt
Copy link
Member

seems the PR for this fizzled out. something to look into during #6030? @ecraig12345

@micahgodbolt
Copy link
Member

micahgodbolt commented Jan 2, 2020

I was about to create a PR to allow a different value for aria-label, but it was basically the same as the PR closed above, which @jspurlin said was unnecessary.

My solution was to add a getAriaLabel similar to getPreviewText:

  private _getPreviewText(item: IComboBoxOption): string {
    return item.useAriaLabelAsText && item.ariaLabel ? item.ariaLabel : item.text;
  }

  private _getAriaLabel(item: IComboBoxOption): string {
    return item.ariaLabel ? item.ariaLabel : item.text;
  }

@jspurlin unless you think this approach is better than the closed PR, i'll let this issue close via "By Design"

@jspurlin
Copy link
Contributor

jspurlin commented Jan 6, 2020

@micahgodbolt , yeah, I think this should be closed as "by design" due to the issues I called out in #6797

@microsoft microsoft locked as resolved and limited conversation to collaborators Feb 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
6 participants