Skip to content
OverviewSolutions
29
Comments
10

Yup

@Yup03Sénégal, Dakar730 points

I’m a mysterious individual who has yet to fill out my bio. One thing’s for certain: I love writing front-end code!

Latest solutions

View more solutions

Latest comments

  • Yup•730
    @Yup03

    Great Challenge @kabir-afk

    At first sight, contribution i can make is that when one click on the button after invoking addToCart , It will be a good behavior to set back numberOfItems to 0 like the following:

    <button
    className="addToCart"
    onClick={() => {
           props.addToCart({ numberOfItems })
            setNumberOfItems(0)
            }}
 >
  • Yup•730
    @Yup03

    Hi @ttsoares nice challenge

    First impression i wanted to put emphasis on is the structure of the todos array

    • I think you should whether have the completed property like so :
    {
    id: "item-1",
    content: "Complete entire javaScript course ",
    completed: false,   /*If the checkbox is "checked", the "completed" property will have "true" as value/ And your checkbox  "unchecked" will result this property becoming "false" */
  },
    • Or the active property :
    {
    id: "item-1",
    content: "Complete entire javaScript course ",
    active: true,  /*If the checkbox is "checked", the "active" property will have "false" as value/ And your checkbox  "unchecked" will result as "true" for this property  */
  },

    And your toggleCompleted function should take the id as argument due its uniqueness

    • Otherwise if you use the content, you can encounter many tasks that have the same content
    • And it's valable for the remove and toggleActive functions
    function toggleCompleted(id ) {/*instead of "todo"*/
    const result = todos.map((elm) => {
      if (elm.id === id) {
        return { ...elm, completed: !elm.completed };
      } else return elm;
    });
    setTodos(result);
    setBackTodos(result);
  }

    And as a result you'll also call toggleCompleted or remove or toggleActive functions with the id of your task like so :

    <div onClick={() => toggleCompleted(todo.id)}></div>

    At first sight, these are some point that i can highlight on your code; And i hope this will help you improve your solution

  • Yup•730
    @Yup03

    Nice challenge @arogersrenee . As an answer to your question , the fast refresh is caused by the action of submitting the form .
    And you can prevent it like this

     form.addEventListener("submit", (e) => {
            e.preventDefault();
//Instead of e.preventDefault
            validateEmail();
        })

    It looks good on mobile screens
    But i think it will be also nice if you fix the responsiveness on larger screens

    Besides this i think it's all good

    Marked as helpful
  • Yup•730
    @Yup03

    Great challenge @ahmetmetinarslan One suggestion that i could make is to add margin to headers and images so you can make it more close to the design

    Besides that i think it's all good in my opinion

    Marked as helpful
  • Yup•730
    @Yup03

    Great challenge @AdeMarq, And i think it's all good but the only thing that i may suggest is to add transition to your buttons to smoothly change properties when passing from "normal" to "hover" state : button{transition: all 0.3s ease}

    You can also add the cursor property to all your buttons in one place instead of putting cursor:pointer to each hover state of each button you could just do button{cursor:pointer} and it will be applied to all your button element

    There is some padding that you can add especially to the top of the container when adding breakpoint :

    @media (max-width: 930px){
.container{
padding-top:100px;/*You can also adjust the value to your need*/
}

    That's in my opinion some improvement that can be made . Hope that you will find it helpful

  • Yup•730
    @Yup03

    Hi @lamriouiaya20 great challenge, But it feel like you use absolute units a little bit too much , instead of using absolute units like "pixels" for sizing your grid you could for example use relative units like "fr" for allowing your grid to be more flexible

    .contenu {
 max-width:1100px; /*utilise "max-width" au lieu de "width" ici et essaie de mettre une valeur superieur a 800px , tu peux même aller jusqu'à 1200px*/
 grid-template-columns: 1fr 1fr 1fr 1fr; /*instead of doing 220px 220px 1fr 1fr*/
 grid-template-rows: 1fr 1fr;  /*instead of using 240px 230px*/
}

    Also instead of using the <br> tag to break the lines you can just use display:block on the inline elements

    /*You can just remove all the <br> tags from your document and apply these lines*/
.blanc{
display:block
}
.opa{
display:block
}

    Use 'grid-row' and 'grid-column' to place your items inside the grid, it will fit best than 'grid-area' for this use case For example:

    .part1{
grid-column:1/3
}
.part2{
grid-column:3/4
}
.part4{
grid-row:2
grid-column:1
} 
etc....

    Try to also add more breakpoints to improve the responsive part

    Je t'encourage beaucoup a plus faire des recherches sur 'grid-column' et 'grid-row' plutot que d'utiliser 'grid-area' Et aussi a renommer tes classes en anglais pour avoir plus de feedback et pouvoir évoluer dans ton parcours de developeur front-end

    Marked as helpful
View more comments