Skip to content

fix(material/list): add radio toggles #25933

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

Merged
merged 1 commit into from
Nov 22, 2022
Merged

Conversation

zarend
Copy link
Contributor

@zarend zarend commented Nov 7, 2022

fix(material/list): add radio toggles

Add radio toggles for single selection. Fix a11y issue where selected
state is visually communicated with color alone.

Rename checkboxPosition Input to togglePosition and deprecate
checkboxPosition. togglePosition configures the position of both the
radio and checkbox indicators. checkboxPosition also configures the
position of both.

Summary of API and behavior changes:

  • MDC List displays radio indicators for single-selection
  • rename checkboxPosition Input to togglePosition
  • rename type MatListOptionCheckboxPosition to type MatListOptionTogglePosition

DEPRECTED:

  • checkboxPosition is deprecated because togglePosition replaces it
  • MatListOptionCheckboxPosition is deprecated because
    MatListOptionTogglePosition replaces it

Closes #7157, Fixes #25900

@zarend zarend added Accessibility This issue is related to accessibility (a11y) area: material/list target: rc This PR is targeted for the next release-candidate labels Nov 7, 2022
@zarend zarend force-pushed the mdc-list-radio branch 2 times, most recently from 2683a4f to 18ea893 Compare November 7, 2022 17:34
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM, nice change!

@zarend zarend force-pushed the mdc-list-radio branch 6 times, most recently from 8e892c4 to 8072226 Compare November 9, 2022 04:01
@zarend
Copy link
Contributor Author

zarend commented Nov 9, 2022

Hi all, I've made fixes to respond to PR comments

  • update the code comments: say checkbox/radio or toggle instead of just checkbox
  • delete MatListOptionCheckboxPosition and @Input checkboxPosition, which are replaced by MatListOptionTogglePosition and @Input togglePosition
  • add getRadioPosition method to test harness

@crisbeto does this PR look good to you?

@zarend zarend added target: minor This PR is targeted for the next minor release and removed target: rc This PR is targeted for the next release-candidate labels Nov 9, 2022
@zarend
Copy link
Contributor Author

zarend commented Nov 9, 2022

I've made fixed based on PR comments and changed the target to minor:

  • checkboxPosition is deprecated because togglePosition replaces it
  • MatListOptionCheckboxPosition is deprecated because MatListOptionTogglePosition replaces it
  • export MatListOptionCheckboxPosition using the export as syntax 👌

@crisbeto @mmalerba this is ready for your eyes again 👀

@zarend zarend requested review from crisbeto and mmalerba November 9, 2022 21:55
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM

@mmalerba mmalerba added the action: merge The PR is ready for merge by the caretaker label Nov 9, 2022
@zarend zarend removed the action: merge The PR is ready for merge by the caretaker label Nov 17, 2022
Add radio toggles for single selection. Fix a11y issue where selected
state is visually communicated with color alone.

Rename `checkboxPosition` Input to `togglePosition` and deprecate
`checkboxPosition`. `togglePosition` configures the position of both the
radio and checkbox indicators. `checkboxPosition` also configures the
position of both.

Summary of API and behavior changes:
 - MDC List displays radio indicators for single-selection
 - rename `checkboxPosition` Input to `togglePosition`
 - rename `type MatListOptionCheckboxPosition` to `type
   MatListOptionTogglePosition`

DEPRECTED:
 * `checkboxPosition` is deprecated because `togglePosition` replaces it
 * `MatListOptionCheckboxPosition` is deprecated because
   `MatListOptionTogglePosition` replaces it

Closes angular#7157, Fixes angular#25900
@zarend
Copy link
Contributor Author

zarend commented Nov 17, 2022

changes since this was last reviewed:

  • fixed regression in radio dark mode because MDC's global variables were not configured correctly
  • consolidated calls to using-mdc-theme

@zarend zarend added the action: merge The PR is ready for merge by the caretaker label Nov 22, 2022
@zarend zarend merged commit 57676e4 into angular:main Nov 22, 2022
zarend added a commit to zarend/components that referenced this pull request Dec 16, 2022
Add an opt-out for radio indicators for single-selection. Adds both an
Input and DI token to specify if radio indicators are hidden. By
default, display radio indicators. If both DI token and Input are
specified, the Input wins.

PR angular#25933 added radio toggles for single-selection. Add an opt-out to
provide a way to have same appearance as before angular#25933.

API changes
 - add `@Input hideSingleSelectionIndicator` to specify if radio
   indicators are displayed
 - add MAT_LIST_CONFIG token to specify default value for
   `hideSingleSelectionIndicator`
zarend added a commit to zarend/components that referenced this pull request Dec 16, 2022
Add an opt-out for radio indicators for single-selection. Adds both an
Input and DI token to specify if radio indicators are hidden. By
default, display radio indicators. If both DI token and Input are
specified, the Input wins.

PR angular#25933 added radio toggles for single-selection. Add an opt-out to
provide a way to have same appearance as before angular#25933.

API changes
 - add `@Input hideSingleSelectionIndicator` to specify if radio
   indicators are displayed
 - add MAT_LIST_CONFIG token to specify default value for
   `hideSingleSelectionIndicator`
zarend added a commit that referenced this pull request Dec 20, 2022
Add an opt-out for radio indicators for single-selection. Adds both an
Input and DI token to specify if radio indicators are hidden. By
default, display radio indicators. If both DI token and Input are
specified, the Input wins.

PR #25933 added radio toggles for single-selection. Add an opt-out to
provide a way to have same appearance as before #25933.

API changes
 - add `@Input hideSingleSelectionIndicator` to specify if radio
   indicators are displayed
 - add MAT_LIST_CONFIG token to specify default value for
   `hideSingleSelectionIndicator`
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Accessibility This issue is related to accessibility (a11y) action: merge The PR is ready for merge by the caretaker area: material/list target: minor This PR is targeted for the next minor release
Projects
None yet
4 participants