-
-
Notifications
You must be signed in to change notification settings - Fork 155
WESTMIDLANDS | ITP-MAY2025 | Peter Lui | Sprint3 Quotes Generator #739
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
base: main
Are you sure you want to change the base?
Conversation
@@ -1,3 +1,73 @@ | |||
function displayRandomQuote() { | |||
if (!quotes.length) return; // Guard against empty quotes array |
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.
Error handling is a good practice! 👍
quoteElement.style.backgroundColor = "#f0f4f8"; // Light blue-gray background | ||
quoteElement.style.padding = "20px"; // Padding for spacing | ||
quoteElement.style.borderRadius = "8px"; // Rounded corners | ||
quoteElement.style.fontSize = "1.2em"; // Larger font for readability | ||
quoteElement.style.color = "#333"; // Dark text for contrast | ||
quoteElement.style.margin = "20px"; // Margin for separation | ||
quoteElement.style.textAlign = "center"; // Center the text | ||
quoteElement.style.fontStyle = "italic"; // Italicize quote | ||
quoteElement.style.borderLeft = "4px solid #007bff"; // Blue accent border | ||
|
||
// Style the author | ||
authorElement.style.color = "#007bff"; // Blue color for author | ||
authorElement.style.padding = "10px 20px"; // Padding for spacing | ||
authorElement.style.fontSize = "1em"; // Slightly smaller than quote | ||
authorElement.style.textAlign = "right"; // Right-align author | ||
authorElement.style.fontWeight = "bold"; // Bold for emphasis | ||
authorElement.style.margin = "0 20px 20px 20px"; // Margin for separation |
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.
You can control styles using the DOM; however, it's recommended to use CSS files rather than controlling them with JavaScript because of performance, separation of interest, and maintenance reasons.
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.
You can control styles using the DOM; however, it's recommended to use CSS files rather than controlling them with JavaScript because of performance, separation of interest, and maintenance reasons.
Hi Day! I totally agree with you - but I think we are not allowed to change the CSS file? Also it wasn't a requirement to style the page, so it was more of an experiment for me. Would you like me to delete the section? Thanks for your help!
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.
Hi @petergmlui, It's just a recommendation. You don't have to create a new css file only for this. Just wanted to share the best practice. 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.
Thank you Day! Highly appreciated!
Learners, PR Template
Self checklist
Changelist
Initial Submission
Questions
Ask any questions you have for your reviewer.