@rherndzSubmitted almost 2 years ago
!Segunda practica realizada! 😁🤞
🚀 ¡Cualquier comentario de ayuda o corrección de código es bienvenido! Estoy abierto a aprender y mejorar, así que no dudes en compartir tus conocimientos.
#HTML 🟠 #CSS 🔵
James Ebimobowei Osuobeni
@EbibeniAll comments
- @EbibeniPosted almost 2 years ago
Hi 👋 RHerndz. Good job on completing the challenge! I have some feedback for you if you want to improve your code.
-. Not all images should have alt text. Car icons are for decoration purposes only, so they can be hidden from screen readers by adding aria-hidden="true" and leaving its alt attribute empty(I see you left yours empty): <img src="./images/icon-sedans.svg" alt aria-hidden="true">
-. Instead of using pixels in font-size, use relative units like em or rem. The font-size in absolute units like pixels does not scale with the user's browser settings. This can cause accessibility issues for users who have set their browser to use a larger font size. You can read more about this here.
-. It is given that for Mobile the screen size is 375px. however, yours is set to 628px (actual figure). Your @media (max-width:700px) and @media (max-width:768px) are only for the border-radius and make the size and shape of your cards uneven.
**IF THIS WAS HELPFUL PLEASE MARK IT AS HELPFUL 🤩 ** I hope you find it useful! 😄 Above all, the solution you submitted is great! Happy coding!
Marked as helpful0 - @jsvigneshkannaSubmitted almost 2 years ago
Hi 👋, I built QR code component in reactJS and styled with tailwindcss. Please review my design and code.
This component is responsive to all devices and has dark-light mode theme
@EbibeniPosted almost 2 years agoGreat work 👋 Vignesh Kanna, You did very well with the design. Good job on completing the challenge
1 - @amooriheshamSubmitted almost 2 years ago
please rate it.
any tip to make the wrapper height be auto cause when I didn't sit its height the button stick to the bottom of the box and when hover it the wrapper height increased to make the button border.
@EbibeniPosted almost 2 years agoWelldone Amr_Hesham! You have done a great job on this project. I did go through your code and discovered that your media query for mobile is not correct. You have it set to 1020px instead of 375px. Here is how it should look like. @media (max-width: 375px) { .wraper { flex-direction: column; width: 330px; height: 100%; margin-top: 2rem; } }
IF THIS WAS HELPFUL PLEASE MARK IT AS HELPFUL 🤩
1 - @boredmahinSubmitted almost 2 years ago
Any suggestion would be helpful
@EbibeniPosted almost 2 years agoHi Mahin, Your work looks great and I would like to add a few. You need to make the cursor turn into a pointer on hover. you can do that by adding the code to the hover stage.
Example.
.card__title:hover { color: var(--first-color); cursor: pointer; } .card__images:hover .card__img-overlay { opacity: 1; cursor: pointer; } .card__creator-data span:hover { color: var(--first-color); cursor: pointer; }
IF THIS WAS HELPFUL PLEASE MARK IT AS HELPFUL 🤩
Marked as helpful0 - @annisa-rachmaSubmitted almost 2 years ago@EbibeniPosted almost 2 years ago
Hi Annisa, Nicely done Project you have here.
After looking at the work you have done I must commend your effort and work. Keep up the Good Work!
0 - @davidsonaguiarSubmitted almost 2 years ago@EbibeniPosted almost 2 years ago
Hi Davidson, Nicely done Project you have here.
After looking at the work you have done I must commend your effort and work. However, I noticed that your Github link isn't working ( 404 error ). Try fixing that.
Keep up the Good Work!
0 - @strael12Submitted over 2 years ago@EbibeniPosted over 2 years ago
Nice one I think you need to improve on the dimensions for your div tags. The width should be around 60-70% of the height. The image is stretched. You can use the "object-fit: contain" to make an image contained to the size of any given div. Grid is a fine tool to use but as for a beginner i would prefer using DISPLAY:FLEX as it is easier to control for this type of content.
Try to solve these problems one at a time. Nice work.
0
