Conversation
…radio buttons with labels, styled quiz-settiong view
| button.addEventListener('click', () => { | ||
| if (what=="about"){ | ||
| this.quizAbout=val; | ||
| } | ||
| if (what=="type"){ | ||
| this.questionsType=val; | ||
| } | ||
|
|
||
| }); |
There was a problem hiding this comment.
Extract handler to separate function to improve readability. Use strict equality.
| } | ||
|
|
||
| }); | ||
| return form; |
| const about = document.createElement('div'); | ||
| const text = document.createElement('p'); | ||
| text.innerText='Quiz about: ' | ||
| about.append(text,this.createRadioButton("cats","about","about","left"), this.createRadioButton("dogs","about","about","right")); |
There was a problem hiding this comment.
There are too many arguments. Check the first comment.
| questionsNum; | ||
| questionsType; | ||
|
|
||
| static createRadioButton(val,nameOfRadio,what,side){ |
There was a problem hiding this comment.
There are too many parameters. They are also so unintuitive.
val - replace with value;
nameOfRadio - I would change it to settingName;
what - what is "what" :P It can be removed. Pass event to the handler and use event.currentTarget.name to check which setting is clicked
side - currently, it is working fine, but this would be hard to add more buttons without rebuilding it
| const questionsNumberdiv = document.createElement('div'); | ||
| const text = document.createElement('p'); | ||
| text.innerText='Questions number: ' | ||
| questionsNumberdiv.append(text, this.createQuestionsNumberInput()); | ||
| return questionsNumberdiv; |
There was a problem hiding this comment.
questionsNumberdiv - be consistent, change to questionsNumberDiv
| const questionsType = document.createElement('div'); | ||
| const text = document.createElement('p'); | ||
| text.innerText='Questions type: ' | ||
| questionsType.append(text, this.createRadioButton("open","type","type","left"), this.createRadioButton("multiple choice","type","type","right")); |
| component.addEventListener(event, () => { | ||
| if (settingName === 'quizAbout') { | ||
| this.quizAbout = value; | ||
| } | ||
| if (settingName === 'questionsType') { | ||
| this.questionsType = value; | ||
| } | ||
| if (settingName === 'questionsNum') { | ||
| this.questionsNum = component.value; | ||
| } | ||
| console.log(this.quizAbout, this.questionsType, this.questionsNum); | ||
| }); |
There was a problem hiding this comment.
Keep in mind, that
we can pass a function to the addEventListener, so
() => {
if (settingName === 'quizAbout') {
this.quizAbout = value;
}
if (settingName === 'questionsType') {
this.questionsType = value;
}
if (settingName === 'questionsNum') {
this.questionsNum = component.value;
}
console.log(this.quizAbout, this.questionsType, this.questionsNum);
}
could've been extracted
| form.addEventListener('submit', (e) => { | ||
| e.preventDefault(); | ||
| if (this.questionsNum === undefined || this.questionsNum < 1 || this.questionsNum > 20) { | ||
| alert('Insert questions number between 1 and 20'); | ||
| } else if (this.quizAbout === undefined) { | ||
| alert('Choose animals'); | ||
| } else if (this.quizAbout === undefined) { | ||
| alert('Choose questionsType'); | ||
| } else { | ||
| alert( | ||
| 'Quiz about: ' + | ||
| this.quizAbout + | ||
| '\nQuestions number: ' + | ||
| this.questionsNum + | ||
| '\nQuestions type: ' + | ||
| this.questionsType, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Same as above:
form.addEventListener('submit', (e) => this.validateForm(e));
and rest moved to this validate form function.
Nefariusek
left a comment
Nefariusek
left a comment
There was a problem hiding this comment.
There are areas for improvement. I left some comments. You can either fix it or merge it
Nefariusek
left a comment
Nefariusek
left a comment
There was a problem hiding this comment.
There are areas for improvement. I left some comments. You can either fix it or merge it
2 participants
Full view with validation is ready, settings shown in an alert.