Skip to content
OverviewSolutions
12
Comments
41
Karim
@yasssuz

All comments

  • Karim•590
    @yasssuz

    div hell, you should learn about semantic HTML

  • Karim•590
    @yasssuz

    Hello Melvin.

    Awesome build, here the answers to your questions:

    • It's better if you use max-height: 100%. The inherit keyword specifies that a property should inherit its value from its parent element. If the parent element had a different value, your code would look totally different.
    • Yes you could. Inside class="avatar" create another div an put inside the h2 and the img. This should work perfectly.
    • You did a very good job, your code is nice and clean. I would just suggest using BEM for naming your classes. This article will help you to get started.

    Happy coding and please upvote my comment :)

  • Karim•590
    @yasssuz

    Hello Ugo!

    Awesome build, just a few tips:

    • Your background is repeating on bigger screens, this is happening because the image is not of the same size. A simple solution would be to add background-repeat: no-repeat; background-size: cover;. This will stop your background from repeating and will make sure that your background is the same size as your screen without stretching it, it's contained automatically.
    • I strongly suggest using unordered lists for semantic reasons. Use ul on id="todoList" and for all the children use li.
    • U should not use a percentage width on your class="to-do", this is making your main content look very small on smaller screens. I suggest using 550px instead of 35%.

    Please upvote my comment if I was helpful and happy coding :)

    Marked as helpful
  • Karim•590
    @yasssuz

    hello!

    Sure, here are my opinions:

    • The best semantically speaking Html structure IMO is this:
      <header></header>
  <main>
    <section></section>
    //all of your sections should go inside a <section> nested inside main
    <section></section>
  </main>
  <footer></footer>
    • No, they don't. I prefer to keep them in another file so it is easier to maintain.
    • For me is actually working fine, but I would suggest using ::before with position: absolute (make sure the parent element is position: relative).
    • I usually use Figma or sketch.
    • If you're not a pro member, you will still have the jpeg of each page of the project. So open Figma, drop them inside your workspace, and create blocks to measure height and width.

    Please upvote my comment and happy coding :)

  • Karim•590
    @yasssuz

    Hello, Sebastian!

    Awesome build, just a few things about your js:

    • For loops are ok, but in javascript we really like using forEach and map, look at this example:
    //your for loop
  for (i = 0; i < radios.length; i++) {
     radios[i].addEventListener("change", openPledge);
  }
//with forEach
  radios.forEach(radio => radio.addEventListener("change", openPledge))

    Trust me, this will make your code easier to write, read and maintain.

    • You don't need a openMenu() and a closeMenu(), u can use toggle and have a compacted, quality code. Look:
      function handleMenu() {
      document.querySelector(".nav-list").classList.**toggle**("open");
     document.body.classList.**toggle**("overlay");

     mobileMenu.childNodes[0].src = "./images/icon-close-menu.svg" ? "./images/icon-hamburger.svg"  :  "./images/icon-close-menu.svg";
  }

    For any question or suggestion feel free to ask :)

    Upvote my comment if I was helpful and happy coding :)

  • Karim•590
    @yasssuz

    Hello Aurora!

    Awesome build, just a few tips for the javascript:

    • You're executing an event listener the first time the window loads. In this case, your javascript is pretty small, so you shouldn't have performance problems. But this is something that you should try to avoid. A solution would be form.addEventListener('submit', () => ...), this will execute the code only when the form is submitted.
    • Is a very good practice to make each function to do something different. I strongly suggest isolating each function, here is an example:
      function validateField() {
    const pattern = (regex here)
    const inputValue = input.value

   inputValue.match(pattern) ? validField() : notValidField() 
  }

//then you create these functions that will render error and stuff

    For any question or suggestion feel free to ask :)

    Upvote my comment if I was helpful and happy coding :)

  • Karim•590
    @yasssuz

    Hello!

    Awesome build, just a few tips:

    • Your App.js in wrapping the entire web app. This can make your code harder to read and to maintain, consider creating components (I strongly suggest doing that with .filters-bar-list, .jobs-list, and job-filters).
    • Having your entire application in one file is causing unnecessary re-renders. A solution to that would be creating components as I said before and isolating them to make them re-render only when necessary (learn about memo, useCallback and useMemo).
    • Take a look at your Html issues reported by front-end mentor!

    For any question or suggestion feel free to ask :)

    Upvote my comment if i was helpfull and happy conding :)

  • Karim•590
    @yasssuz

    Hey ApplePieGiraffe!

    I'm working on the same project and I will submit it in a few days. Your build is awesome, it's very responsive and reactive. However, using react dev tools, I saw that a lot of re-renderings happening on your application, mostly from components on your Invoice page. So I started looking at your code and I saw that you're not using memo or useCallback on your functions and components. What's happing is that every time the state of your parent element changes, all the components are going to re-render again, even if they don't need to re-render. If possible, I would like to copy your repo and create a solution for this problem. Let me know if I can, and maybe, we could work together to solve this problem!

    Thanks for your time and happy coding :)

  • Karim•590
    @yasssuz

    Awesome build!

  • Karim•590
    @yasssuz

    Hello Jack!

    CSS Grid and Flexbox are awesome features. But remember:

    • flexbox: when you're working with mono-dimensional layouts, for example, a navbar. Because you could put a space between the logo and the nav, or you want your logo at the left, nav in the middle, and buttons on the right and you can do that with just a line of code justify-content: space-between;
    • grid: when you're working with bi-dimensional layouts, in fact, It helps us to create layouts that can be redefined using Media Queries and adapt to different contexts. Grid Layout lets us properly separate the order of elements in the source from their visual presentation.

    Happy coding and please upvote my comment if I was helpful :)

  • Karim•590
    @yasssuz

    Hello Ushiyama!

    Awesome build, but I would change somethings:

    • Your mobile version looks ok, but the elements are attached on the left and right, you can solve this by adding a margin: 20px; inside the .social-panel .
    • after 375px until 1200px your layout just die. This is because the computer layout will not work on a screen of small size. Change your media query from 375px to at least 1050px.

    happy coding and if I was helpful please upvote my comment :)

  • Karim•590
    @yasssuz

    Hello Ushiyama!

    Awesome build, but I would change somethings:

    • Your mobile version looks ok, but the elements are attached on the left and right, you can solve this by adding a margin: 20px; inside the .social-panel .
    • after 375px until 1200px your layout just die. This is because the computer layout will not work on a screen of small size. Change your media query from 375px to at least 1050px.

    happy coding and if I was helpful please upvote my comment :)

  • Karim•590
    @yasssuz

    Hello, you forgot the background of the hero!

    happy coding :)

  • Karim•590
    @yasssuz

    hello, awesome build!

    I would suggest you save your data in the local storage, so when you refresh the page you still have your todos. Maybe this article will help you.

    Happy coding and if i was helpful please upvote my comment :)

  • Karim•590
    @yasssuz

    Hello Syed.

    Sure, the cleanest way is using a negative margin value. margin-top: -52px; This should work for you.

    Happy coding and please upvote my comment if I was helpful :)

  • Karim•590
    @yasssuz

    wow, your javascript is so clean!!!!!! I'm going through advanced javascript right now and I learned a new way of validation from you. Using regex and match to then activate a function is an awesome idea. Good job!!

  • Karim•590
    @yasssuz

    Hello Asmaa,

    Amazing job, but here a few things:

    • the button looks good, but you should definitely add a cursor: pointer to make sure the user knows that it is clickable.
    • I would as well add a cool animation on the button to make the transition between colors more smooth, here the code: transition: color 0.3s ease, background-color 0.3s ease
    • in the tablet version, it looks a little bit buggy, try to change the media query from 800px to 1025px.

    happy coding and to support me please upvote my comment :)

  • Karim•590
    @yasssuz

    hello, you did an amazing job :)