LONDON | 25-ITP-MAY | TEWODROS BEKERE | M-D-G | SPRINT-3 | Quote_Generator_App #677
Conversation
…ndom quote functionality in JavaScript for Quote Generator App
iteddy16
added
📅 Sprint 3
jenny-alexander
added
Review in progress
| @@ -490,4 +510,5 @@ const quotes = [ | |||
| }, | |||
| ]; | |||
|
|
|||
| // call pickFromArray with the quotes array to check you get a random quote | |||
| console.log(pickFromArray(quotes)); | |||
There was a problem hiding this comment.
It's a good idea to remove console logs before submitting your work for review.
| @@ -1,3 +1,22 @@ | |||
| function getRandomQuote() { | |||
There was a problem hiding this comment.
If your variable will not be reassigned within it's scope, it's better practice to use 'const' instead of 'let'.
Further reading: https://www.freecodecamp.org/news/differences-between-var-let-const-javascript/
There was a problem hiding this comment.
You did a great job assigning quote and author from the random quote object.
🌱 This suggestion isn't required but I wanted point out that you can use destructuring here. You can use object destructuring to extract both properties (quote and author) in one line. For example:
let { quote, author } = pickFromArray(quotes);
This is a common JS feature that lets you pull out several properties from an object at once
| function pickFromArray(choices) { | ||
| if (!Array.isArray(choices) || choices.length === 0) return null; |
There was a problem hiding this comment.
Since pickFromArray could possibly return null (i.e. if the quotes array was empty), what can you do to safeguard your code to ensure that if null is returned, your function doesn't throw an error?
There was a problem hiding this comment.
Fixed, it's handled by displaying an alert to the user.
There was a problem hiding this comment.
Can you run Lighthouse on your app? There are a few suggestions to implement in order to make your page a bit more accessibile.
(In order to run lighthouse, go to dev tools, and choose 'Lighthouse' from the menu of items available).
There was a problem hiding this comment.
No, I checked it was 93%. Now it's fixed the background color (white), and the new-quote button text (white) was the same; that's why.
jenny-alexander
added
Reviewed
|
@jenny-alexander, thank you for your comments and suggestions. I think I fixed the issue raised. Will you check it now? Thank you. |
…ndom quote functionality in JavaScript for Quote Generator App
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.