Skip to content

Conversation

@Iswanna
Copy link

@Iswanna Iswanna commented Nov 20, 2025

Learners, PR Template

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Changelist

In this pull request, I completed the quote generator project by implementing the random quote functionality. The app now displays a random quote and its author when the page loads.

Questions

Hi. Please could you review my PR? I’d really appreciate your feedback.

@Iswanna Iswanna added 🏕 Priority Mandatory This work is expected 📅 Sprint 3 Assigned during Sprint 3 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Data-Groups The name of the module. labels Nov 20, 2025
Downgraded @testing-library/user-event from v14 to v13.5.0 because version 14 introduces asynchronous userEvent handlers, causing the existing synchronous Jest tests to fail.
Version 13 matches the behaviour the tests were originally written for, allowing all tests to pass again.

// call pickFromArray with the quotes array to check you get a random quote
// Function to display a random quote on the page
function displayRandomQuote() {

Choose a reason for hiding this comment

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

Good job keeping displayRandomQuote() focused on a single responsibility — fetching a quote and updating the DOM. This helps keep the logic readable and easy to maintain.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you so much, @jaymes15.

}

// Add a click event listener to the "New quote" button
document.getElementById("new-quote").addEventListener("click", () => {

Choose a reason for hiding this comment

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

This is good! but we can make it cleaner by using a named function.
for example:
document.getElementById("new-quote").addEventListener("click", displayRandomQuote);

Copy link

@jaymes15 jaymes15 left a comment

Choose a reason for hiding this comment

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

Nice work — your solution is clean and readable

@jaymes15 jaymes15 added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Nov 22, 2025
@Iswanna
Copy link
Author

Iswanna commented Nov 22, 2025

Nice work — your solution is clean and readable

Thank you so much for the feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. Module-Data-Groups The name of the module. 🏕 Priority Mandatory This work is expected 📅 Sprint 3 Assigned during Sprint 3 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants