Crowdfunding page

Community feedback

• 

  Zhiul•220

  @ZhiulPosted almost 2 years ago

  Hi! Your project looks really great and your code looks is mostly very clean. 😃💫 Here's some feedback:

  1- You could use <a> elements inside your .navbar-list__element. It could be a bit more semantic (they most likely could be links), also it would make them focusable (also, you could consider to style how certain elements should be while in focus, like buttons and so on, sometimes you will want to set a tabindex).

  2- Overall your code looks really great. I suggest you to use Prettier or some kind of tool that could format the code automatically.

  3- When it comes to your JavaScript code, it is great, but I think you could make it a bit cleaner by doing a few things like naming the functions, so they could be like "toggleOverlay", "toggleHamburger" and so on.

  4- You are using innerHTML it can be ok, but usually for setting text, if you're only setting text textContent seems to be a better practice.

  5- You should assign a type for each button. In this case, I think type="button should suffice. Also when it comes to some elements that are button but use other element, you should considerate to use a <button> element or at least using a role="button".

  6- You should use rem a bit more in order to make the page more accesible. Some parapgrahs and elements that should change stay with a static font-size. Also, you could want to set the font-size in your .page in rem, so you could use 1rem. More than that, if you're using rem in some text elements, you would most likely also need to handle some paddings and properties either by using rem or em (em can be more convenient in some cases); you should consider too some edge cases like if you increase browser font-size and the viewport is small, there could be some overflow.

  7- When doing transitions, only transition what is necessary. In this way, you could avoid some unnecesary transitions, and overall it is a better practice.

  8- You could achieve making the border green in this way:

  • You use a querySelector to check if some previous project has a class of green border (or selected, though this could also be written with a data attribute just like data-selected="true").
  • If some previous .popup-project has a class/data-attribute of green/selected remove it.
  • Add the class of green/selected to the .popup-project that has just been selected.

  And when it comes to getting the parent, you could use closest() 😃 So it could be a function that runs when you click a .popup-project__radio.

  It could also be possible to use has:(), however you must take into account that right now it only has a 82% support, so you should stick to JS by now and if you decide to use it, you should at least use a fallback. https://caniuse.com/css-has

  Marked as helpful

  0
• 

  Bogdan-Mihail Pavel•625

  @codeguy9Posted almost 2 years ago

  Hi. Awesome work. I see you know javascript. Can you teach me a few things?

  Thanks in advance.

  0