Frontend Mentor | Alex Wilkom's profile

All comments

@dipaksinha1Submitted 3 months ago

suds

@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.

Sergio Fraga•40

@SergioZF09Submitted 3 months ago

What are you most proud of, and what would you do differently next time?

I'm pround that I could finished well this challenge. In next time I would use Bootstrap to create better the card with the QR and the information.

What challenges did you encounter, and how did you overcome them?

I had problems to put in center the card, because when I put the height for the card, this card was now on the left, so I take it off and the card finally was at the center.

What specific areas of your project would you like help with?

Not now. But, if you see something wrong in the code or in the design, you can write a different option or different way to do.

Any feedback is welcome!!

QR code component using HTML and CSS

Alex Wilkom•50

@alexwilkomPosted 3 months ago

In the name of the classes there is no need to specify 'section'. Card is already a section.
In style.css, .card does not need to have 'display: flex', the content already have a 'normal flow' (column direction).
Good use of color custom properties and border-box with the wildcard *.
Consider using custom properties for spacing and typography, too.
Good CSS organization.

Marked as helpful