Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

select(multiple): checkboxes do not appear when using ng-multiple="true" #11759

Closed
Splaktar opened this issue Jul 5, 2019 · 2 comments
Closed
Assignees
Labels
for: internal contributor The team will address this issue and community PRs are not requested. P3: important Important issues that really should be fixed when possible. type: bug type: docs ux: polish

Comments

@Splaktar
Copy link
Member

Splaktar commented Jul 5, 2019

Bug

CodePen and steps to reproduce the issue:

CodePen Demo which demonstrates the issue:

https://codepen.io/Splaktar/pen/YoOEMm?editors=1010#0

Detailed Reproduction Steps:

  1. Open the select menu
  2. Note that there are no checkboxes, but you can select multiple options

What is the expected behavior?

Checkboxes appear for multiple select options.

What is the current behavior?

PR #7455 added checkboxes for options in a multiple select, but it did not account for the ng-multiple case which is undocumented and has no tests.

What is the use-case or motivation for changing an existing behavior?

Consistency. If you use multiple you get checkboxes, if you use ng-multiple="true" or some other truthy expression, you get no checkboxes.

Which versions of AngularJS, Material, OS, and browsers are affected?

  • AngularJS: 1.5.5+
  • AngularJS Material: 1.1.0-1.1.19
  • OS: macOS
  • Browsers: Chrome 75

Is there anything else we should know? Stack Traces, Screenshots, etc.

multiple:

Screen Shot 2019-07-05 at 18 07 53

ng-multiple="true":

Screen Shot 2019-07-05 at 18 07 27

I ran into this inconsistency while working on a11y issues: #10967 and #10748.

@Splaktar Splaktar changed the title select(multiple) select(multiple): checkboxes do not appear when using ng-multiple="true" Jul 5, 2019
@Splaktar Splaktar self-assigned this Jul 5, 2019
@Splaktar Splaktar added P3: important Important issues that really should be fixed when possible. type: bug for: internal contributor The team will address this issue and community PRs are not requested. needs: unit tests This PR needs unit tests to cover the changes being proposed ux: polish labels Jul 5, 2019
@Splaktar Splaktar added this to the 1.1.20 milestone Jul 5, 2019
@Splaktar Splaktar modified the milestones: 1.1.20, 1.1.21 Aug 5, 2019
@Splaktar Splaktar modified the milestones: 1.1.21, 1.1.22 Aug 15, 2019
@Splaktar Splaktar modified the milestones: 1.1.22, 1.1.23 Oct 22, 2019
@marosoft
Copy link
Contributor

marosoft commented Nov 3, 2019

@Splaktar isn't it related to #9705?

@Splaktar
Copy link
Member Author

Splaktar commented Nov 3, 2019

@marosoft yes, I forgot about that one, thank you! I'll close this one out as that issue lays out why ng-multiple support is not documented or tested (it was removed from AngularJS in 1.2.0). It makes sense to leave that one planned for 1.2.0 of AngularJS Material since it's a breaking change.

@Splaktar Splaktar closed this as completed Nov 3, 2019
@Splaktar Splaktar removed this from the 1.1.23 milestone Nov 3, 2019
@Splaktar Splaktar removed the needs: unit tests This PR needs unit tests to cover the changes being proposed label Nov 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
for: internal contributor The team will address this issue and community PRs are not requested. P3: important Important issues that really should be fixed when possible. type: bug type: docs ux: polish
Projects
None yet
Development

No branches or pull requests

2 participants