Skip to content
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

[Issue #9631] fix radiobutton to support model-driven form with formControl #9632

Closed
wants to merge 2 commits into from

Conversation

lisuizhe
Copy link

@lisuizhe lisuizhe commented Dec 9, 2020

###Defect Fixes
Fixed: Radio buttons do not get unselected on the same group, with Reactive Forms using formControl instread of formControlName #9631

@sergeypayu
Copy link
Contributor

I wonder why the b823983 commit was needed. Angular uses internal registry for all radio buttons without separating model-driven from template-driven approaches. I don't see any downsides of always using the registry.

Also it's kinda sad that it took 5 weeks to merge my fix and in the end radio buttons are still broken because of the applied "refactoring".

@loxy
Copy link

loxy commented Feb 3, 2021

This is not really working for me (using Formly), because this.formControl is also undefined. It should be something like this:

ngOnInit() {
    const control = this.injector.get(NgControl);
    if (control) {
        this.control = control;
        this.checkName();
        this.registry.add(this.control, this);
    }
}
...
select(event) {
    if (!this.disabled) {
        this.inputViewChild.nativeElement.checked = true;
        this.checked = true;
        this.onModelChange(this.value);
        if (this.control) {
            this.registry.select(this);
        }
        this.onClick.emit(event);
    }
}

@yigitfindikli
Copy link
Contributor

Hi @lisuizhe,

Thank you for your work. But will be added manually.

Best Regards.

@lisuizhe
Copy link
Author

@yigitfindikli Thanks! Appreciate your effort to fix this issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants