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

Pink Magic 8 Ball #279

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

Pink Magic 8 Ball #279

wants to merge 5 commits into from

Conversation

nella-x
Copy link

@nella-x nella-x commented Sep 13, 2024

Netlify link

Add your Netlify link here.
PS. Don't forget to add it in your readme as well.

https://pink-magic-8-ball.netlify.app/ <----

Copy link
Contributor

@HIPPIEKICK HIPPIEKICK left a comment

Choose a reason for hiding this comment

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

What a fun idea for a chatbot! 🎱

HTML/CSS

  • Well structured and styled, not much to say ⭐
  • Since you're using the name form for all the inputs, consider changing the class and ID for this (and the corresponding DOM selector) to make it more generic

JavaScript

  • You've broken down the code into different functions like saveNameAndRespond, and generateMagic8BallResponse. This modular approach is great for readability and reuse. Kudos for descriptive variable naming!
  • Easy to follow the flow, and it feels like you're not overcomplicating things.
  • Consider disabling the button or in some other way validating the user input so that it's not possible to send empty inputs

Clean Code

  • Be consistent with whether you're using arrow functions or function declarations.
  • Remove console.logs before handing it, they are good for developing but not needed in production.

Nice job, I had fun trying this out 😄

Comment on lines +5 to +6
const nameForm
= document.getElementById('name-form');
Copy link
Contributor

Choose a reason for hiding this comment

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

Slight error here (unnecessary newline):
const nameForm = document.getElementById('name-form');

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.

2 participants