fix(list): added #adapter.listItemAtIndexHasClass to prevent user state change to disabled items#4922
Conversation
…ion from changing the list item state
|
There are 2 different paths of selecting a listitem (w/ checkbox/radio/none) -
Pre-PR, MDC-List didn't allow any disabled checkboxes or radio buttons to be selected. |
Codecov Report
@@ Coverage Diff @@
## develop #4922 +/- ##
===========================================
- Coverage 98.67% 98.65% -0.02%
===========================================
Files 122 122
Lines 6032 6036 +4
Branches 796 797 +1
===========================================
+ Hits 5952 5955 +3
- Misses 79 80 +1
Partials 1 1
Continue to review full report at Codecov.
|
| */ | ||
| isRootFocused(): boolean; | ||
|
|
||
| /** |
There was a problem hiding this comment.
You also need to sync this to internal and implement this adapter before merging it.
abhiomkar
left a comment
abhiomkar
left a comment
There was a problem hiding this comment.
@allan-chen LGTM! minor comment.
| .${cssClasses.LIST_ITEM_CLASS} input[type="checkbox"]:not(:disabled) | ||
| `, | ||
| RADIO_SELECTOR: 'input[type="radio"]:not(:disabled)', | ||
| RADIO_SELECTOR: 'input[type="radio"]', |
There was a problem hiding this comment.
This began as an effort for better naming consistency but actually fixes a bug where if the first radio element of a list is disabled, the remaining radio list breaks. This was due to the following lines in layout() of foundation.ts not realizing a disabled radio at index 0 also makes the whole list a radio list, as a result of the constant above.
...
else if (this.adapter_.hasRadioAtIndex(0)) {
this.isRadioList_ = true;
}
| .${cssClasses.LIST_ITEM_CLASS} a | ||
| `, | ||
| ENABLED_CHECKBOX_RADIO_SELECTOR: 'input[type="checkbox"]:not(:disabled), input[type="radio"]:not(:disabled)', | ||
| ENABLED_CHECKBOX_SELECTOR: 'input[type="checkbox"]:not(:disabled)', |
There was a problem hiding this comment.
These two ENABLED_* constants are not used anywhere at this point. They are essentially renamed versions of the old CHECKBOX_RADIO_SELECTOR and CHECKBOX_SELECTOR before this PR, which were too restrictive in terms of what constituted a valid checkbox/radio to be toggled programmatically. Per Matt's philosophy that all programmatic changes to disabled checkboxes are allowed, I think we can remove these constants?
There was a problem hiding this comment.
We can remove if unused.
|
🤖 Beep boop! Screenshot test report 🚦6 screenshots changed from |
|
🤖 Beep boop! Screenshot test report 🚦6 screenshots changed from |
|
🤖 Beep boop! Screenshot test report 🚦6 screenshots changed from |
|
Make sure you add the breaking change notice to the merge request. |
allan-chen
changed the title
7 participants
fixes #4696
BREAKING CHANGE: New adapter method
listItemAtIndexHasClass