2
\$\begingroup\$

I would appreciate some feedback on this single price grid component project completed using HTML and CSS.

I am trying to recreate the content shown by this image: https://res.cloudinary.com/practicaldev/image/fetch/s--FzGw2rbZ--/c_limit%2Cf_auto%2Cfl_progressive%2Cq_auto%2Cw_880/https://dev-to-uploads.s3.amazonaws.com/i/3y0bhisv135j7979dk3m.jpg

HTML file

<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <link rel="stylesheet" href="styles.css">
    <title>Price Grid</title>
</head>
<body>
    <div class="container">

        <div class="description">
            <h2>Join our community</h2>
            <h3>30-day, hassle-free money back guarantee</h3>
            <h3>Gain access to our full library of tutorials along with expert code reviews.
                <br>Perfect for any developers who are serious about honing their skills.
            </h3>    
        </div>

        <div class="pricing">
            <h3>Monthly Subscription</h3>
            <h2>$29 per month</h2>
            <h2>Full access for less than $1 a day</h2>
            <button>Sign up</button>
        </div>

        <div class="featuredContent">
            <h2>Why us?</h2>
            <ul>
                <li>Tutorials by industry experts</li>
                <li>Peer & expert code review</li>
                <li>Coding exercises</li>
                <li>Access to our GitHub repos</li>
                <li>Community forum</li>
                <li>Flashcard decks</li>
                <li>New videos every week</li>
            </ul>
        </div>

    </div>
</body>
</html>

CSS File

html {
    margin: 0;
    padding: 0;
    font-family: 'Abel', sans-serif;
}

body {
    background-color: rgb(230, 230, 230);
}

.container {
    position: absolute;
    left: 50%;
    top: 50%;
    transform: translate(-50%, -50%);
    width: 800px;
    height: 500px;
    border: 20px solid rgb(20, 148, 20);
    border-radius: 25px;
}

.description {
    padding: 20px;
    text-align: left;
    background-color: rgb(255, 255, 255);
}

.pricing {
    position: relative;
    float: left;
    top: 5.5%;
    height: 55%;
    width: 50%;
    color: white;
    background-color: rgb(43, 179, 177);
    text-align: center;
}

.featuredContent {
    position: relative;
    float: right;
    top: 5.45%;
    color: rgb(255, 255, 255);
    background-color: rgb(74, 190, 189);
    height: 55%;
    width: 50%;
}

.featuredContent h2 {
    text-align: center;
}

.featuredContent ul {
    position: relative;
    list-style: none;
    left: 20%;
}
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

HTML

Several elements are marked up as headlines, although they certainly aren't ones:

  • The text "Gain access..." in the description element should be two(!) paragraphs. (Don't use <br> to create paragraph-like line breaks.)

    <p>Gain access to our full library of tutorials along with expert code reviews.</p>
    <p>Perfect for any developers who are serious about honing their skills.</p>    
    
  • "$29 per month" and "Full access..." in pricing shouldn't be headlines either - especially not h2s after a h3. Also the price ("$29") itself needs to be markuped separately, since it's highlighted compared to the rest of the line. For example:

    <div class="price"><strong>$29</strong> per month</div>
    <p>Full access for less than $1 a day</p>
    
  • "Why us?" looks identical and is structurally identical to the headline "Monthly Subscription" in pricing, so it should have the same headline level.

The "button" "Sign up" functionally looks more like a link, than a <button>. (Buttons are primarily used to submit forms, which this is not.)

The class names are too generic and so can collide with other classes used on the site. Instead of container something like price-component would be more appropriate. And for the inner components either use a child/descendant selector in the CSS so that the rules only apply to your component:

.price-component > .description

Or use a CSS naming scheme (such as BEM) to create a unique class names for the inner components such as price-component__description.

CSS

Avoid absolute positioning. Centering the price component like that is only appropriate, if it's only used as a modal dialog, that covers the rest of the content of the page and the screen shot doesn't seem to indicate that. In order to "just" center it, use CSS grid or flex.

Also the use of relative positioning isn't appropriate. On those floats it is strange. It just creates a gap where there shouldn't be one and lets the elements overlap the bottom border. And on the list use either a left-margin or just a general padding on the featuredContent element.

Float is not appropriate for layout any more. This component is the perfect example for a grid layout.

Hard coding the width and height in pixels makes the component unresponsive to the screen and font size. For the max-width use relative units such as rem and don't set the height at all, letting it resize dynamically based on the content size. Also consider an alternative media query based layout for smaller screen sizes where it would be too wide.

Don't center all the text. For one, it contradicts the screen shot, but more importantly centered text is difficult to read.

Example: https://jsfiddle.net/vjr2ogpq/

\$\endgroup\$
1
  • \$\begingroup\$ Thanks so much and such a beautiful design you've made. \$\endgroup\$
    – King Cold
    Nov 24, 2020 at 16:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.