@hsyigitogluSubmitted over 1 year ago
I'm waiting feedbacks from you
Francisco Carrillo
@frank-itachiAll comments
- @frank-itachiPosted over 1 year ago
Hello there 👋. Congratulation for completing the challenge👍!
I have some suggestions about your code that might interest you.
HTML 📄:
- Wrap the page's whole main content in the
<main>tag. - Since the mobile design has a different image, you can use the
<picture>tag that allows you to interchange the images depending of the viewport size. Red more about this awesome tag here. - Make sure that the
<img>elements in your HTML code has an alternate (descriptive) short text. The reason for this is that screen readers can’t translate images into text. So to fix this you can do the following<img src=”…” alt=”short text” >
CSS 🎨:
- You can use grid or flexbox to center the content no matter the viewport size. Using Grid:
body { min-height: 100vh; display: grid; place-content : center; }FlexBox
body { min-height: 100vh; display: flex; align-items: center; justify-content: center; }- You can also use the
text-transform: uppercase;property to make the perfume word appear in upper case even though you typed it in lower case in the HTML file.
I hope you find it useful! 😄 Above all, you did a good job!
Happy
<coding />😎!Marked as helpful1 - Wrap the page's whole main content in the
- @qumrranSubmitted over 1 year ago
Here is my solution for the Frontend Mentor task. I had some difficulties with media queries, but I hope I fixed what was needed.
@frank-itachiPosted over 1 year agoHello there 👋. You did a good job!
I have some suggestions about your code that might interest you.
HTML 📄:
- Wrap the page's whole main content in the
<main>tag. - Since the mobile design has a different image, you can use the
<picture>tag that allows you to interchange the images depending of the viewport size. Red more about this awesome tag here.
I hope you find it useful! 😄 Above all, the solution you submitted is great!
Happy
<coding />😎!Marked as helpful0 - Wrap the page's whole main content in the
- @sajidrecSubmitted over 1 year ago
redesigned please leave your feedback
@frank-itachiPosted over 1 year agoHello there 👋. Congratulation for completing the challenge👍!
I have some suggestions about your code that might interest you.
HTML 📄:
- Since the mobile design has a different image, you can use the
<picture>tag that allows you to interchange the images depending of the viewport size. Red more about this awesome tag here.
I hope you find it useful! 😄 Above all, you did a good job!
Happy
<coding />😎!Marked as helpful1 - Since the mobile design has a different image, you can use the
- @MichalBobkaSubmitted over 1 year ago
Hi I think it looks good. Its responsive only for two resolutions as in project description. I'm unsure about :hover on button. I had problem doing smooth transition with gradient so i did it with solid color. I will be happy if u could help me handle this :)
@frank-itachiPosted over 1 year agoHello there 👋. Congratulation for completing the challenge👍!
- You could use the
background: linear-gradient(#color1, #color2);property-value to style the pseudo class :hover of the button. - Also try not nest too much rules in SASS/SCSS becuase the more you nest the more time will take to the browser to load the code. This way you keep the code clean as well.
I hope you find it useful! 😄 Above all, you did a good job!
Happy
<coding />😎!Marked as helpful0 - You could use the
- @shashikantdev3Submitted almost 2 years ago
Any recommendations, feedback and advice to improve my project or skills are welcome. :D
@frank-itachiPosted almost 2 years agoHello there 👋. Congratulation for completing the challenge👍!
I have some suggestions about your code that might interest you.
- Your code is readable and well structure but when it comes to functionality and user experience there are some aspects we’ve got to think. For instance, according to de design in the
<footer>section there must be some links that should redirect the user to a different page that provides additional information. The About Us link is a good example. So, you should use a<a href=””>tag to achieve such purpose instead of using the<p>one. - Remember, we also gotta take into account the user experience and not only the design.
I hope you find it useful! 😄 Above all, you did a good job!
Happy
<coding />😎!0 - Your code is readable and well structure but when it comes to functionality and user experience there are some aspects we’ve got to think. For instance, according to de design in the
- @SavindushehanSubmitted almost 2 years ago@frank-itachiPosted almost 2 years ago
Hello there 👋. Congratulation for completing the challenge👍!
I have some suggestions about your code that might interest you.
HTML 📄:
- The icon-star images are decorative so you can add the
alt=""attribute to the<img>tag and leave it empty. But the profile images aren't so in that case you need to add a short description.
I hope you find it useful! 😄 Above all, you did a good job!
Happy
<coding />😎!Marked as helpful0 - The icon-star images are decorative so you can add the
- @ChamodJSubmitted almost 2 years ago@frank-itachiPosted almost 2 years ago
Hello there 👋. You did a good job!
I have some suggestions about your code that might interest you.
HTML 📄:
- Wrap the
<div class="main-container"content in the<main>tag.
CSS 🎨:
- Avoid using absolute length units px, especially for font-size and width properties, because they are not relative to anything else so that means they will always be the same size. Instead, you can use relative lengths like em or rem. The benefit of that last one is element which has that unit will scale relatively to everything else within the page, e.g., the parent container. You can dig up about it here.
I hope you find it useful! 😄 Above all, the solution you submitted is great!
Happy
<coding />😎!Marked as helpful0 - Wrap the
- @adamstopinskiSubmitted almost 2 years ago@frank-itachiPosted almost 2 years ago
Hello there 👋. Congratulation for completing the challenge👍!
I have some suggestions about your code that might interest you.
HTML 📄:
- Wrap the
<div class="card">content in the<main>tag. - You could also use the
<picture>tag that allows you to interchange the images depending of the viewport size. Red more about this awesome tag here. - In this case there is only one
<footer>tag so you can get rid of theclass="footer"attribute of it and use the selector tag in you CSS stylesheet.
I hope you find it useful! 😄 Above all, you did a good job!
Happy
<coding />😎!Marked as helpful1 - Wrap the
- @MoinwkhanSubmitted almost 2 years ago@frank-itachiPosted almost 2 years ago
Hello there 👋. You did a good job!
I have some suggestions about your code that might interest you.
HTML 📄:
- Wrap the page's whole main content in the
<main>tag. - The heading order is important in the html structure so try to always start your headings and/or titles with an
<h1>tag and then you can increase by one if you need to use more heading in your html code. - The image is not decorative in this case so make sure that the
<img>element in your HTML code has an alternate (descriptive) short text. The reason for this is that screen readers can’t translate images into text. So to fix this you can do the following<img src=”…” alt=”short text” >
I hope you find it useful! 😄 Above all, the solution you submitted is great!
Happy
<coding />😎!0 - Wrap the page's whole main content in the
- @NafisMahmudAyonSubmitted almost 2 years ago@frank-itachiPosted almost 2 years ago
Hello there 👋. You did a good job!
I have some suggestions about your code that might interest you.
CSS 🎨:
- You can use grid or flexbox to center the content no matter the viewport size. Using Grid:
body { min-height: 100vh; display: grid; place-content : center; }FlexBox
body { min-height: 100vh; display: flex; align-items: center; justify-content: center; }- Avoid using absolute length units px, especially for font-size and width properties, because they are not relative to anything else so that means they will always be the same size. Instead, you can use relative lengths like em or rem. The benefit of that last one is element which has that unit will scale relatively to everything else within the page, e.g., the parent container. You can dig up about it here.
I hope you find it useful! 😄 Above all, the solution you submitted is great!
Happy
<coding />😎!0 - @SuivezSubmitted almost 2 years ago@frank-itachiPosted almost 2 years ago
Hello there 👋. Congratulation for completing the challenge👍!
I have some suggestions about your code that might interest you.
HTML 📄:
- Wrap the
<div class="attribution">section inside the<footer>tag. - you could also use the
<picture>tag that allows you to interchange the images depending of the viewport size. Red more about this awesome tag here.
CSS 🎨:
- Avoid using absolute length units px, especially for font-size and width properties, because they are not relative to anything else so that means they will always be the same size. Instead, you can use relative lengths like em or rem. The benefit of that last one is element which has that unit will scale relatively to everything else within the page, e.g., the parent container. You can dig up about it here.
I hope you find it useful! 😄 Above all, you did a good job!
Happy
<coding />😎!0 - Wrap the
- @Ahmed-abdulhamedSubmitted almost 2 years ago
my code for product-preview-card-component, I will appreciate any feedback on what i could change or could have done better
@frank-itachiPosted almost 2 years agoHello there 👋. You did a good job!
I have some suggestions about your code that might interest you.
HTML 📄:
- You could also use the
<picture>tag that allows you to interchange the images depending of the viewport size. Red more about this awesome tag here. - The perfume images are not decorative so make sure to add a valid descriptive alternate text.
I hope you find it useful! 😄 Above all, the solution you submitted is great!
Happy
<coding />😎!0 - You could also use the
