Ryan Hardy
@ryyHardyAll comments
- @omima-atefSubmitted 5 months ago@ryyHardyPosted 4 months ago
Really excellent job on this one. The design is nearly identical to the images. I can also see that you handled the mobile and desktop versions well.
Here are some (minor) critiques:
- I would recommend using semantic alternatives to the <div> element wherever you can.
- You didn't need to use a font-awesome icon because the icon svg is already provided in the images folder.
- Honestly I can't think of anything else because this is really good. :D
0 - @jasonlimasSubmitted 4 months ago@ryyHardyPosted 4 months ago
This is extremely close to the design images so I had trouble poking holes in it. The CSS is very well-organized, too.
The only significant thing I'd like to point out is that there are a lot of hard-coded pixel sizes in the CSS code, so the design doesn't scale to smaller screens. Responsive design is hard, though, so I understand. Here is a Kevin Powell video about responsive design I really like, if you want to learn more: https://www.youtube.com/watch?v=x4u1yp3Msao. Really great person for learning CSS overall.
0 - @DiegoAlveesSubmitted 4 months ago@ryyHardyPosted 4 months ago
SInce the GitHub link doesn't work, I tried to use the devtools to review your code. I like the HTML a lot and how you chose accessible options like the section and nav elements. The only thing I noticed in the HTML was that the social links didn't have anchor tags which means they aren't really links. Still, I like how it's organized.
As for matching the design, it's nearly identical. The only difference I saw was in some of the font weights as well as the size of the name in the profile.
Overall, really good job!
Marked as helpful0 - @Andre-DMSubmitted 7 months agoWhat are you most proud of, and what would you do differently next time?
I didn't know how to import fonts from a local folder, I'm not sure if I did everything correctly, so I'd appreciate some feedback on that! I am not sure I want to continue using css nesting. In small projects it may be useful, but when facing more complex challenges I fear it may become too confusing later on. If you have any advice on this I welcome it!
What challenges did you encounter, and how did you overcome them?I had difficulty rendering the image exactly as the design and... I didn't make it. Maybe I should have used background-image? And if so, how?
What specific areas of your project would you like help with?As I said above I had problems with the image, I also didn't understand how I was supposed to resize the font without using media queries.
@ryyHardyPosted 7 months agoI really like the way you wrote your CSS. I can tell you have a good foundation for this stuff. As for what you mentioned:
- I feel like it's usually better to use Google Fonts, but this article has a pretty good summary of your options for loading fonts.
- I don't mind the CSS nesting, but you can do the same thing using a descendant selector or something similar. Try looking at a reference of CSS selectors to see if there are alternatives to nesting because it isn't that widely used yet.
- I think the image is good. All I noticed was that the padding on the card as a whole looks a little large.
- As an alternative to media queries, I would try to mess around with the clamp() function. It's really good in some cases. I don't know for sure, though.
That's all I have to say. Everything else is really good.
Marked as helpful1 - @fullmoonemptysunSubmitted 8 months agoWhat are you most proud of, and what would you do differently next time?
I feel good about how well I could implement the layoting and the shadows to match the design template very closely.
What challenges did you encounter, and how did you overcome them?None, Just time taking
What specific areas of your project would you like help with?Ability to think and write code faster.
@ryyHardyPosted 8 months agoI really like the way you spiced up the appearance and gave it your own theme. The gradient on the QR code looks sick and it's really eye-catching (especially with the box shadow). I think this is okay because everything else follows the design guide really well.
A few ideas for improvement in the code:
- You don't need to use Flexbox. Instead, you could use "text-align: center;" on the card. For the image, you could just make the width 100% and add some more padding to the card for the same look.
- I would be weary of how many divs there are. I'd recommend a <figure> element.
That's it. It's really tough to poke holes in this.
Marked as helpful1