Skip to content
OverviewSolutions
23
Comments
8
Mathis
@MathisHumbert

All comments

  • Mathis•770
    @MathisHumbert

    Hi! Nice work, everything seems to work! This looks great and responsive, good job!

    I already did this project in javaScript but I will dot it with react soon. I'm currently learning React and I loved your code.

    I just think that you could improve your app by keeping track of any changes by the user with LocalStorage.

  • Mathis•770
    @MathisHumbert

    Hi! Nice work, everything seems to work! This looks great and responsive, good job!

    You JavaScript file is quite heavy and you have a lof of functions in the same file. I would recommend you to use specific module for each of your function and import them in your main js file.

    If you are not familiar with modules (export / import functions) I would recommend you to read this article: https://developer.mozilla.org/fr/docs/Web/JavaScript/Reference/Statements/import

    Marked as helpful
  • Mathis•770
    @MathisHumbert

    Hi! nice work! I love the animation that you coded!

    Your css is almost perfect, You just have an issue with your text. For tablet and big mobile display the text and image are not responsive because you probably fixed a max-width. Try to fix this, it's quite easy.

    I don't understand how you toggle the display of your text because I don't see any JS. But it's working so congratulation!

    Marked as helpful
  • Mathis•770
    @MathisHumbert

    Hi! Nice work, everything seems to work! This looks great and responsive, good job!

    I think you could improve your app using more JS and less html. Because you are hardcoding every cart in your html.

    I recommend to map over the data array that you have and return html that you will set as innerHtml to your container. If you are new to this go check this video: https://www.youtube.com/watch?v=80KX6aD9R7M.

    Then call call it using 'DOMContentLoaded' event to your window.

    Marked as helpful
  • Mathis•770
    @MathisHumbert

    Hi! Nice work!

    Your project is almost responsive. between 760px and 960px you have an issue with the css because a part of your project exceeds the display. I recommend you to set a max-width for your container.

    Also in your html you are coding the preset values for all of the cards. What you could do is creating a function that display the html and call it using 'DOMContentLoaded' event to your window.

  • Mathis•770
    @MathisHumbert

    Hi! Nice work.

    You could improve the css by adding a special design for the mobile view. I think you forgot to do it, go learn more about mobile first.

    Also in your html you are coding the preset values for all of the cards. What you could do is creating a function that display the html and call it using 'DOMContentLoaded' event to your window.

    Marked as helpful
  • Mathis•770
    @MathisHumbert

    Hi @boymelvs!

    Nice job with this project! Everything seems to work!

    Your JavaScript is working but not perfect. Try to use the json file provided with this project. With this file you can get access to the data using fetch, follow this link if you want to learn more and go check for 2. GET JSON data. https://dmitripavlutin.com/fetch-with-json/

    Once you have your data you can create a function to display the html!

  • Mathis•770
    @MathisHumbert

    Hi! Nice work, everything seems to work! This looks great and responsive, good job!

    I think you could improve your app using localhost to save the user's theme and list items.

    Also you handle the color theme in js by changing each element. A better approach is to toggle a classList on the html element and in css user the html element and the classList name to change the root color.

    For example

    :root{ --main-color: #fff }

    .darkhtml { --main-color: #222; }

    Marked as helpful