components icon indicating copy to clipboard operation
components copied to clipboard

fix(material/core): add checkmark for single-select

Open zarend opened this issue 1 year ago • 3 comments

Add checkmark to mat-option for single-select. Fix a11y issues where selected state is visually communicated with color alone. Communicate selection with both color and a checkmark indicator. Affect components that use mat-option for single-selection, which include select and autocomplete.

Add an appearance Input to mat-pseudo-checkbox. "full" appearance renders a checkbox, which is the current behavior. Render "full" appearance by default. "minimal" apperance renders only a checkmark.

Summary of API and behavior changes:

  • Add an @Input appearance to pseudo-checkbox with options for "full" and "minimal".
  • mat-option renders "minimal" appearance for single-select, which looks like a checkmark.
  • Select and autocomplete components display checkmark on selected option.

What's not changing:

  • does not affect multiple selection
  • mat-option renders "full" apperance by default, which is same as existing behavior

Fixes: #25961

image image image

zarend avatar Nov 11 '22 22:11 zarend

rebased with upstream/main. Maybe this will fix the issue with dev-app deployment 🤞

zarend avatar Nov 17 '22 17:11 zarend

Deployed dev-app to: https://ng-dev-previews-comp--pr-angular-components-25962-0077-q1n3hdgg.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

github-actions[bot] avatar Nov 17 '22 17:11 github-actions[bot]

image The autocomplete theming fix has landed. I rebased this branch, and we can see that the colors are corrected.

zarend avatar Nov 18 '22 16:11 zarend

@mmalerba @crisbeto could you review this again please as it has changed. Since last reviewed, added opt-outs for Select and Autocomplete.

I have a few questions:

  • Do we need an opt-out for when used outside of select or autocomplete? As implemented, the opt-out only affect mat-option inside of a Select or Autocomplete. For ripple, there's a way to disable it from the Select/Autocomplete and from the mat-option too. Should hideSingleSelectionIndicator do the same?
  • Could you take a look at _syncParentProperties? I ran into an issue where autocomplete wouldn't work when testing on the autocomplete overview demo (/autocomplete). Clicking the "Hide Single-Selection Indicators" didn't seem to do anything. It seemed like change detection wasn't running because we mutated the _parent property on MatOption and it uses on-push change detection. I couldn't repro on Select, but I did the same fix for Select anyways.
  • For MatAutocomplete, would it be better to this another way, like assigning defaults to the base class, or using inject function? That way we don't have the change the constructor of the derived class?

zarend avatar Jan 05 '23 17:01 zarend

Since this was last reviewed, changed out it puts the checkmark on the right of the mat option. Using margin to make space between the checkmark and the list item instead of using flex-grow: 1;. Increasing the size of the list item content causes unintended layout changes.

The only file I touched was option.scss, in case anyone wants to see what changed.

zarend avatar Jan 11 '23 23:01 zarend

This issue has been automatically locked due to inactivity. Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.