Conversation
There was a problem hiding this comment.
Pull request overview
Adds several student practice files demonstrating basic DOM interactions (hide/show text, button events, random background/quotes) and updates Task 3’s student-processing script to store eligibility on each student object.
Changes:
- Added multiple HTML pages under
Students/Aakriti/for DOM/event-handling exercises. - Implemented a “change background + show quote” mini-project in
task4.html. - Modified
task3.jsto store eligibility directly on each student object (but the PASS/FAIL counting logic is currently broken).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| Students/Aakriti/try.html | Simple visibility toggle demo via buttons |
| Students/Aakriti/task4.html | Random background + quote button interaction |
| Students/Aakriti/task3.js | Updates eligibility handling; attempts to compute pass/fail counts |
| Students/Aakriti/domtask.html | Button click/hover/mouseout behavior demo |
| Students/Aakriti/dom1.html | Random background button demo with “no repeat” attempt |
| Students/Aakriti/dom.html | Inline onclick demo for changing button text/style |
Comments suppressed due to low confidence (1)
Students/Aakriti/task3.js:149
- Fail-count loop also uses
for(let i=0;i<students[i].length;i++), which will throw for the same reason as the pass-count loop. Iteratei < students.lengthand compute FAIL count fromstudents[i].eligibility.
//fail count
let fcount=0;
for(let i=0;i<students[i].length;i++){
if(students.eligibility[i]==="PASS"){
continue;
}
else{
fcount+=1
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for(let i=0; i<colors.length;i--){ | ||
| if(randomColor===lastindex){ | ||
| randomColor=Math.floor(Math.random()*colors.length); | ||
| } | ||
| else{ | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
for(let i=0; i<colors.length; i--) decrements i while the loop condition is i < colors.length, which stays true as i goes negative; if randomColor keeps matching lastindex, this can loop indefinitely. Replace this with a bounded retry (e.g., do...while), a while loop with a max-iterations guard, or adjust the for loop to increment correctly.
| document.body.style.backgroundColor = colors[randomColor]; | ||
|
|
||
| // let randomQuotes=Math.floor(Math.random()*quotes.length) | ||
| lastindex=randomColor; | ||
| const div=document.querySelector('div') | ||
| div.textContent=quotes[lastindex]; | ||
|
|
There was a problem hiding this comment.
The quote selection uses quotes[lastindex], where lastindex is being set from the chosen color index. This couples quote choice to the color array length/order and doesn’t produce a random quote. Generate a separate random quote index (independent of the color index) and use that for div.textContent.
| for (let index = 0; index < colors.length; index--) { | ||
| if(random === lastindex){ | ||
| random = Math.floor(Math.random()*colors.length); | ||
| } | ||
| else{ | ||
| break; | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
for (let index = 0; index < colors.length; index--) decrements index while the condition is index < colors.length, which remains true as index becomes negative; if random keeps equaling lastindex, this can loop indefinitely. Use a do...while retry or fix the loop to increment/bound attempts.
| btn.addEventListener("mouseout",function(){ | ||
| if(btn.textContent === "Click me"){ | ||
|
|
||
| btn.style.backgroundColor = "#007BFF"; | ||
| } | ||
| }) |
There was a problem hiding this comment.
The mouseout handler checks btn.textContent === "Click me", but the button text is set to "Click Me" / "Clicked" elsewhere. This condition never becomes true, so the background may stay red after hover. Align the string comparison (or better, track clicked state and reset on mouseout when not clicked).
| function clicked(){ | ||
| button.textContent="Clicked"; | ||
| button.style.backgroundcolor | ||
| } |
There was a problem hiding this comment.
clicked() references button, but no variable named button is defined, so clicking will throw a ReferenceError. Select the element (e.g., via document.querySelector) or pass the element into the handler (e.g., onclick="clicked(this)") and then update its properties; also backgroundcolor should be backgroundColor and the assignment is currently incomplete.
| for(let i=0;i<students[i].length;i++){ | ||
|
|
||
| if(eligibility[i]==="PASS"){ | ||
| if(students.eligibility[i]==="PASS"){ | ||
| pcount+=1 | ||
| } | ||
| } | ||
| console.log("The pass count in letslearn is: ",pcount) | ||
|
|
||
| //fail count | ||
| let fcount=0; | ||
| for(let i=0;i<eligibility.length;i++){ | ||
| for(let i=0;i<students[i].length;i++){ | ||
|
|
||
| if(eligibility[i]==="PASS"){ | ||
| if(students.eligibility[i]==="PASS"){ | ||
| continue; | ||
| } |
There was a problem hiding this comment.
students.eligibility[i] is incorrect because students is an array; the eligibility property is on each student object. Use students[i].eligibility (and similarly in the fail-count loop) so the PASS/FAIL counts reflect the values set earlier.
| button.addEventListener("click",function(){ | ||
| button.textContent="clicked"; | ||
| let lastindex=0; | ||
| let randomColor=Math.floor(Math.random()*colors.length); | ||
| for(let i=0; i<colors.length;i--){ | ||
| if(randomColor===lastindex){ |
There was a problem hiding this comment.
lastindex is declared inside the click handler (let lastindex = 0;), so it resets on every click and can’t prevent repeating the same color across clicks. Declare the last-used color index outside the handler (persist between clicks) and update it after choosing a new color.
No description provided.