Any feedback is welcome. Thanks mates ✌️
A generic review and every kind of feedback are welcome 😎 Thanks ✌️
I haven't any particular areas where I need help for this challenge but any suggestions are very welcome.
Thanks ✌️
When I become on the larger device the element fills the container properly while the doesn't, there is a small padding on the bottom, I think because the right side has padding all around.
By myself, I opted to set the eight: 100% on the img.
I would like to know if this solution is fine or if there is a better solution to use.
Thanks a million in advance ✌️
Cheers 😎 Daniele
I would like to receive feedback on how I made the li to make it as close as possible to the design.
Thanks a million.
As always I would like to know if I am using Custom Properties properly. Thanks
Finishing the project
What challenges did you encounter, and how did you overcome them?Faded background color in footer, used absolute positioning
What specific areas of your project would you like help with?N/A
Hi @vcollins1, great job by solving this challenge 💪 I want to bring to your attention a few things you might want to fix.
alt text;background-image;padding;Let me know if I may help you to improve this solution, I hope you find it useful 🤞
Ciao ✌️
Keep Learning 📖 Keep Coding 💻
I am proud that I was able to solve the cards grid layout and I also made it responsive.
Well done man 🥳 I reviewed your code and I found some bugs to fix:
I think that setting max-width on the body tag is not the best solution, maybe you can put it on the main tag or your .testimonials;
Another thing is to change the font-size unit from px to rem is better for accessibility purposes;
You need to fix the layout on large screens, at the moment over 1440px the layout sits on the left of the screen;
That's it, I like how you keep your html and css well organised, and I like how you set the style on the grid items with :nth-of-type(), I'll keep it in mind for future challenges 😜
Well done again man 😎 Cheers ✌️
Keep Learning 📖 Keep Coding 💻
.
What challenges did you encounter, and how did you overcome them?.
What specific areas of your project would you like help with?.
Hi Kamran, well done to solve this challenge. I'm here to give you some suggestions.
for HTML
h1 and h2, you can simply wrap this part " Powered by Technology " in a b tag,section for the cards, instead you can opt for article or div;main tag should be used to wrap all the content, except for the footer of course;for CSS
calc doesn't cause any problems with performance? I ask you because I never seen something like that before;Hoping you find this useful
Cheers ✌️
Keep Learning 📖 Keep Coding 💻
Hi Dhia, great work to solve this challenge, I looked to your code because I need to solve a problem that I encountered and I found something where I can give you some tips.
To render proper images based on display size you need to use a <picture> tag instead of two <img> tags with display: none set on it;
Here is a useful article for getting started with picture element
Use relative units instead of px, for example for padding and media queries you can use em while for margin, border-radius or font-size you can use rem;
Here an article to getting started with relative units
Try to make your alt text more descriptive and avoid the word image because the screen readers start with ' image of ', in your case this will sound like ' image of product image '. Try with something like this ' A bottle of Chanel Gabrielle Parfum ' this is still poor alt text but it describes better the image to people with disabilities.
Hoping you find this useful ✌️
Keep Learning 📖 Keep Coding 💻
I really get an appreciation of how difficult it can be to get the correct spacing, colors etc without the design file.
What specific areas of your project would you like help with?I always appreciate any form of feedback.
Hey Dan, you are doing a great job of solving this challenge, your code is clear and easy to read.
I have just one suggestion, as you can see in the challenge folder we have two images, one for mobile and one for desktop, instead of using the image as a header background, check out this article to understand how to render images properly based on the device by using <picture>, with that the images are fully accessible and the browser upload just the right images with the right device.
Don't forget to add the alt text if you decide to use this approach.
Cheers✌️
📖Keep Learning 💻Keep coding
I'm proud of the fact that its finally starting to click. I'm also proud of the fact that I was able to use the feedback I got from my last project ( improving centering items with grid) right away. It took me a few times to get to this point but unless I did something wrong that I'm not aware of I wouldn't change it.
What challenges did you encounter, and how did you overcome them?I'm still a little confused on when to push things to git hub and what I should put for my commit messages. I usually do the initial commit and then not push again until I'm done with the project.
What specific areas of your project would you like help with?If possible some tips on git and git message like when to push updates and I think I'm still not centering my cards vertically correctly.
Hey there @mking0823,
About Git, before starting the learning path I had never used it, I used GitHub Desktop, but to push the updates I had to go into the application. So every time I created the repository and then I forgot to push the progress into the repository which I promptly did when I closed VS Code or even when I reopened GitHub.
I also brought this flaw into the first two challenges of the learning path, obviously, this thing takes away all the work utilities of Git or GitHub itself, but this is to tell you that you are not alone.
Now the advice:
reset initialized.
Do you write the first element of HTML? Push it. Example card structure madeSo summarize what you did and push every time to your repo so you're enable to scroll through the history and if you need you know the meaning of every step.
About centering the card vertically I think you need to change this code grid-template-rows: 94% 1fr; with this grid-template-rows: 1fr; or this grid-template-rows: 1fr auto;
I will suggest also using Relative Units as rem & em instead of px, Logical property as padding-inline-start instead of padding-left, wrapping all the card elements in an article tag and last but not least think Mobile-first when you start the next challenge, because the card isn't responsive on a mobile device.
Hoping you find this helpful ✌️
Keep learning 📖 Keep coding💻