@Jo-with-vision
Posted
Hi,
Great work on the challenge, here's some little bits of feedback for your consideration:
Switching the image: So I see you used JavaScript as the solution to changing the image - this could have been achieved without any JavaScript at all, by using the html picture element. That's something to consider if you weren't already aware of that.
Responsive units: I would advise for best-practices to avoid using pixel units and use em/rem units for all font-sizes, padding, margins.
custom properties for fonts and font-weight I see you have created a class to apply your fonts and weight, but this could be more easily achieved in your custom properties alongside the colours like this:
--font-family-secondary: 'Fraunces', serif;
--font-weight-700: 700;
...and similar for the primary font and its weights
The benefit being you could then utilise the separate font-weight custom property elsewhere and also use Fraunces elsewhere with a different font-weight, instead of having them 'tied up' in the same class. Just something to keep in mind.
use max-width: we always try to avoid setting widths when possible. If a challenge needs a limit to the width of a component, set it as max-width not just width. It is also becoming very standard to use em/rem for widths (including widths in media queries) instead of pixels, or even percentage. This is all to increase responsiveness and aid accessibility.
button states: :active and :focus are generally included alongside :hover to cover all ground. You will notice when using the tab key on your keyboard to tab to the button on your live preview, the button colour does not change. If you were to include focus then the colour would change when tabbing. This helps to indicate the state change to a user using a keyboard.
extra challenge: You have included the SVG icon as an img in your html. Research how you could use a CSS pseudo element to include the icon in the button instead for an optional challenge. The benefit of using pseudo elements for decorative icons is that it keeps decoration clutter out of the html document structure.
(If you decide to leave it in the html: As the button includes text saying what the button does, you don't need to include an alt description for the icon. You can leave the alt description blank like this alt="". The assistive technology will skip it and just read the button text to "add to cart".)
Happy Coding!
