Developer Matusala A.
@matusalab-devAll comments
- @Fariv@matusalab-dev
Congrats, bro. for achieving this far. at the first, I don't think react class component is best practice in 2023, it deprecated feature of React. I recommend you to use React functional components. Take the validation logic, put it into a custom hook that would be reusable.
- @integeek@matusalab-dev
congrats for achieving this far. the problem with the image is that you didn't specify the right path at the src attribute of the img tag. get the relative path of the images from the assets folder
- @chinaza-Sommie@matusalab-dev
congrats on achieving this far, I found a bug on the [learn more] cta button when you hover on the button the text will disappear. make sure the text and background are having good color contrast
- @momin-riyadh@matusalab-dev
congrats for achieving this far... I've got a couple of suggestions, let's start with the animation when the cards return to their original position the overlapping of the card's transition isn't smooth. you can make the animation using a CSS property z-index. because the z-index property is animatable. the other one is responsiveness, your app is not responsive. you can make it responsive with a CSS grid or simply just use the flex-box flex-direction property from row to column at an appropriate breakpoint.
- @Taylan0125@matusalab-dev
congrats on getting this far, I've some suggestions because your layout isn't responsive. and on your CSS file, I notice that you use flexbox. keep learning Flexbox then try to make it responsive using flex properties like flex-direction and flex-wrap to make them stack one on the other at a specific breakpoint. if you can try to use CSS Grid. If You can't it's okay, just using flex-box is fine for this landing page. I highly recommend you to read Zell's blog post to build the mobile navigation bar [https://zellwk.com/blog/js-in-dom/] and others of his blog post.
- @antoru@matusalab-dev
congrats on making it this far, it's hard to find bugs or improvements on your CSS file, but, there is an accessibility issue in HTML markup by not using semantic tags. like div for everything instead, you can use h2 for the title, p for the description, and write a description of the icon in the alt tag. thanks, I hope this will help improve your accessibility issue.
Marked as helpful - @ignaciofigueroadev@matusalab-dev
congrats for making it this far, the only bug that I found is in the layout justify content center breaks the layout at some point( tablet mode). try to use media query then set the justify content to flex-start at that break point.
Marked as helpful - @yoqedo@matusalab-dev
I think it's better to use details and summary semantic HTML tags for better accessibility.
- @Jey223@matusalab-dev
I didn't solve the challenge but from my perspective, refactor the toggle section by storing the color,background, and box-shadow because they're the same. in case you want to change the value of these properties, you have to go through each individual manually. they're not reusable. the second one is the array of buttons, you don't have to set the index explicitly you can use a loop. that's it. I hope this'll help you to refactor your code.
Marked as helpful - @Renzorr@matusalab-dev
the design and the solution aren't the same.
- @stkhalisha@matusalab-dev
it's hard to find an issue in your code. your code is readable and semantic. the only thing that I try to avoid is the overflow property.
Marked as helpful - @akhribabderahmane@matusalab-dev
let's start from the semantic part: I recommend you to use a section other regioning / sectioning semantic HTML to your container, a heading tag for your "perfume name", "p" tag is perfect for description, I think using a "p" tag for the price numbers rather "span" that are non-descriptive. the other is a layout problem the container is not perfectly centered. the right section doesn't have enough space as the design, giving both the same width will help to fix. last one, stop using the position property for this layout it's overkill instead use the flexbox alignment property. and try to determine the height of container by the content of the items.hope, this will help you to improve.
Marked as helpful