HiveBrain v1.2.0
Get Started
← Back to all entries
patternhtmlModerate

HTML inline CSS

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
inlinehtmlcss

Problem

This HTML code seems to work: the page looks as I intended it. I have checked it multiple times, but it's difficult to tell how to improve. I would like somebody to review and point out any mistakes or bad practices, or other recommendations to make it better.

This is to be output as an email:



`

Commentary Notification



















New Commentary Available


A new commentary has been posted to the Commentary Blog.







Commentary:
Current Item


Posted By:
Created By


Category:
Cat


 To access this Commentary post please click on the following link:


 


Commentary Blog→




 









This email and any files transmitted with it are confidential and intended solely for the use of the individual/s or entity to whom they are addressed. Please notify Email adress immediately if you have received this e-mail by mistake and delete this e-mail from your system. If you are not the intended recipient you are notified that disclosing, copying, distributing or taking any action in reliance on the contents of t

Solution

Broken HTML

Browsers are forgiving. I see a fairly nice looking page,
but the truth is, this HTML is seriously broken.


    
        
            
                

                
            


You have table > tr > td > tr, which is illegal: you cannot have a tr element inside a td element.

Maybe you meant this structure instead:


    
        
            
                
                    <img alt="" src="" width="600" height="110"


Simple syntax errors

There are also several simple syntax errors:

  • The first line should be `, and NOT



  • You have this:




    New Commentary Available


The text-shadow color
FFFDF2 should have been #FFFDF2.

Separate the CSS from HTML

The whole point of CSS is to separate the fine-tuned styling from the HTML. You really shouldn't inline so many
style attributes everywhere. Move the content of all those inlined style declarations to a separate .css` file. You will be able to reduce a lot of the duplication, and you will get a much more maintainable solution: you will be able to make style changes in one place and they will apply in all the places where that style should be used.

UPDATE: the original post did not mention that this HTML is to be used in an email, that bit was added later. And in that case, inline CSS can be acceptable:

http://css-tricks.com/using-css-in-html-emails-the-real-story/

(Thank you @pbk1303, excellent point & link!)

The ultimate test

There is a website that checks for correct HTML formatting and bad practices:

http://validator.w3.org/check

You can paste the content of your HTML file in the Validate by Direct Input tab and it will tell you what else is wrong with your HTML. Ideally you should only post on Code Review after you have corrected all the common violations captured by this tool.

Inlining CSS with npm juice

@Ben left an excellent remark in comments:


Might also be worth mentioning that you can install juice with npm, to be able to inline the CSS from an external file; that way you keep all the benefits of separate CSS whilst still being able to generate an email client friendly version. npmjs.org/package/juice

Code Snippets

<table cellpadding="0" cellspacing="0" width="600" bgcolor="#FFFDF2">
    <tr>
        <td style="padding: 5px 0px 5px 0px;" bgcolor="#FFFDF2">
            <tr>
                <td align="center"><a href=""><img alt="" src="" width="600" height="110"  style="display: block; font-family: Helvetica, Arial; color: #000000; font-size: 14px; padding: 0px 5px 0px 5px;" border="0"></a>

                </td>
            </tr>
<table cellpadding="0" cellspacing="0" width="600" bgcolor="#FFFDF2">
    <tr>
        <td style="padding: 5px 0px 5px 0px;" bgcolor="#FFFDF2">
            <table>
                <tr>
                    <td align="center"><a href=""><img alt="" src="" width="600" height="110"
<td bgcolor="#FFFDF2" colspan="2"
    style="padding: 0px 0px 0px 0px; font-size: 32px;
           font-family: Helvetica, Arial, sans-serif;
           font-weight: bold; color: #0A3775;
           text-shadow: 2px 2px 2px FFFDF2; text-align: center;">
    New Commentary Available
</td>

Context

StackExchange Code Review Q#60188, answer score: 12

Revisions (0)

No revisions yet.