Skip to content
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

NW6| Bakhat Begum | MODULE-JS2 | Js quotes/week 3 | WEEK-3 #189

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

BakhatBegum
Copy link

@BakhatBegum BakhatBegum commented Jan 14, 2024

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

When the page loads it should show a random quote from the quotes array on the screen. It should also show who said the quote.

When you click a button on the screen it should change the quote on the screen.

  • Add a toggle-switch or checkbox input to your existing quote generator.
  • When the switch is ON:
    • ... the generator should pick and display a new quote every 60 seconds. (When testing you will probably want to reduce this to every 5 seconds.)
    • ... the generator should display "auto-play:ON" somewhere on the page.
  • When the switch is OFF
    • ... the generator should NOT change quote automatically

Questions

Is it good practice to do style in CSS?

Copy link

netlify bot commented Jan 14, 2024

Deploy Preview for cute-gaufre-e4b4e5 ready!

Name Link
🔨 Latest commit 58c13c6
🔍 Latest deploy log https://app.netlify.com/sites/cute-gaufre-e4b4e5/deploys/65b6f500272ea900084761d8
😎 Deploy Preview https://deploy-preview-189--cute-gaufre-e4b4e5.netlify.app/week-3/quote-generator
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

<input type="checkbox" id = "check-box"/>
<p id="auto-play-text"></p>
</div>
</section>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's great to see that you're using semantic elements like <section>, <h1>, <p>, and <button>.
As a further step, you can consider using a <form> and, you can improve the accessibility of your checkbox by adding a <label> element:

<form>
      <label for="check-box">Auto Play:</label>
      <input type="checkbox" id="check-box" />
 </form>

@@ -25,469 +25,509 @@ function pickFromArray(choices) {
const quotes = [
{
quote: "Life isn't about getting and having, it's about giving and being.",
author: "Kevin Kruse",
author: "- Kevin Kruse",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of manually adding the dash ("-") to each quote in the array, you could use JS to insert it into your HTML code dynamically :)

quoteParagahraph2.innerText = `- ${currentQuote.author}`;

By using JS to handle the quote and formatting, your code becomes more flexible and easier to maintain!

Comment on lines 499 to 500
const quoteParagahraph2 = document.getElementById("author");
quoteParagahraph2.innerText = currentQuote.author;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! In the future, consider using more meaningful variable names like authorName for better clarity and readability.

function eventhandler(event){
console.log("click");
getQoutes();
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While console logs can be helpful during development and debugging, they are typically not necessary in the final version of the code. It would be helpful to remove the console log statements before submitting your code!

#check-box{
position: absolute;
margin-top: 80px;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your preview looks great! Well done @BakhatBegum !

@BakhatBegum
Copy link
Author

BakhatBegum commented Jan 29, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants