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

Accessibility and styling updates for MultiDropDownComponent #2093

Merged
merged 1 commit into from
Dec 4, 2017

Conversation

andimarc
Copy link
Contributor

@andimarc andimarc commented Nov 29, 2017

(Fixes #1738)

I closed the related PR (Accessibility updates for MultiDropDownComponent #1751) because I decided to use aria-activedescendant to handle moving the "focus".

Updates:

  • Control collapses if display text or arrow are clicked.
  • Control does not lose track of focus when an option is clicked.
  • Navigation to top or bottom of list using "Home" and "End" keys is supported.
  • The selected state for a given option is indicated by styles instead of using checkbox elements.
  • The "Select all" option is automatically selected if all other options are set to selected.
  • Aria attributes have been added.

Below are the old vs. updated styles (both Light and Dark theme) for the following scenarios:

  • Component is collapsed and has focus
  • Component is expanded and display box has focus
  • Component is expanded and "South Central US" option has focus
    • "Select All" option has hover
    • "West US" option is not selected

multidropdown-light

multidropdown-dark

@dnfclas
Copy link

dnfclas commented Nov 29, 2017

@andimarc,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

}
// host: {
// '(document:mousedown)': 'onDocumentMouseDown($event)',
// }
Copy link
Contributor

@ammanifold ammanifold Nov 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove? #Resolved

public selectedValues = new ReplaySubject<T[]>(1);
public hasFocus = false;

private focusedIndex = -1;
Copy link
Contributor

@ammanifold ammanifold Nov 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private focusedIndex = -1; [](start = 1, length = 27)

needs to be public #Resolved

<div
role="combobox"
[class.focused]="hasFocus && focusedIndex === -1"
[attr.aria-label]="ariaLabel"
Copy link
Contributor

@ammanifold ammanifold Nov 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"ariaLabel" [](start = 22, length = 11)

variable does not exist #Resolved

// else if (this.opened) {
// this._notifyChangeSubscriptions();
// }
// }
Copy link
Contributor

@ammanifold ammanifold Nov 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove? #Resolved

Copy link
Contributor

@ammanifold ammanifold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🕐

this.options.forEach(o => o.isSelected = false);
}
}
else {
Copy link
Contributor

@ammanifold ammanifold Nov 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: same line #Resolved

Copy link
Contributor

@ammanifold ammanifold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@andimarc andimarc merged commit 7846e23 into dev Dec 4, 2017
@andimarc andimarc deleted the andimarc_multiddfixes branch December 4, 2017 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants