feat(checkbox): Add initial checkbox implementation #4638
Conversation
| # be disabled for that file, with an explanation. | ||
| selector-no-type: | ||
| - true | ||
| - |
There was a problem hiding this comment.
Nit: Why the extra empty - here?
There was a problem hiding this comment.
my bad 😅 there's an object definition which is indented so it's just another array entry. I'll change it to make it on the same line though.
|
Style-wise and current docs LGTM overall. Will play around with implementing and using JS triggers a bit to see how that feels before full approval. |
| <button type="button" onClick="document.getElementById('basic-checkbox').indeterminate = true;">Make indeterminate</button> | ||
| <button type="button" onClick="this.parentElement.hasAttribute('dir') ? this.parentElement.removeAttribute('dir') : this.parentElement.setAttribute('dir', 'rtl');">Toggle RTL</button> | ||
| <button type="button" onClick="document.querySelector('.mdl-checkbox-wrapper').classList.toggle('mdl-checkbox-wrapper--align-end');">Toggle Align End</button> | ||
| <input type="checkbox" id="disable-element-basic" onchange="document.getElementById('basic-checkbox').disabled = this.checked;"> |
There was a problem hiding this comment.
Why don't we actually make this use the checkbox component itself instead of using a generic checkbox?
There was a problem hiding this comment.
hmm my thinking was that using another checkbox component might detract from the component that's being demo'd. e.g. does "disable" apply to that checkbox component or the one for the original demo? I don't feel too strongly either way about it though.
There was a problem hiding this comment.
It is a bit inceptiony. But, it also shows in an easy demo how a developer can handle events in an actual application of the component.
In lieu of a checkbox we could just have a "Toggle Disabled" button which does e => e.target.disabled = !e.target.disabled; on click. That way all the toggles are buttons vs this one checkbox node.
There was a problem hiding this comment.
The toggle button makes sense; I'll add that in.
traviskaufman
force-pushed
the
feat/checkbox
branch
from
59a4892
to
626e56e
Compare
traviskaufman
force-pushed
the
feat/checkbox
branch
from
626e56e
to
b5e832d
Compare
|
Changes made and fix is in for tests; apparently property descriptors aren't even returned within the android browser. |
This commit converts the checkbox POC code over to the new architecture, adds unit tests, and ports the demos over to working implementations. It also adds a README, and modifies .stylelintrc.yaml to increase the number of compound selectors we can use. Note that ink ripple functionality will be covered in a separate issue. Finishes #4471 [Delivers #126819487]
traviskaufman
force-pushed
the
feat/checkbox
branch
from
b5e832d
to
b6a0fc1
Compare
|
@Garbee changes made PTAL! |
|
Schweet. Just played around with the native setup with toggling in custom JS. Works great. LGTM. |
This commit converts the checkbox POC code over to the new architecture, adds unit tests, and ports
the demos over to working implementations. It also adds a README, and
modifies .stylelintrc.yaml to increase the number of compound selectors
we can use.
Note that ink ripple functionality will be covered in a separate issue.
Finishes #4471
[Delivers #126819487]