5
\$\begingroup\$

I'd like this reviewed.

Demo

HTML

<body>
    <!-- Introduction to the text shadow property -->
    <div class="container one">
       <h1>The Dark Knight</h1>
    </div>
    <!-- Show the inset shadow 'hack' -->
    <div class="container two">
       <h1>The Dark Knight Rises</h1>
    </div>
</body>

CSS

    body{
        background: #424243;
        margin: 0;
    }
    h1{
        font-size: 5em;
        font-family: Georgia;
    }
    .container{
        padding: 20px 50px;
    }
    .one h1{
        color: #fff;
        text-shadow: 4px 3px #222;
    }
    .two{
        background: #ccc;
    }
    .two h1{
        background-color: #222;
        color: transparent;
        text-shadow: 0px 2px 2px rgba(255,255,255,0.6);
        -webkit-background-clip: text;
            -moz-background-clip: text;
                background-clip: text;
    }
\$\endgroup\$
  • 3
    \$\begingroup\$ It would really help to have a description of what you have there, what its purpose is, etc. \$\endgroup\$ – kleinfreund Feb 12 '14 at 14:52
  • \$\begingroup\$ @user1668270 What makes you think the use of text-shadow is a hack? Also, it is very helpful if you provide a demo (see: jsfiddle.net, codepen.io, etc.) \$\endgroup\$ – cimmanon Feb 12 '14 at 15:59
  • \$\begingroup\$ I've added a demo now. In light of the changes to the CSS it no longer seems like a hack. It did seem like one when I was clipping the h1 container background to fit the h1 text. \$\endgroup\$ – icottam91 Feb 12 '14 at 16:09
  • 1
    \$\begingroup\$ @user1668270 This isn't a hack, too. This is the purpose of background-clip. ;) Please add a description to your question, because our ways of reviewing are limited without one. \$\endgroup\$ – kleinfreund Feb 12 '14 at 16:11
6
\$\begingroup\$
  1. You're using prefixed versions for the background-clip property. There is only a need for the -webkit- prefix here, so you have support for Android 2.x. You can omitt -moz-, unless you need to support Firefox 3.6.

    Can I use CSS3 Background-image options?

  2. It looks like you're trying to illustrate the styling of text with text-shadow. You probably want to use example text with a higher font-size. Instead of heading elements, you should be using span's:

    <span class="demo-snippet">The Dark Knight</span>
    

    h1h6 are for headings and subheadings. In your case a heading would be something like Usage of the text-shadow property.

  3. Use 0 instead of 0px. You don't need to add the unit for zero values.

Other than that, I don't know what else one should review. I'll update my question, if you add some more information.

\$\endgroup\$
  • \$\begingroup\$ Thanks Kleinfreund. I've made some changes based on what you've said. I'm using the above for training material to demonstrate how to use the text-shadow property effectively. \$\endgroup\$ – icottam91 Feb 12 '14 at 15:57
  • \$\begingroup\$ @kleinfreund There's not enough content here to say whether or not the use of the headline element is appropriate. \$\endgroup\$ – cimmanon Feb 12 '14 at 15:58
  • 1
    \$\begingroup\$ @user1668270 I've rolled back your edit, because it would invalidate my answer. If you make some changes based on a review, you can update your question with new stuff, but only if you keep the old code as well. \$\endgroup\$ – kleinfreund Feb 12 '14 at 15:59
  • \$\begingroup\$ @cimmanon I didn't say it's necessarily inappropriate in his case, but when he is using it for demonstration purposes (e.g. illustrating the effect of text-shadow on an element with a higher font-size). \$\endgroup\$ – kleinfreund Feb 12 '14 at 16:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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