Skip to content
OverviewSolutions
1
Comments
1
ieeah
@ieeah

All comments

  • ieeah•40
    @ieeahPosted almost 3 years ago

    Hi Sayed, first of all good job! Id' like to give you a few advices which would, in my opinion, improve your code :)

    1. You used a lot of different <article> inside of the main container, but single div, containing two different sections would have been a lott easier to style, and also more correct, since everything is part of the same element, and not different elements. For Example:
    <div class="container"> <div class="card"> <div class="cover"> <img class="square" src="" alt="" /> </div> <div class="details"> <h3> title </h3> <h5>subtitle</h5> ecc ecc </div> </div> </div>

    this would also make a lot easier the targeting of the elements in the css, I've seen you used a lot of :nth-of-type, which is really cool that you know, but used like this are a lot of energy and effort which could be used on a better way.

    1. about the <article></article> tag itself, it would'nt be semantically correct: The articole tag is used for shop-articles, or blog-articles; of course you can use it as you prefer, but i think a regular div with appropriates classes would be better, but probably that's just my opinion :p

    2. in the css, from line 119 to line 134, you use a lot of flex, and margins mixed together, but would had been a lot easier to a) not using <ul> and <li> but just using a regular <div> with "display: flex;" and "justify-content: space-between;" to reache the same result (maybe i'm missing some reasonss for which you decided for that ath, if it's the case, sorry :p)

    Anyway, keep working and develop on!

    Marked as helpful
    1