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

Liselottes chat-bot #293

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

Liselottes chat-bot #293

wants to merge 3 commits into from

Conversation

IamLise
Copy link

@IamLise IamLise commented Sep 15, 2024

Netlify link

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

https://splendorous-pegasus-012d17.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.

Hey, nice chatbot! ☕ You've taken a little different approach than we expected (by putting all HTML elements directly in the HTML file), but since you are modifying the styles in JS, we'll allow it 😊 Let's look at some feedback!

HTML

  • Instead of having a lot of different forms, you could have one form with different sections in it, that you pick up in JS to display/hide

CSS

  • Since all parts of the form but one, have the display none as default, maybe that could be put in the CSS file? And then you can change whatever you need when you need it in the JS file

JavaScript

  • The flow of the bot works well and I like that you used buttons ⭐
  • Your JavaScript code could benefit from additional modularity. For example, if you have repetitive logic (e.g., handling different input types or bot responses), consider splitting these into separate functions for better readability and reusability.
  • You're using the onclick method instead of addEventListener, is there a specific reason for this? 👀

Clean Code

  • Be consistent with whether you're using double or single quotes. If you want to use double quotes, you should change all single quotes from the starter code to be double quotes.
  • Remember that functions should be named after what action they are performing
  • Great naming of your DOM selectors ⭐

Copy link

@gittebe gittebe left a comment

Choose a reason for hiding this comment

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

Hej Liselotte,
The chatbot works well and you used name input and buttons! Well done!!!
that's super interesting how you made it work because you have the forms in your HTML and let them "disappear". I believe if you would have more information/data, it might be easier in terms of shorter and easier to follow to use the innerHTML in the javascript.
Try to make more comments on your code about what you are doing or where one part ends and another starts. That helps me always a lot to read and understand codes.
Overall: Well done!!! :)

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