material-components / material-components-web

Modular and customizable Material Design UI components for the web
https://material.io/develop/web
MIT License
17.11k stars 2.15k forks source link

[MDCList] Checkboxes do not toggle anymore #7618

Open henkpb opened 2 years ago

henkpb commented 2 years ago

Bug report

In version 14, clicking a list item or a checkbox in a list with checkbox items does not toggle the checkbox, if the list is initialized with the MDCList JavaScript component.

Steps to reproduce

  1. Use the code example "List with checkbox items" shown in the README of the list component
  2. Initialize the list using const list = new MDCList(document.querySelector('.mdc-deprecated-list'));

Actual behavior

Clicking a list item does not change the checked-state of the checkbox. Neither does clicking the checkbox.

Expected behavior

Clicking a list item changes the checked-state of the checkbox. So should clicking the checkbox itself.

Your Environment:

Software Version(s)
MDC Web 14.0

Possible solution

Debugging the code reveals that the state of the checkbox actually is changed twice. From 'unchecked' to 'checked' and back to 'unchecked' again. Or from 'checked' to 'unchecked' and back to 'checked' again.

It looks like the cause of this problem was introduced in the commit of list\foundation.js on December 10, 2021. Then, the name and the meaning of the second parameter in function handleClick in list\foundation.js was changed. From toggleCheckbox ("please toggle the checkbox") to isCheckboxAlreadyUpdatedInAdapter ("do not toggle the checkbox, because it already has been toggled").

The caller function MDCList.prototype.handleClickEvent in list\component.js is unaware of this commit, as the comment in this function indicates (there, the parameter still is called toggleCheckbox). Changing this line of code: this.foundation.handleClick(index, toggleCheckbox, evt); to: this.foundation.handleClick(index, !toggleCheckbox, evt); causes the checkboxes to work as expected.

youbet1980 commented 2 years ago

I too have had this problem. Only solution I can find is to put them inside the mdc-form-field and initiate each checkbox like this.

XX commented 1 year ago

Why hasn't anyone fixed it?

anentropic commented 1 week ago

can confirm the fix above seems to work ok

current location of the code that needs patching is: https://github.com/material-components/material-components-web/blob/v14.0.0/packages/mdc-list/component.ts#L420-L427