Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

Order Summary Component using HTML and CSS (Flexbox)

José Pessoa• 65

@jgbpessoa

Desktop design screenshot for the Order summary component coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


Hey Guys!

How are you doing?

I've started my journey to become a Front End Developer a month ago and I would love to receive some feedback! This is my first challenge o/

I've tried to practice Flexbox which is the latest layout technique I've learned. I just feel like sometimes I complicate things instead of making them easier haha so if you guys could take a look at my code and give me tips on how to optimize it I would love that! I also had a little trouble using semantic HTML for this challenge so I don't know if i chose the right tag.

Thank you!

Community feedback

Shahin NJ• 1,190

@SJ-Nosrat

Posted

Hi Jose, Great solution! Well done.

  1. With respect to semantic HTML I'd wrap everything inside your <body> with the <main> tag.
  2. I'd remove width: 100vw; from your .card class and keep the max-width: 100%;
  3. Your use of CSS Flexbox is fine.
  4. I'd look into your card in the mobile view; the card is too wide and is not giving enough breathing room between the edge of the screen and the card; do you approach the challenge with a mobile first approach? The issue here might be: width: 400px; on the .container class in your CSS.
  5. I think removing width: 400px; from the .container class and adding a margin: 0 1.5rem would help, although it does squeeze some of your other content inside the card; I'd look into restyling those.

It can be hard coding the challenge just by eye-balling it. You're doing really well! Keep on coding and best of luck with your coding journey; looking forward to more of your solutions.

Cheers!

Once again well done

Marked as helpful

2

José Pessoa• 65

@jgbpessoa

Posted

@shahin1987

Thank you so much!

I completely forgot about the mobile view and the mobile first approach haha Thanks for reminding me!

I'm gonna update this solution with your suggestions and also add a better looking summary component for mobile screens. I just gotta say thank you again for taking your time to review my code.

Have a great rest of the weekend!

1
Shahin NJ• 1,190

@SJ-Nosrat

Posted

@jgbpessoa My pleasure! Have a wonderful weekend too.

1
Vanza Setia• 27,855

@vanzasetia

Posted

@shahin1987 I would recommend everything in your page content should be wrapped with main tag, except the attribution. I will use footer instead of div for my attribution.

2
Shahin NJ• 1,190

@SJ-Nosrat

Posted

@vanzasetia Didn't notice that; good catch!

0
Vanza Setia• 27,855

@vanzasetia

Posted

👋Hi José Pessoa!

I have some feedback on this solution:

  • For any decorative images, you should leave the alt="" empty and add aria-hidden="true" or role="presentation". In this case all images are decorative only.
  • I would recommend to wrap any text with meaningful tag, like in this case, the Annual Plan and the price should be wrapped in paragraph tags.
  • Why did you do something like this? If there's no reason, then I recommend to just remove the id and just write # for the href value.
<div class="change" id="change">
              <a href="#change">Change</a>
            </div>
  • On landscape mode of mobile view (640px * 360px), your card get cut off.
  • Use rem or sometimes em units instead of px. Using px will not allow the use to scale the size of your page.

That's it! Hopefully this is helpful!

Marked as helpful

1

José Pessoa• 65

@jgbpessoa

Posted

@vanzasetia

Thank you, Vanza!

I really appreciate your feedback! I did the #change just to see the unvisited state because using only # was showing the visited one. I was gonna change back to just href="#" and I forgot haha I think I should use pseudo-classes to style these states, right?

Thanks for taking your time to review my code! Can't wait to update this solution and also start new challenges. I'm really enjoying this platform and you guys are making me feel encouraged to study more. So thank you again!

Have a great weekend!

0

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord