fix(checkbox): re-evaluate label visibility when label is updated#30980
fix(checkbox): re-evaluate label visibility when label is updated#30980OS-jacobbell wants to merge 1 commit intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
bot
added
package: core
ShaneK
left a comment
ShaneK
left a comment
There was a problem hiding this comment.
Looking good! Just a couple of change requests
| onClick={this.onDivLabelClick} | ||
| > | ||
| <slot></slot> | ||
| <slot onSlotchange={() => forceUpdate(this)}></slot> |
There was a problem hiding this comment.
Generally, we should avoid using inline anonymous functions in templates. Doing this both creates a new anonymous function on every render and can make debugging trickier (anonymous functions are harder to find in stack traces than named functions).
It would be better to declare an onSlotChange function in the component and use it here.
| export class CheckboxComponent { | ||
| dynamicLabel = ''; | ||
|
|
||
| ngAfterViewChecked(): void { |
There was a problem hiding this comment.
| ngAfterViewChecked(): void { | |
| ngAfterViewInit(): void { |
This is a test and normally I wouldn't make change requests to tests that are just kinda minor architecture changes, but ngAfterViewChecked runs after every change detection cycle and is meant for reading, not writing. I'm concerned this test may be a bad example for our users to follow if they reference it.
2 participants
What is the current behavior?
Checkbox's render function applies the
label-text-wrapper-hiddencss class when there is no label text to prevent extra margin from being added. The render function is not re-evaluated if the label is updated. This causes a problem in Angular where dynamic variables get applied after the page is loaded, and a checkbox using a variable as a label gets stuck with its label hidden until something else triggers a re-render, e.g. ticking the box.What is the new behavior?
Does this introduce a breaking change?