-
Notifications
You must be signed in to change notification settings - Fork 15
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
feat(checkbox): refactor checkbox component #409
Conversation
Run & review this pull request in StackBlitz Codeflow. |
…-oss/forge into checkbox-next
…-oss/forge into checkbox-next
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Things are looking good, just a couple questions/comments:
When a slotted <input>
is used, can we have it automatically add the slot="input"
attribute when initializing the adapter if one doesn't exist? This makes it easier to use, but on top of that we can improve performance because it can immediately initialize against the slotted <input>
instead as well regardless:
const slottedInput = this._component.querySelector('input[type=checkbox]:not([slot])') as HTMLInputElement;
if (slottedInput) {
slottedInput.slot = 'input';
}
Additionally, I know you're probably waiting until the end, but we should update the delegate as well included tests for anything that is missing to improve coverage.
🚀 PR was released in |
PR Checklist
Please check if your PR fulfills the following requirements:
Describe the new behavior?
Removed Material dependencies from the checkbox component, added a toggle method, added tokens and new animations, set up element internals, refined the base form interface/class and forwarding/reflection utilities, and fixed related bugs around the state layer, focus indicator, and switch components.
Closes #370