Answer:50 bug effect signal #864
Conversation
github-actions
bot
added
50
| // The problem with the previous implementation is that `||` is | ||
| // short-circuiting; if the first signal is true, the second and third | ||
| // signals are never checked, so we don't get the desired effect on | ||
| // subsequent checkboxes. |
There was a problem hiding this comment.
That's what i struggled with too. In my opinion the goal of this challenge is to understand why this effect works, how it works. And from a user perspective, the alert should be displayed when a checkbox is about to become checked. But that doesn't matter because this challenge is about this particular bug.
There was a problem hiding this comment.
Your solution definitely fixed the effects being short-circuited. But then it also had the alert showing in some cases when a checkbox is unchecked. I'm not sure if that's acceptable or a different bug.
There was a problem hiding this comment.
If you mean the initial case, so no checkbox is checked, and then check one. From my perspective this is the case where the alert should display. And if you uncheck the last checked checkbox, there is no alert as well. That's how it worked in my solution.
There was a problem hiding this comment.
Let's imagine this sequence:
- Check box 1
- Uncheck box 1
- Check box 1
- Check box 2
- Check box 3
- Uncheck box 1
- Check box 1
My solution will show the alert for steps 1, 3, 4, 5, and 7. I believe your solution would also show the alert for step 6, because the expression would be true at that point. Maybe that's the intention, but it doesn't make sense to me.
I think the original would show the alert for steps 1, 3, and 7.
There was a problem hiding this comment.
I think this was unintended.
Yes, the variable solution will display an alert anytime there is one true value. All the values are tracked.
| effect(() => { | ||
| if (this.name() || this.age() || this.address()) { | ||
| if (this.name()) { |
There was a problem hiding this comment.
In the solution from @jdegand is talked about this too. I don't think it's a good idea to add 3 different effect's for this case. They all do the same stuff, show the alert if one of these values is true.
There was a problem hiding this comment.
@LMFinney has a point with the short-circuiting. All it takes is one true for the alert to run. @svenson95 your solution is valid and correct, but because of the short-circuiting here -> multiple effects is the correct answer for the way the challenge is presently constructed.
|
ohhhh 馃槄 you are going too far 馃槀. I didn't think at all this use cases at all. My goal was just to understand why the effect is not called when the first signal is true. Maybe the message in the alert should be different. |
No problem :) Since this was released just yesterday, I'll count myself as just a beta user ;) Perhaps a sequence description like what I used in #864 (comment) would be good to show what the acceptance criteria should be. I'm curious if there's a way to match the behavior of my implementation with a single effect. I don't think there is. |
Or to make thing simpler, just show the alert if at least one checkbox is checked. But the alert should be displayed at each click if at least one checkbox is clicked. So we can keep one effect, or multiple effect or a computed. My goal is not about the use case but about the undesirable side-effect |
For that simple description, show the alert on both checking or unchecking if at least one of the three is checked? Yeah, that would work :) |
|
Maybe it's easier to understand, if we use a better example. Imagine you try to buy a laptop, and each checkbox is a extra which can be added - more drive space, ram, better gpu. If one checkbox gets checked, the alert should display "Price increased". If you deselect all checkboxes, no alert should be displayed, since all extras are unselected. |
I like that. A good motivated example like that would really help. |
|
This pull request is stale because it has been open for 15 days with no activity. |
Checklist for challenge submission
Warning:
Alternatively, you can still submit your PR to join the list of answered challenges or to be reviewed by a community member. 馃敟