BadMango Posted April 11, 2013 Report Share Posted April 11, 2013 I'm practising my html and css. So I searched around for a cool looking PSD to slice up and put together. Here's what I came out with. programs I used were: Ubuntu 12.04 Gimp Bluefish and some button generator Any tips or any constructive criticism is greatly appreciated in case you didn't want to download my .zip file here are links to my code and the original My CSS code My HTML what the original looks like What do you think? POLO360.zip Quote Link to comment Share on other sites More sharing options...
Andrea Posted April 11, 2013 Report Share Posted April 11, 2013 I uploaded your files as they are (minus path adjustments that have nothing to do with your code, only how/where I save these forum pages) and I get: http://aandbwebdesign.com/KSforum/polo.html Overall, it looks very good for a pretty complex page. Maybe some adjustments around the top navigation. Next, I ran the page through the validator - also good, only 9 errors. I honestly have no idea what the problem with the closing head tag is supposed to be, it looks good to me (but then, I don't have all the answers). The next error about the duplicate id tag is simply because IDs are meant to be unique per page, meaning you can use each only once. Use a class if you need something more than once. The line 46 error, you have a quote misplaced. alt=css button generator"lady jumping" should be alt="css button generator lady jumping" -- or probably better: alt="lady jumping" The alt tag is so you can describe the image for those who cannot see it. The rest looks pretty good. Some minor things are for example your footer - you should not need the footer div and the footer_container div inside that. You can apply any styling directly to the footer div itself. In your CSS you have some redundancies - example, the reset css applies the zero padding, margin, and border to a whole list of elements. There is no need to repeat this zero setting on the other sheet (or ever). Also, there really is no need for a separate reset CSS, you can paste all that stuff to the top of your style css. Nothing wrong with multiple css documents, but it might make more sense if you have a very complex navigation to keep it separate - for example. Anyway, just a couple pointers, hope it helps - let me know if you have an questions. Quote Link to comment Share on other sites More sharing options...
falkencreative Posted April 11, 2013 Report Share Posted April 11, 2013 I honestly have no idea what the problem with the closing head tag is supposed to be, it looks good to me (but then, I don't have all the answers). I think that is related to a missing <title></title> element in the head section. Quote Link to comment Share on other sites More sharing options...
Andrea Posted April 11, 2013 Report Share Posted April 11, 2013 I think that is related to a missing <title></title> element in the head section. That is absolutely it - I added a title tag, and the error is gone. Quote Link to comment Share on other sites More sharing options...
grabenair Posted April 11, 2013 Report Share Posted April 11, 2013 (edited) Just a little more for your css zeroing out you can simply put * {margin:0; padding:0;} The * is what is called a wild card and effects all of the html Dom elements. If you not understand what the Dom is that is ok you will when you start writing jQuery. For now just know that it effects everything on the page. Also html5 elements are block by default, not that it hurts to declare then anyway. But less code is always better. You should add this to the head just before the closing head tag, anyway that is where I put it but as long as it is in the head. The code is for IE as you can tell because older IE does not like html5. <!--[if lt IE 9]> <script src="http://html5shiv.googlecode.com/svn/trunk/html5.js"></script> <![endif]--> Also good job on the id's and classes, exempt for the one as Andrea said. So many people want to use only classes all of the time where id's are better. I would give you 93% an A Edited April 12, 2013 by grabenair Quote Link to comment Share on other sites More sharing options...
BadMango Posted April 12, 2013 Author Report Share Posted April 12, 2013 I was in such a hurry to post this I completely forgot to validate I would have cleared most of that up. Sorry about that. On line 46, you are correct, it was just supposed to be "lady jumping". that button generator must have been some kind of copy and past accident. The rest looks pretty good. Some minor things are for example your footer - you should not need the footer div and the footer_container div inside that. You can apply any styling directly to the footer div itself. About the footer. I put the footer div for the background image to go side to side 100% of the way, but used the inside container to keep everything inside the 940 width. How would I go about cutting this down to just one div? I could have the id=footer and class=container on one div, but how would I make that image go from side to side? Now I look at it again I really didnt need to put id=footer_container. Thank you all for being so helpful! Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.