London | 26-ITP-May | Alex Jamshidi | Sprint 3 | Quote Generator App #1229
London | 26-ITP-May | Alex Jamshidi | Sprint 3 | Quote Generator App #1229Alex-Jamshidi wants to merge 2 commits into
Conversation
illicitonion
left a comment
There was a problem hiding this comment.
This works, but there are a few naming things to think about - please take a look :)
| state.author = chosenQuote.author; | ||
| } | ||
|
|
||
| function appendQuote() { |
There was a problem hiding this comment.
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?
| const authorElem = document.getElementById("author"); | ||
| quoteElem.textContent = ""; | ||
| authorElem.textContent = ""; | ||
| quoteElem.append(state.quote); |
There was a problem hiding this comment.
This works, but you could also directly set textContent to this value:
quoteElem.textContent = state.quotewhich may be more clear as to your intent when reading, and may avoid a brief flash of an empty quote in the middle.
There was a problem hiding this comment.
instead of resetting .textcontent to "" and appending new quote/author, replaced quote/author directly with .textcontent
and removed append lines
| } | ||
|
|
||
| document.getElementById("new-quote").addEventListener("click", function () { | ||
| setup(); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
renamed function to showNewQuote
Learners, PR Template
Self checklist
Changelist
Created quote generator app to reandomly generate quotes on button click