Skip to content
OverviewSolutions
18
Comments
10
Reza Sajjadian
@reza-sjdn

All comments

  • Reza Sajjadian•290
    @reza-sjdnPosted over 1 year ago

    Hey dear Ahmet

    Instead of sections, you could use articles and it's more suitable because of the independence of client reviews!

    But your style is great and almost pixel-perfect! Good Luck

    Marked as helpful
    1
  • G0maa•30
    @G0maaSubmitted over 1 year ago

    Hello again :)

    I think this is not the best "responsive" implementation (if it is 😬), so I appreciate any reviews I get.

    Thanks in advance.

    Product Preview Card

    2
    Reza Sajjadian•290
    @reza-sjdnPosted over 1 year ago

    Hi @OoMiDOoO, Good Job!

    Your work at mobile view is great but at desktop view is a little bad. Set width: 50% and height: 100% for the #perfume-img. This way it will likely be ok.

    but anyway Keep going your journey!

    1
  • Reza Sajjadian•290
    @reza-sjdnPosted over 1 year ago

    hey @YoungZV, Good Job

    I have some recommendations for you.

    • First, you set margin-top: 1rem for .article__2 through .article__5. You didn't really need that. Instead, increase the gap between articles. This causes article__1 and article__2 to not being on the same line in desktop view!
    • Second, you have multiple h1 elements and this is not allowed in semantic html! Instead use p or h2 through h6 elements.

    Except of above, You made it successfully!

    Marked as helpful
    0
  • PJ•110
    @Kitezh1Submitted over 1 year ago

    I found it difficult to use the media queries, to convert it to mobile design. Specifically I'm not sure why the 100vh wouldn't work for the mobile screen size. Any tips are appreciated.

    Preview Card, using Flexbox & media queries.

    1
    Reza Sajjadian•290
    @reza-sjdnPosted over 1 year ago

    Hey @Kitezh1,

    I liked your solution. It was PERFECT! About why height: 100vh; doesn't work at mobile view, in fact height: 100vh; is a fixed length and to be flexible you must set min-height: 100vh; That's it! Good Luck

    0
  • Reza Sajjadian•290
    @reza-sjdnPosted over 1 year ago

    hey @Mohammad-Mahfoud

    The structure of your solution(not considering its responsibility) is great. Only I guess you haven't read style guide file at all. Match your solution's style with the style guide.

    0
  • Levi Ribeiro•140
    @ribeiroLeviSubmitted over 1 year ago

    Feel free to tell me about any mistakes ou things i didn't do in the best way, any feedback is welcome, thanks in advance :)

    Results summary component

    2
    Reza Sajjadian•290
    @reza-sjdnPosted over 1 year ago

    hello dear @ribeiroLevi, well done, everything's almost perfect but

    1. It's not responsive and you haven't designed mobile view
    2. you didn't set any box-shadow on white section
    3. and maybe it's a little bit big to my eyes

    anyway, your attempt is worth a lot!

    1
  • Reza Sajjadian•290
    @reza-sjdnPosted over 1 year ago

    Hi @DezzaAnnePeregrina, Good Job! Overall your solution is great but I have some small fixes to make your solution more responsive.

    In CSS file:

    1. In selector .card, set max-width: 900px;.
    2. In selector .card__item, instead of width: 19rem; type flex: 1;.
    3. In @media, set the break-point at max-width: 687px.
    4. and finally in @media in selector .card, set the margin: 2rem; 3.5rem is much. besides, if you set border-radius: 10px; , it would be more beautiful!

    I hope It will help you. GOOD LUCK : )

    Marked as helpful
    0
  • Reza Sajjadian•290
    @reza-sjdnPosted over 1 year ago

    Hey @sandrvvu, I have some advises for your solution.

    • First, If you use h2 elements instead of h1 elements, it would be better. because h1 has an important markup role and there should be just one of them in a page.
    • Second, Your solution doesn't look very good at width like 700px. In fact in my opinion, you just need one break-point at width like 992px! You can check my solution if you would like.

    Anyway, Your solution is awesome and deserves admiration. GOOD LUCK

    0
  • aulia•70
    @powrezeSubmitted over 1 year ago

    Hi there! I'm Aulia 👋 this is my solution for Order Summary Card.

    Any feedback would be really appreciated.

    Thank you 😀✌

    Order Summary Card

    1
    Reza Sajjadian•290
    @reza-sjdnPosted over 1 year ago

    Hey @powreze,

    I looked at your solution's code and figured out it's really perfect. You're great.

    But maybe I could give you some small advises.

    • If you'd increased the distance between annual plan and $59.99/year it would've been better. additionally, if you set line-height: 1.5; for content paragraph, it would look neater.
    • For limiting your content box from around, try to handle content width, like instead of width: 100%; use width: 85%;. This way you don't need to decrease padding for smaller medias and preventing overflow of price box content!

    Anyway I'd love your code! GOOD LOCK!

    Marked as helpful
    0
  • Reza Sajjadian•290
    @reza-sjdnPosted over 1 year ago

    You've done this challenge well but you didn't consider some details.

    • you had better increase border-radius of sub-boxes of summary from 5px to 10px.
    • you didn't set any hover effect on continue button and its original background-color is dark blue not black.
    • this is not so important but in header, the word "Great" is a bit close to its following paragraph.

    I hope these will help you. GOOD LUCK.

    Marked as helpful
    0