-
Notifications
You must be signed in to change notification settings - Fork 39
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
Courses: Fix Exam error messages #6752
base: master
Are you sure you want to change the base?
Conversation
@paulbert The "You must add a choice" error message does pop up when a choice is added and then removed. Also, I was unable to get that error message to pop up on text questions, it only showed up on multiple choice questions. I think the reason the main "Some required fields are missing error" won't go away dynamically is because it's based of a boolean in ExamsAddComponent that only changes when the form is trying to be submitted. I'm not sure how to fix that issue. I'm not sure if you could have that variable sent to ExamsQuestionComponent, and then emit its value on change back to ExamsAddComponent. Any guidance would be appreciated. |
@paulbert My most recent commit is the idea I have to get the "Some required fields are missing" in ExamsAddComponent to automatically go away when the choices in ExamsQuestionComponent are valid. I'm not sure why it's not working. Any guidance would be appreciated. |
Two things:
|
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.
In addition to the note below, it would be better to keep showFormError
local to ExamsAddComponent only.
Using statusChanges
on the form in ExamsAddComponent would be preferred.
I think the statusChanges
is needed for the choices, but you can have a local showChoicesError
on the ExamsQuestionComponent instead. The other condition in the *ngIf
for the choices error can be removed because it is not working.
@@ -59,6 +60,11 @@ export class ExamsQuestionComponent implements OnInit, OnChanges, AfterViewCheck | |||
this.choiceAdded = false; | |||
this.cdRef.detectChanges(); | |||
} | |||
this.choices.statusChanges.subscribe((data) => { | |||
if (data) { | |||
this.choices.valid ? this.showFormError = false : this.showFormError = true; |
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.
You shouldn't run statements as clauses with a ternary operator. It makes it more difficult to read.
In this case you can pretty easily invert this so it's an assignment to this.showFormError
based on this.choices.valid
.
@paulbert When you are talking about keeping showFormError local to ExamsAddComponent, and showChoicesError local to ExamsQuestionComponent, are you saying that those two properties shouldn't be bound to each other? Also, do I need to put Also if you want me to put it into |
@jsschf Yes. The 2-way binding is incomplete in this implementation, anyway, but ExamsQuestionComponent should not be passing its show error value back up to ExamsAddComponent and it's not necessary to pass the value down.
I'm seeing the error message show up immediately when adding a question. In |
Fixes #3784