suds

Community feedback

• 

  P

  Alex Wilkom•50

  @alexwilkomPosted 3 months ago

  Dear Dipak kumar sinha,

  I am very visually pleased about the final outlook of your project. It really resembles the design of the original. You really did put an effort into it. Well done.

  Let me suggest some improvements as well as any learning resources that you may find useful:

  • I did not find any documentation of the project. The Github link that you provided takes to another unknown project. I had to rely on using the developer tools to have a look at your code. Update the link to the actual GitHub repository if you have it to give peers a better experience on reviewing your project.

  • Make use of Semantic HTML. It is essential for improving accessibility and meaning to the structure of the page.

  • The alt attribute on the avatar image must be a description of what the actual image is. For example: alt="Article author's face in a three quarter view"

  • Make sure to use media queries for building a responsive site. Your card is not adjusting to the change of the screen. The style-guide.md as well as the figma design file explicitly give details of what the values are to text and a breakpoint for a mobile design and desktop design. Take care of following these requirements.

  • It is always a good idea the use of custom properties. However, take care of actually using them. I can see that --font-family and --withe (misspelled) are declared but never used. Otherwise, remove them. This is to minimise the size of the file and thus improving performance as well as avoiding confusion.

  • I do not suggest using the trick font-size: 62.5% in the html element. Josh Comeau will explain you why. But it is up to you to use it, just be mindful when and how applying the rem unit in your project.

  • Be mindful of consistency in your code. For example in the use of the units. The .card uses pixels for width, but .contenedor uses rem. It is recommended to use rem. The unit ratio is attached to the font-size of the root (html) of the page. This is the best option for responsive layout as well as accessibility. Read the entire article by Josh Comeau, and look for MDN resources for a deep dive.

  • I can see that you tried to centre the card with the margin property. This gets the task done but it is not centred vertically. There is a better option for this and is the use of Flexbox. You have used it only in your avatar element. But it can be applied to the layout of the card. I would suggest to apply the following code:

  body {
      min-height: 100svh;
      display: flex;
      justify-content: center;
      align-items: center;
  }
  

  As mentioned above, although you used Flexbox in your avatar element, it is not used 100% correctly. The justify-content: flex-start is redundant as it is the default value when applying display: flex with a, also default, flex-direction: row. If you are not very familiar with Flexbox, I strongly suggest you go through the free interactive course Flex Zombies, and reading about it in this MDN article. Flexbox is an essential skill for any front-end developer. With this new knowledge, you will be capable of improving the layout of the card as well any content inside it. I would suggest recreating the card after you learn Flexbox. It will give you a taste of what it can be achieved with it with clearer code.

  I hope you find these suggestions helpful and will bring you clarity on how and what to keep improving in your journey. I wish you all the best. Keep up the hard work. It is a matter of practice. I am still practicing, and I will continue to do so. I hope you will too.

  1