Skip to content
OverviewSolutions
12
Comments
5
Peter Hannell
@peterhannell

All comments

  • Peter Hannell•200
    @peterhannellPosted over 2 years ago

    D'oh... I forgot the <footer> section :-) I always forget something. I'll add this later on. Also my padding and margins are slightly off as I usually reset everything and write all the CSS from scratch. I was a bit lazy this time but I wanted to focus more on the JavaScript and not the styling!

    Thanks!

    0
  • Peter Hannell•200
    @peterhannellPosted over 2 years ago

    Hi Mouad, good job with your solution. I have a couple of suggestions:

    1. your <html> tag is missing a language attribute. You can correct this by using <html lang="en">
    2. in regards to your question about changing the colour of the image: you first need to apply a background colour to your image wrapper, then you can reduce the opacity of your image element with the opacity: property in your CSS file.

    Hope this helps!

    Marked as helpful
    0
  • Chloe•20
    @chloeafpSubmitted over 2 years ago

    Hello,

    I think I've done pretty well with the challenge. But I am always open to any feedback! :)

    So, do you have any feedback on the HTML or CSS? Thank you!

    Card component using HTML and CSS

    2
    Peter Hannell•200
    @peterhannellPosted over 2 years ago

    Hi Chloe, I'm a fellow learner but I can offer some feedback/advice.

    Firstly make sure you include enough 'landmarks' in your HTML - these are tags like main nav footer etc., that define sections of content in your page which greatly improves accessibility for assistive navigation technologies such as screen readers. For example, you should wrap your 'attribution' class in a footer tag.

    You should also try to make your class names a bit more descriptive; for instance 'header' is a very generic and could refer to any header section within your page. This could be somewhat confusing when it comes to maintaining code later on. Maybe something like 'card__header' would be more appropriate just as an example.

    Finally, try to avoid the use of px for sizing everything in your CSS. You can certainly use it in some cases but most of the time will you want to use rem (and also em in specific cases) which is scalable and respects the users' browser font size preferences.

    But other than that you've done a great job! I hope my comment was helpful.

    Marked as helpful
    1
  • Peter Hannell•200
    @peterhannellPosted over 2 years ago

    Hi :-)

    Your CSS isn't linked properly. Make sure you include ./ before the path to your styles.css in your HTML head. Your code should read like this:

    <link rel="stylesheet" href="./styles.css">

    You also don't have any images in your github repository so you need to include them there.

    Hope this helps!

    0
  • Marek•60
    @kozlowskimarekSubmitted almost 3 years ago

    Is my solution correct? Thank You!

    Order Sumary

    2
    Peter Hannell•200
    @peterhannellPosted almost 3 years ago

    Hi @kozlowskimarek!

    Good job. However, your HTML mark-up is missing a <main> and <footer> landmarks. Other than that your solution looks good.

    0