-
-
Notifications
You must be signed in to change notification settings - Fork 212
Glasgow | 25-ITP-Sep | Abraham-Habte | Sprint 3| Quote_Generator_App #853
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Sprint-3/quote-generator/quotes.js
Outdated
|
|
||
| // call pickFromArray with the quotes array to check you get a random quote | ||
| let randomQuote = pickFromArray(quotes); | ||
| console.log(randomQuote); // maybe logs { quote: "...", author: "..." } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Always remember to remove console.log before pushing your codes. It is a bad practice having this pushed to production.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, thanks, when i see " DO NOT modify this array, otherwise the tests may break!" i just leave it.
Sprint-3/quote-generator/quotes.js
Outdated
| quoteElement.innerText = `"${randomQuote.quote}"`; | ||
| authorElement.innerText = `- ${randomQuote.author}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using textContent instead of innerText when setting the quote and author. textContent is generally preferred for performance and consistency because it doesn’t trigger layout calculations and ignores CSS visibility. innerText is useful only when you need to respect rendered styles, which isn’t necessary here since you’re just inserting plain text.
You can read more about innerText and textContent here:
https://www.geeksforgeeks.org/html/difference-between-textcontent-and-innertext/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok thanks i will consideded it
| const authorElement = document.getElementById("author"); | ||
| const newquoteButton = document.getElementById("new-quote"); | ||
|
|
||
| function displayRandomQuote() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having a function where you can fetch a quote and update your DOM is good, as it improves code readability and quality. Well-done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it
bp7968h
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, a small change required. Also I noticed you haven't attempted the stretch task, I would advise you to do so, there is a nice learning opportunity in that task.
| <h1 class="greeting">Hello There, Enjoy the Quote</h1> | ||
| <div class="quote-box"> | ||
| <blockquote id="quote"></blockquote> | ||
| <p class="author" id="author"></p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job using the blockquote. Is there better html semantic to denote the author?
Self checklist
Changelist
Modification of the html and js file and and writing a new css in style.css file
Questions
No questions