Skip to content

London | 26-ITP-May | Alex Jamshidi | Sprint 3 | Quote Generator App #1229

Open
Alex-Jamshidi wants to merge 2 commits into
CodeYourFuture:mainfrom
Alex-Jamshidi:quote-generator-app
Open

London | 26-ITP-May | Alex Jamshidi | Sprint 3 | Quote Generator App #1229
Alex-Jamshidi wants to merge 2 commits into
CodeYourFuture:mainfrom
Alex-Jamshidi:quote-generator-app

Conversation

@Alex-Jamshidi
Copy link
Copy Markdown

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

Created quote generator app to reandomly generate quotes on button click

@Alex-Jamshidi Alex-Jamshidi added 📅 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 May 14, 2026
@illicitonion illicitonion added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels May 14, 2026
Copy link
Copy Markdown
Member

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

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

This works, but there are a few naming things to think about - please take a look :)

Comment thread Sprint-3/quote-generator/quotes.js Outdated
state.author = chosenQuote.author;
}

function appendQuote() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Append isn't the most clear name here - it suggests that each time you call it an additional quote will be added to the page, rather than replacing the existing quote.

(Even though you implementation does use append internally, the effect of the function isn't to append something to the page).

Can you think of a more clear name?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

renamed to replaceQuote

Comment thread Sprint-3/quote-generator/quotes.js Outdated
const authorElem = document.getElementById("author");
quoteElem.textContent = "";
authorElem.textContent = "";
quoteElem.append(state.quote);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This works, but you could also directly set textContent to this value:

quoteElem.textContent = state.quote

which may be more clear as to your intent when reading, and may avoid a brief flash of an empty quote in the middle.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

instead of resetting .textcontent to "" and appending new quote/author, replaced quote/author directly with .textcontent
and removed append lines

Comment thread Sprint-3/quote-generator/quotes.js Outdated
}

document.getElementById("new-quote").addEventListener("click", function () {
setup();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

setup isn't a very clear name for this action. It makes sense on load that this is what you're doing.

Personally I'd probably just have one function in file: showNewQuote, and have that be called both onload and on button click.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

renamed function to showNewQuote

@illicitonion illicitonion added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels May 14, 2026
@Alex-Jamshidi Alex-Jamshidi added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels May 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Module-Data-Groups The name of the module. Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. 📅 Sprint 3 Assigned during Sprint 3 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants