Could I please get opinion on my HTML and CSS coding

Discussion in 'CSS' started by buckmajor, Dec 16, 2007.

  1. #1
    This was a topic from (ChaosFoo) who posted an opinion on his CSS coding and interest me into seeking feedback for my own HTML and CSS code.

    Here is the link to my site:
    www.css-example.digitalprodusa.net

    I would like opinions on:

    1-Code simplicity, and structure. Let me know if I am doing good, and what can be done better.
    2-Page size. I want to make the page size as lean as possible.
    3-Better CSS practices, including compatibility with multiple browsers.
    4-Anything else that will make this site an easier venture.
    5-Validation error and how to fix them

    Many thanks in advance

    CHEERS :)
     
    buckmajor, Dec 16, 2007 IP
  2. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,999
    Best Answers:
    253
    Trophy Points:
    515
    #2
    Ok, here we go.

    Tranny doctype - this is a newly coded site, you should be using strict if for no other reason than to stop yourself from using deprecated tags.

    no language declared in the HTML tag. I'd add lang="en" xml:lang="en"

    'stylesheet.css' is pretty vague - and you have no media types. I always suggest naming your stylesheet by the media type it targets, and using media types. screen.css for "screen,projection", print.css for 'print', etc, etc.

    Pointless commments like <!-- Start Body Container--> - oh, the <body><div id="container"> so definately needs explanation, right. Meanwhile the willy-nilly formatting and lack of marking where they CLOSE makes it a bit confusing to read.

    &nbsp; - if you have to use that, you probably are doing something the wrong way.

    empty h1 - DEFINATELY doing something wrong. I would assume that H1 is the header image, as such I would REALLY recommend some form of image replacement like Glider-levin.

    Flash menu for no good reason. Bloated, fails at accessability, fails at giving search engines a means to navigate the content of the site, fail fail fail fail fail. This is a simple flat menu there's no need for that bullshit. The only thing worse than flash for presentational elements is flash for navigation. (and being that it doesn't even have dropdowns you don't even need to worry about hovers in IE6/earlier) Rip that rubbish out.

    presentational names - left and right should not be in your classnames as a good HTML layout should be able to be reskinned to any order - if at some point in the future you decide to reverse those columns, those names aren't going to make a lot of sense.

    Extra wrapper - this is fixed width, #content_body doesn't actually DO anything.

    H3's in #content_home_right - those are NOT subsections of the H2 above it, if anything they are kin so they should ALSO be H2's.

    use of double breaks - that's what paragraphs and margins are for... while you've got paragraphs around elements that are not paragraphs. Is "BREAKTHROUGH COMMUNITY" a paragraph? No, it is not. Is a single image a paragraph? No, it is not - so why are they marked up as paragraphs?

    Anchors showing the URL instead of meaningful text - the text above those should be the anchors.

    #footer - that menu should be a list... 'signature' should probably be INSIDE #footer without a div... likewise there is NO reason for that 'footer' class to be on each and every link when you've got a perfectly good ID wrapping the whole thing.

    CSS - well first off you have the univeral reset, THEN you have the not-so universal. If you have the first, you don't need the second. You are resetting borders when usually the only thing you 'need' to reset border on is image, and the rest of those properties you are declaring aren't even needed. I see multiple properties that could be condensed to single lines, etc, etc. By the time the CSS is added for a REAL menu, you would still have a smaller CSS file after condensing properties. You also have a LOT of redundant CSS, particularly those anchors which are just BAD.

    a.copyright:link, a.copyright:visited {
    	font-family: Verdana, Arial, Helvetica, sans-serif;
    	font-size: 10px;
    	color: #3399CC;
    	text-decoration: none;
    }
    
    a.copyright:active {
    	font-family: Verdana, Arial, Helvetica, sans-serif;
    	font-size: 10px;
    	color: #3399CC;
    	text-decoration: none;
    }
    
    a.copyright:hover {
    	color:#3399CC;
    	font-family: verdana, arial, sans-serif;
    	font-size: 10px;
    	font-weight: normal;
    	text-decoration: underline;
    }
    Code (markup):
    Uhm... no. :active should probably be the same as :hover so keyboard navigators have something to look at, and it's identical to the one before it so it should be combined there - AND you are declaring a bunch of values multiple times that could be inherited from the parent class.

    functionally identical:
    a.copyright {
    	font:normal 10px/14px verdana,arial,helvetica,sans-serif;
    	color:#39C;
    	text-decoration: none;
    }
    
    a.copyright:active,
    a.copyright:hover {
    	text-decoration: underline;
    }
    Code (markup):
    not that I'd be using a class on .copyright, as it would probably be #footer a in my version.

    Other observations:

    "Welcome To" - trite, just say where they are.

    Font sizes - you're default font sizes are too small and do not adjust to the system metric. Large font/120dpi users are automatically diving for the zoom the moment they arrive on your site. Fixed px font sizes on content is a total /FAIL/

    spacing between elements - that sidebar with the 'projects' is just FUGLY.

    Contrast - that dark blue text on the dark blue background is nigh illegible, major eye strain at best. A good rule of thumb is the '50% luminance rule' - which can be taken from the original IBM VGA specification - convert the foreground and background colors to the following numbers which represent screen luminance:

    L=0.3*red + 0.59*green + 0.11*blue

    Which represents the human sensitivity to light. Since we're using 8 bit, 0..255, you want the two colors to be at least 50% apart, aka 128. In your case you're darkest background point has a luminance of 12.96, while you're headers have a luminance of 92.31, a difference of only 79.35, way too low a contrast making the text either 'invisible' or at bare minimum causing major eye strain. It is also quite possible it's invisible to the color blind something the above luminance formula also helps avoid. (it is nigh impossible to have a 50% difference in luminance the color blind can't read). In the same hue you'd need to go all the way up to #4D9BE8 MINIMUM.

    Quickly, this is how I'd probably have that same page written:
    <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" 
    "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
    <html xmlns="http://www.w3.org/1999/xhtml" lang="en" xml:lang="en"><head>
    
    <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1" />
    
    <link type="text/css" rel="stylesheet" href="screen.css" media="screen,projection" />
    <link type="text/css" rel="stylesheet" href="print.css" media="print" />
    
    <title>Digital ProdUSA</title>
    
    </head><body>
    
    <div id="container">
    	<h1>
    		<strong>Digital ProdUSA</strong>
    		A Creative Mind with Integrity
    		<span></span>
    	</h1>
    	
    	<ul id="mainMenu">
    		<li><a href="#">Home<span></span></a></li>
    		<li><a href="#">Core Team<span></span></a></li>
    		<li><a href="#">Services<span></span></a></li>
    		<li><a href="#">Portfolio<span></span></a></li>
    		<li><a href="#">Contact<span></span></a></li>
    	</ul>
    	
    	<div id="content">
    		<h2>Welcome to the home of Digital Produsa</h2>
    		<p>
    			You can browse some of our recent work by clicking on our 
    			<a href="portfolio.html">portfolio</a>.
    		</p><p>
    			We are a Brisbane based web development firm offering our clients
    			a full range of multimedia services including design, signage, web
    			development and web hosting.
    		</p><p>
    			We have built strong relationships with our clients by working closely
    			with them to understand their needs. We specialise in building 
    			customised web sites that successfully meet these needs.
    		</p><p>
    			Digital Produsa brings a new dimension to web site design, combining 
    			creativity and functionality in a practical and professional environment,
    			focussed on providing effective solutions for our clients.
    		</p>
    	<!-- #content --></div>
    
    	<div id="sideBar">
    
    		<div class="sideBox">
    			<h2>Latest Project</h2>
    			<a href="http://www.bcintl.org/">
    				<img src="img/home_thumbnails_Lproject.jpg" 
    					alt="Breakthrough Community" 
    					width="226" height="89" 
    					class="sideImage" 
    				/>
    				BREAKTHROUGH COMMUNITY
    			</a>
    		</div>
    		
    		<div class="sideBox">
    			<h2>Recent Project</h2>
    			<a href="http://www.nmg.com.au">
    				<img src="img/home_thumbnails_Rprojects.jpg" 
    					alt="Norris Motor Groups" 
    					width="226" 
    					height="89"
    					class="sideImage"
    				/>
    				NORRIS MOTOR GROUPS
    			</a>
    		</div>
    
    	<!-- #sideBar --></div>
    
    	<div id="footer">
    		<ul>
    			<li><a href="index.html">Home</a></li>
    			<li><a href="core.html">Core Team</a></li>
    			<li><a href="services.html">Services</a></li>
    			<li><a href="portfolio.html">Portfolio</a></li>
    			<li><a href="contact.html">Contact</a></li>
    		</ul>
    		&copy; 2007 <a href="mailto:webmaster@digitalprodusa.net?subject=Enquiry">
    		Digital Produsa</a>
    	<!-- #footer --></div>
    
    <!-- #container --></div>
    
    </body></html>
    Code (markup):
    The empty spans are for glider-levin style image replacements.

    If I have time later I'll work up the screen.css that would drive that.
     
    deathshadow, Dec 16, 2007 IP
  3. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,999
    Best Answers:
    253
    Trophy Points:
    515
    #3
    ... and here it is working 'my way':

    http://battletech.hopto.org/for_others/buckmajor/template.html

    as always the directory:
    http://battletech.hopto.org/for_others/buckmajor

    is unlocked for ease of access to the gooey bits. Most of the images were remastered to make it work. Validates XHTML 1.0 Strict, tested working 100% in IE 5.5, 6 & 7, Firefox, Opera and Safari. CSS would validate except for the SINGLE zoom:1 for haslayout (BFD), degrades images off, degrades CSS off. It's about 1.1k less CSS too, and some image optimizations cut the total page size by more than HALF.
     
    deathshadow, Dec 16, 2007 IP
  4. buckmajor

    buckmajor Active Member

    Messages:
    574
    Likes Received:
    3
    Best Answers:
    0
    Trophy Points:
    58
    #4
    I love you Deathshadow ;) :)

    Thank you thank you, I never expected this much breakdown but thanks heaps for the feeedback and retification in my CSS coding. It makes sense, so I just need to find out how to make the footer links work in other browsers ;).

    All is good though, I really like the way you remove unnecessary coding to keep it consistent and tidy using relevant comments. You know your stuff D and really admire how people get to that level of competence.

    I am almost done with the updated site so I will post it up very shortly.

    CHEERS :)
     
    buckmajor, Dec 16, 2007 IP
  5. buckmajor

    buckmajor Active Member

    Messages:
    574
    Likes Received:
    3
    Best Answers:
    0
    Trophy Points:
    58
    #5
    Hey D,

    How do I change the cursor on mouseup over the main menu to a finger?

    It only works on mousedown. I decided not to use the flash menu and stick with the CSS navigation.
     
    buckmajor, Dec 16, 2007 IP
  6. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,999
    Best Answers:
    253
    Trophy Points:
    515
    #6
    Huh? That makes no sense - I'm not even sure what you are asking. Do you mean on mouse over? Why would it be a finger if it's not over...

    My process for coding a layout is different from most people - I write the HTML FIRST using semantic markup, dividing areas up into sections as appropriate with ZERO regard for how the site is going to actually look... Thats right, my first pass on the HTML is about getting the content and controls on the page with no care for what the layout is going to look like. Only once I have a logical document structure do I add the simple stuff like empty spans for glider-levin, sliding doors or eight corners, but I avoid adding extra tags beyond the base semantic markup as much as possible, instead trying to use the CSS to bend it to my will. Above all, is the separation of presentation from content as much as possible. Admittedly there are cases where thanks to CSS2 being barely implemented on the majority of browsers a decade after it's creation you are forced to toss in a span or div here or there to hang CSS on, but really even in those cases if you take the time to make them 'clean' and 'minimal' you won't have any problems.

    "Designers" often lose sight of the fact that pages are about the content and the navigation to reach the content, not the graphical layout you hang on it. Working with the content first, THEN applying styles to it gives cleaner code, cleaner layouts and in general better accessability. They forget that not everyone browses with images on, people will often disable CSS, search engines ignore attributes AND CSS, etc, etc.

    Likewise coders often forget that the less code you use, the less there is to break... The point of markup is to separate sections meaningfully, not start slapping on tags, classes and id's on everything in sight, tossing javascripts on everything just because you can, etc, etc.

    ... and NOBODY seems to be paying any attention to filesizes anymore - you hear the excuse "But everyone has broadband now"... A> tell that to states like Utah or the Dakota's where 90% of their population can't even get dialup faster than 14.4k and are unlikely to over the next decade, and B> sure, client bandwidth has gone up, but server bandwidth has not. Go ahead, overload your server choking out a 10mbps connect for a site that could be coded to serve the same number of people on a tenth of that..
     
    deathshadow, Dec 16, 2007 IP
  7. buckmajor

    buckmajor Active Member

    Messages:
    574
    Likes Received:
    3
    Best Answers:
    0
    Trophy Points:
    58
    #7
    Oopps sorry D
    I meant to say "On mouse over at the main navigation, it doesnt change the cursor to a finger (to show the user that its a button) but a paragraph icon. Only when you click on the button then it shows the finger icon.

    You have true meaning to every that was stated but this one sums it up the most. I have forgotten the reason why target audience and world wide viewers are so important these days. I never thought there will be many people still on dialup and probably most rural areas are limited to broadband connection too. I like how you point out the way you can optimize your site and code it to serve the number of people who are limited to a faster connection.

    Personally 'you can ignore this if you want' but this is like a faminine principle where people are starving but are lacking in resources.

    Thanks D I will post up my lastest update with the CSS code

    CHEERS :)
     
    buckmajor, Dec 17, 2007 IP
  8. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,999
    Best Answers:
    253
    Trophy Points:
    515
    #8
    What browser? Those are anchors and we don't touch cursors in the CSS, so they should work normally in all of them (and do so here)

    Oh, wait, I see it - IE. Huh, odd. Usually doesn't do that.

    Ok, add:
    cursor: pointer;
    cursor: hand;

    to the "#mainMenu span" CSS, and that should clear up that one issue.
     
    deathshadow, Dec 18, 2007 IP
  9. MichaelS

    MichaelS Guest

    Messages:
    595
    Likes Received:
    40
    Best Answers:
    0
    Trophy Points:
    0
    #9
    For the hover mouse thing, it's because it's flash.
     
    MichaelS, Dec 18, 2007 IP
  10. buckmajor

    buckmajor Active Member

    Messages:
    574
    Likes Received:
    3
    Best Answers:
    0
    Trophy Points:
    58
    #10
    Hey D

    Yea it works for the main navigation in all browsers thanks heaps D :).

    My only problem I have now is the footer links which I've been trying for a couple of hours now. Works perfectly in IE but not in other browsers. I tried that (cursor: hand) in the #footer, .first and even in the .a anchor and still no luck.

    What do I do?
     
    buckmajor, Dec 18, 2007 IP
  11. buckmajor

    buckmajor Active Member

    Messages:
    574
    Likes Received:
    3
    Best Answers:
    0
    Trophy Points:
    58
    #11
    Hey MichaelS thanks for your concern. I got main navigation going now and here is an update to my Test Site
    http://www.css-example.digitalprodusa.net/index.html

     
    buckmajor, Dec 18, 2007 IP
  12. buckmajor

    buckmajor Active Member

    Messages:
    574
    Likes Received:
    3
    Best Answers:
    0
    Trophy Points:
    58
    #12
    buckmajor, Dec 20, 2007 IP