hello
Q-bert
@QBERT18All comments
- @TheMakDevSubmitted 4 months agoWhat are you most proud of, and what would you do differently next time?@QBERT18Posted 4 months ago
Hi there, Great job on your solution—it’s coming along nicely! I do have a small suggestion for improvement.
Instead of relying on width: 80%, consider working more with padding. Using padding can provide better spacing and flexibility, especially when designing for responsive layouts.
Feel free to ask if you’d like further clarification or additional suggestions—I’d be happy to help!
Best regards, Q-bert
0 - @m1st1nh0Submitted 4 months agoWhat are you most proud of, and what would you do differently next time?
i'm most proud of completing it, and i hope next time it could get easier.
What challenges did you encounter, and how did you overcome them?i got a lot of obstacles in the way of my omelette, sometimes i almost give up (yes im very newbie), but with some AI i could learn where my mistake is and solve it.
What specific areas of your project would you like help with?how i could make it more similar to the real one
@QBERT18Posted 4 months agoHi there,
I had a chance to review your code and noticed a few areas for improvement, particularly in the design of your CSS. It seems you’re aiming for a utility-based CSS class approach, which can be beneficial. However, it’s important not to overuse utility classes for every element. Additionally, improving the naming conventions of your utility classes could enhance readability and maintainability.
For example: • brown → This could be clearer. • bg-brown-800 → A more descriptive and consistent approach.
Using clear and meaningful class names not only helps you but also makes the code easier for other developers to work with.
Another suggestion is to move your CSS into a separate file instead of embedding it within the <head> of your HTML. This improves code organization and makes your project easier to scale.
Lastly, I noticed that SEO could use some attention. While I understand this is a simple page, and SEO might not have been a priority, it’s still an important aspect. A well-optimized page can significantly improve its visibility on search engines like Google, which is often overlooked but crucial for your project’s success.
Feel free to reach out if you need more help or have any questions—I’d be happy to assist further!
Best regards, Q-bert.
Marked as helpful1 - @natushkissSubmitted over 2 years ago@QBERT18Posted over 2 years ago
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 helpful0 - @chopinexSubmitted over 2 years ago@QBERT18Posted over 2 years ago
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 helpful0 -
- @QBERT18Submitted over 2 years ago@QBERT18Posted over 2 years ago
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 - @hkmarcootSubmitted over 2 years ago@QBERT18Posted over 2 years ago
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 helpful0 - @coddingeekSubmitted over 2 years ago@QBERT18Posted over 2 years ago
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 helpful1 - @pe-salvianoSubmitted over 2 years ago@QBERT18Posted over 2 years ago
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 - @aaronstsSubmitted over 3 years ago@QBERT18Posted over 3 years ago
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 helpful1