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 card solution using HTML and CSS

@tab21

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


Please provide feedback

I also wanted to know how to make my CSS code neater or is there any rule in it like in html we indent?

Community feedback

P
Katrien S 1,070

@graficdoctor

Posted

Hey there, this looks rather good. You almost nailed it but bumped into a few issues in regards to accessibility, semantic html and some css-mistakes.

  • If you're using headings in html, you should always begin with h1. A screenreader will always look for the most important information first. You declared 'annual plan' as h4. Which is wrong for a few reasons. Because you used it inside a span, because 'annual plan' isn't a title, and because you didn't respect the hierarchy of an h1 first. Though, in this exercise, write
<p>
<span class="annual-plan__title">Annual Plan</span>
$59.99/year
</p>

As for span: The <span> tag is an inline container used to mark up a part of a text, or a part of a document.

  • As for the background; you didn't add a color, nor does the image span across the entire page. Try adding this to the body, including the link to the mobile image. (I will explain later).
background-image: url("images/pattern-background-mobile.svg"); 
background-size: contain;
background-position: top;
background-color: hsl(225, 100%, 94%);
  • No need to put the 'proceed to payment' inside a div. But add in css a display: block;

  • Typing error in your css for .payment: margin: 0 30px 20px;;

  • Bundle code. For .payment and .cancel you wrote text-decoration: none;. You could actually add this to a when you are declaring your typography.

  • In you solution min-width equals max-width. Which is not really good. This is not how to set breakpoints. Part of the solution is to start mobile first and then move on to desktop. As for the max-width, even if they in the assignment say 'minimum size is 375px', there are still phones who go smaller. iPhone SE for example has a 320px-width and is still widely used. To set the min-width, try to guess yourself where it would be more beautiful for the mobile-image to change into the desktop-image. I chose, in my solution, a min-width of 650px. ex.

@media screen and (min-width: 650px) {
body {
  background-image: url('./images/pattern-background-desktop.svg');
  background-size: contain;
  background-position: top;
  }
}

As for writing a more neat code, there are no real rules in css besides: 'respect the cascade', 'code that is similar can be put together (like the text-decoration)' and as you move on, you will learn about variables and such that make code look 'neat'. But yours looks good. It's tidy, concise and good to read. Keep going!

Marked as helpful

0

@tab21

Posted

@graficdoctor Thank you for such a detailed analysis and explanation. I will just start working on it. It was really helpful.

0

@JulianJ44

Posted

Hi @tab21, there is a site that is really good for cleaning up your CSS and HTML https://html-cleaner.com/css/. If you have VScode as your editor you can get the extension called Prettier which is a code formatter.

Hope the above helps.

Julian

0

@tab21

Posted

@JulianJ44 Thank you

I don't use VScode as web dev editor but I do have it so will definitely try it out

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