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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Answer: 50: Bug in effect solution #856

Closed
wants to merge 1 commit into from
Closed

Answer: 50: Bug in effect solution #856

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 18 additions & 1 deletion apps/angular/bug-effect-signal/src/app/app.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,24 @@ export class AppComponent {

constructor() {
effect(() => {
if (this.name() || this.age() || this.address()) {
/*
This effect executes only once when name is true, because the value of name doesn't change.
Effects run when a signal inside changes, since name value is the same as before, the effect doesn't trigger.
If we click on age first, the effect will run again after click on name.
Because there is no signal value which has the same value as before.
*/
// if (this.name() || this.age() || this.address()) {
// alert('Checkbox was checked');
// }
/*
This effect should run if any of these 3 values is true, you have to define those values.
With this all signal values are interpreted as relevant values for this effect.
So if any of these values changes, the effect will run.
*/
const name = this.name();
Copy link
Contributor

@LMFinney LMFinney May 8, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this implementation, the alert will be shown when you have 2 or 3 of them checked, and then you uncheck one (because name || age || address will still be true if any of them are true). I think that's not what @tomalaforge wants. I think he wants the alert to show when each checkbox is checked, independent of the state of any other, but not as the result of unchecking.

Thomas can clarify, of course.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but i wasn't really sure what is part of the challenge and what's not. This is the simplest solution without changing the code a lot. As thomas already mentioned, this could be solved with a computed signal as well. I already have some things in mind, i'll update the docs for this challenge.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need clarification on the requirements.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LMFinney you are right, but I didn't go that far 馃槄
My goal was to trigger the effect when we check or uncheck one checkbox which is not the case if the first one is true. But I didn't think of your case @LMFinney; So I should change the instruction to be clearer indeed 馃憤

const age = this.age();
const address = this.address();
if (name || age || address) {
alert('Checkbox was checked');
}
});
Expand Down