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

Seafood Restaurant #284

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

Seafood Restaurant #284

wants to merge 9 commits into from

Conversation

gittebe
Copy link

@gittebe gittebe commented Sep 15, 2024

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.

Nice work on this project! 🎉 You've put a lot of effort into creating a dynamic chatbot. Let's go through some feedback:

HTML/CSS

  • The HTML and CSS looks good and structured 🌟
  • There's some commented-out code (a div with a crab image). It's a good habit to remove or uncomment this code to maintain a clean file. If it's not needed, just get rid of it to avoid clutter.

JavaScript

  • You've done a great job dividing the script into multiple functions ⭐
  • Good start with checking if the username is entered in the askName() function!
  • Consider refactoring to create smaller, reusable functions where there's repeated logic (like setting up buttons and event listeners). This will make your code more modular and easier to debug or extend in the future. For example: the chooseFish, chooseShellfish and chooseMollusks could be one function called chooseDish and do different things based on different params.

Clean Code

  • Your code is quite readable, with consistent naming conventions and the use of comments to explain sections. 👌 However, I noticed some commented-out code that should be removed if it's not going to be used.
  • Consider refactoring to create smaller, reusable functions where there's repeated logic (like setting up buttons and event listeners). This will make your code more modular and easier to debug or extend in the future.
  • Remember to indent properly, even when you're writing HTML in the JS file

Keep up the good work, and continue to think about good descriptive names for your variables and keep the code modular!

</form>
</div>
</main>
<body id='body'>
Copy link
Contributor

Choose a reason for hiding this comment

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

HTML attributes should always be in double quotes

Copy link

@HeleneWestrin HeleneWestrin left a comment

Choose a reason for hiding this comment

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

Really great work, Gitte! The code was really easy to follow and felt well structured. I couldn't come up with any areas to improve (with my current knowledge anyhow) 😄 Just one tiny suggestion for the CSS.

Keep it up ⭐

}

/* buttons to choose the food category */
.foodtype-container,

Choose a reason for hiding this comment

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

Since these containers and buttons seem to have the exact same CSS, perhaps you could have just used the same class on each element instead of having unique class names for each.

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.

3 participants