Natia• 20
@natushkiss
Submitted
P
Q-bert
@QBERT18
All comments
Hello! Nice try! But there stuff to improve.
HTML Improvement:
- I like your creativity. You solved the image responsivenes with 2 different image tags, by switching them on and off. But that isn't the optimal way to do it. Read this https://developer.mozilla.org/en-US/docs/Web/HTML/Element/picture?retiredLocale=de or type "picture" html tag in google :).
- It is also creative that you used whitespace between the letters of your headline. Also you typed capital letters manually. Also not optimal way to solve that problem. Read this https://developer.mozilla.org/en-US/docs/Web/CSS/letter-spacing?retiredLocale=de and this https://developer.mozilla.org/en-US/docs/Web/CSS/text-transform?retiredLocale=de for better solution.
- section and aside tag are overkill for just 2 short price strings haha. Just use span or p or div. Read about the usage of section and aside to understand their usage.
CSS Improvement:
- You are giving individual width's to both of the left and right containers. Why not one width to the parent element? Just wrap them into section and give it 500px with instead of 250px to each container.
- Float is old use flex or grid.
- You are using to much px. Learn rem for more responsivenes.
- padding-top: 14px; padding-bottom: 12px; padding-left: 70px; padding-right: 70px; can be written shorter like this padding: 14px 12px 70px 70px; read more about padding here: https://developer.mozilla.org/en-US/docs/Web/CSS/padding
That were the short list of stuf i could find. I hope i could help you a little bit. Let me know if i did.
Best Regards Q-bert :).
Marked as helpful
0@chopinex
Submitted
Hello, nice try! But there many stuff to improve!
-
You should stop using % and vh/vw to often. Just stick with px for precice sizes and rem for relative sizes. For fonts you can also use pt if you want, but it depends on the developer.
-
Stop giving precice sizes to elements. Like don't force stuff to have fixed size. It is bad for responsive design. Work with rems and layout tools like flex or grid.
-
You should use more flex tools. I saw that you are setting stuff as flex, but then you are not using any alignment tools of flex.
Example: instead of using this margin: 16vh auto; use { display: flex; align-items: center; justify-content: center; } That will position everything inside the main tag in the middle of the screen.
- You are using background-image for the image on the left. Which isn't that bad. I am not a fun of it. Don't rely on it to much. You could solve this challenge with just a img tag.
That are few stuff i could find for now. Ich hope i could help you. Let me know if i could. If you have any questions then feel free to ask them :).
Best Regards Q-bert.
Marked as helpful
0-
Hey! Sorry but i'am seeing you guys posting the same comment on literally every solution you see. Stop please farming points by posting unhelpful comments. I mean yeah you are right on some points, but as a web developer i know that the stuff you saying aren't that necessary. So please stop cheating.
0- Marco Tsang• 50
@hkmarcoot
Submitted
What did you find difficult while building the project? Using flexbox correctly is pretty challenge to me. I find it difficult to put Card and Text in column, then Image and Description in row, and finally centre all the items in the middle of page.
Which areas of your code are you unsure of? This is the first time I use overflow: hidden to hide the two scroll bar on right and bottom.
Do you have any questions about best practices? Desktop-first or mobile-first: Which one is better? Or it depends?
Hi! nice solution. If you want to center anything horizontally than you can just use text-align: center property for any block element. The text-align will position any child element in the center of the calling element (div in this case). If you want to center any individual element inside his parent element you can use a css trick margin: 0 auto.
Vertical Centering is bit differente. There are no out of the box way to center stuff vertically. Thats why you should eather use flexbox or grid. After some challenges here i stated to become more grid user. I had issues with the flexbox expecially when i tried to make 2 same size columns. So maybe you try grid. Because grid doesn't have that problems.
You can compare your solution to mine https://github.com/QBERT18/product-preview-card-component-main :) Let me know if i could help you bit more. If you have more questions than just ask me :) Best Regards Q-bert.
Marked as helpful
0 - Ayush maheshwari• 10
@coddingeek
Submitted
how to do vertical center in cards in bootstrap???
Hey! there are 2 ways as i know to center vertically. Eather you use the transform property or you make your element to a flexbox or grid and use the justify-content or align-items properties to center stuff. Google about the two displaying methods. Let me know if that helped you. Best Regards Q-bert.
Marked as helpful
1 - Pedro Salviano• 60
@pe-salviano
Submitted
Project completed! All projects I am doing in my own way, after completing this project, I looked at the other solved ones and noticed that I can make the HTML code smaller. But I decided to leave it my way and apply this code reduction in the next projects.
My biggest difficulty was to put the color on the image, I couldn't find a solution, so if anyone has a tip just comment.
I'm open to suggestions and improvements in my code.
Persistence is the path to success 🚀💻
Hey nice work! I have a suggestion about your color problem. Here is a one way to solve it. Just add :after to you box, set top and left to 0 and give width and height 100% so it fits the original element. Then give the box a color and opacity. Don't forget to set content: "". Without the content property the before and after elements don't work. Your code should look like this .img-box:after { content: ""; position: absolute; left: 0; top: 0; width 100%; height: 100% background-color: purple; opacity: 0.5; } this is the basic way. There are other ways to set opacity to the color. But again google is your friend ;) Let me know if that helped you. Best Regards Q-bert.
0 - Aaron Staes• 10
@aaronsts
Submitted
Really enjoyed this little project, just have a question. The way I do my hover effect on the image, is there a easier/better way to do it? Right now I made it as a background-image for a div instead of an image tag
Thanks! :)
You can do it like me with the :after sudo element. Sudo Element don't works on the img tag selfe. You have to do it on the parent element don't forget to give the parent element position:relative. If you don't know why just google about positioning in css :) I hope i could help you.
.card-header:after { content: ""; position: absolute; width: 100%; height: 100%; top: 0; left: 0; background-color: any color you want; opacity: 0; border-radius: 10px; /setting the eye icon the center after you hover it/ background-image: url("./images/icon-view.svg"); background-repeat: no-repeat; background-position: center; } .card-header:hover:after { opacity: 0.8; /* to make it transparent / transition: 500ms all; / little bit of animation / cursor: pointer; / turn the standard cursor to an finger pointing */ }
Marked as helpful
1
