I created this testimonial section as a prototype.
testimonial concept
HTML
<div class="testimonials">
<blockquote>
<p>This was pretty good.</p>
<cite>
<span class="author">– Bobby,</span> Jersey City, NJ
</cite>
</blockquote>
<blockquote>
<p>This is pizza is the most bearable thing I ever had in Joisey. But it still sucks, just like everyone from there.</p>
<cite>
<span class="author">– Vinny,</span> New York, NY
</cite>
</blockquote>
<blockquote>
<p>I really savored the smoky undertones of the imported sausage. The cheese had a wonderful texture. It paired well with my pinot noir in a dance of sensations.</p>
<cite>
<span class="author">– Alex,</span> San Francisco, CA
</cite>
</blockquote>
</div>
CSS
.testimonials {
width: 720px;
max-width: 96%;
margin: 0 auto;
}
.testimonials blockquote {
background-color: #fff;
border-left: 4px #61acca solid;
font-size: 21px;
line-height: 1.6;
}
.testimonials blockquote {
padding: 10px 20px;
}
.testimonials blockquote {
background-image: url('http://shared-assets.s3.amazonaws.com/codepen/img/elements/quotes/quote-999.jpg');
background-repeat:no-repeat;
background-size: 33px 45px;
background-position: 10px 5px;
}
.testimonials cite {
font-size: 0.7em;
font-style: normal;
}
.author {
font-weight: bold;
font-size: 1.3em;
margin: 0 6px 0 36px;
}
p { margin: 0 0 0 36px; }
2 Answers 2
Not bad at all. I wish we could see more of this kind of code online! However, allow me to make a few suggestions.
HTML:
- Technically this is a list of testimonials. So I would suggest using a unordered list.
- Where you use the
cite
tag, I would use thefooter
oraddress
tag in stead. - I would place your author inside a
cite
element - The
-
in font of the name can be considered styling, not content, so I would use a:before
pseudo element inside the css to add it
That means the HTML would result in something like this:
<ul class="testimonials">
<li>
<blockquote>
<p>This was pretty good.</p>
<footer>
<cite class="author">Bobby,</cite> Jersey City, NJ
</footer>
</blockquote>
</li>
...
</ul>
Note that the cite
/ footer
/ address
thing is open for discussion. Your way should be fine as well. It is just the way I would do it...
CSS:
- As said by @JosephTheDreamer you should indeed keep your
font-size
units consistent. Personally I would choosepx
however. Just because I find them easier to work with. And the days we neededem
to enable browsers to resize fonts are luckily behind us. Eather should be fine though, just stick with you choice. - Why do you repeat your
.testimomials blockquote
selector 3 times? Just keep them in one block. You could add whitespace for good readability, but repeating the selector will just make it harder to maintain your code and even slow down the processing. - Rather then adding a
margin-left
of36px
to youp
and.author
, would would add an extra36px
ofpadding-left
to your wrappingblockquote
. This way they are easier to maintain, and you eliminate the need for youp
selector. - Namespacing your css is indeed a good idea. Just be careful not to overdo it. The longer your selector gets, the slower and heavier it becomes. Also you are in danger of writing a lot of styles multiple times. In the case of your blockquotes i.e. one could wonder if there are going to be other blockquotes on your site that are going to look entirely different. Chances are that you want a uniform style for your website, and that other blockquotes will have to look almost the same. I would suggest putting all styles that apply to all blockquotes in a simple
blockquote
selector. Anything that is different then the 'normal' style can be put inside the namespaced selector you are using. - There should be no need to set your
font-style
tonormal
on yourcite
element. Defenitly if you are using some sort of reset or normalize css (which you should). This means you can ommit that rule. p { margin: 0 0 0 36px; }
Please don't, and I mean don't, put your selector and your properties on the same line! It looks awfull and makes it harder to maintain your code, or find something quickly. Use a minimizer for your production css, but keep your selectors and properties each on their own line in your dev version!- I actually disagree with @JosephTheDreamer on resizing your background image. I quite often bring my images to halve their size to make them look nice and crisp on hegh resolution (retina) screens. Just be careful not to make your image much bigger then that, as it will just increase the load time. I would even go for a icon font inside a
:before
pseudo element in the case of that quotation mark. It is probably smaller, renders sharp on all screens, and can be scaled and colored as you want trough css, as it is in fact a vector. Something like icomoon can make this very easy for you.
That's about it I think. And again, note that I have been nitpicking here. This is codereview after all. Your code is quite fine as it is, and I would probably hire any developer that writes code of this quality.
-
\$\begingroup\$ Good response, I'll also add you could go even further with using Microdata/RDFa for authors/location as well and add a
<time datetime="">
for when the testimonial occurred. \$\endgroup\$darcher– darcher2014年06月25日 22:44:58 +00:00Commented Jun 25, 2014 at 22:44 -
\$\begingroup\$
<cite>
shall not be used for Peoples names but for the title of a work. HTML 5 Specs: "A person's name is not the title of a work — even if people call that person a piece of work — and the element must therefore not be used to mark up people's names." html.spec.whatwg.org/multipage/… \$\endgroup\$Andreas Riedmüller– Andreas Riedmüller2020年01月13日 10:09:36 +00:00Commented Jan 13, 2020 at 10:09
Everything looks clean. However, I found some things though:
font-size
- Make the units consistent. You usepx
andem
. I suggest you goem
all the way.background-size
- Well, it exists for that reason, but it would be better if you actually sized the image in those dimensions rather than having them resized in CSS.You "namespaced" everything by prefixing
.testimonials
. That's being a good developer. You're avoiding your styles from clobbering other elements. However, you forgot to do that for.author
andp
.
-
\$\begingroup\$ No suggestion of the
rem
value withpx
fallbacks? ;) \$\endgroup\$darcher– darcher2014年06月25日 22:47:26 +00:00Commented Jun 25, 2014 at 22:47
inherit
settings. \$\endgroup\$