London | May-26 | Liridona Shehu | Sprint 3 | Module-Data-Groups- Qoute Generator - #1312
London | May-26 | Liridona Shehu | Sprint 3 | Module-Data-Groups- Qoute Generator#1312shehu-dona wants to merge 5 commits into
Conversation
| ]; | ||
|
|
||
| function updateUI(quoteObject) { | ||
| let quoteParagraph = document.getElementById("quote"); |
There was a problem hiding this comment.
Do these change everytime updateUI is called? Is it efficient to query them here?
There was a problem hiding this comment.
Removed document.getElementById("") outside the function.
| } | ||
|
|
||
| // call pickFromArray with the quotes array to check you get a random quote | ||
| let randomQuote = pickFromArray(quotes); |
There was a problem hiding this comment.
Can we apply the DRY principle (Don't repeat yourself) here?
| </head> | ||
| <body> | ||
| <h1>hello there</h1> | ||
| <h1>Hello there</h1> |
There was a problem hiding this comment.
Is this H1 needed?
Just a suggestion - Worth playing around with the html and css to alter the look, format the quote etc
There was a problem hiding this comment.
Removed unnecessary heading.
Definitely I will do that, soon as i will finish with mandatory tasks :)
|
Hello @madhuranakate |
|
Hello @madhuranakate, |
|
|
||
| // call pickFromArray with the quotes array to check you get a random quote | ||
| function showRandomQuote() { | ||
| let randomQuote = pickFromArray(quotes); |
There was a problem hiding this comment.
randomQuote never gets reassigned after this line. Which declaration keyword says that to the next reader?
| authorParagraph.innerText = quoteObject.author; | ||
| } | ||
|
|
||
| button.addEventListener("click", () => { |
There was a problem hiding this comment.
The arrow function here does nothing except call showRandomQuote() with the same arguments. What happens if you pass showRandomQuote directly as the second argument instead?
There was a problem hiding this comment.
Hello @selyafi
I read your comments and made the changes.
Self checklist
Changelist
A PR with a quote generator app