SitePoint Sponsor

User Tag List

Results 1 to 16 of 16
  1. #1
    The I's for intelligent silver trophy iTechno's Avatar
    Join Date
    Jan 2006
    Location
    Yorkshire, UK
    Posts
    1,796
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Rate my coding...

    I'm always looking to excel and was wondering what you guys thought of the coding on this:

    www.freshcoders.com/Southern%20Arizona/

    Thanks .

  2. #2
    SitePoint Member talktechno's Avatar
    Join Date
    Apr 2006
    Posts
    0
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by iTechno View Post
    I'm always looking to excel and was wondering what you guys thought of the coding on this:

    www.freshcoders.com/Southern%20Arizona/

    Thanks .
    you might want to view it in firefox

  3. #3
    SitePoint Wizard bronze trophy C. Ankerstjerne's Avatar
    Join Date
    Jan 2004
    Location
    The Kingdom of Denmark
    Posts
    2,702
    Mentioned
    7 Post(s)
    Tagged
    0 Thread(s)
    Looks OK, but you have some semantically incorrect code, and some redundant code.

    Your paragraphs should be in P-tags.

    Your headers should be H2 rather than H3, when you don't have any H2 above them (while you do have an H2, it isn't a header, but a slogan - you also have a SPAN inside the H2, which does nothing.
    Christian Ankerstjerne
    <p<strong<abbr/HTML/ 4 teh win</>
    <>In Soviet Russia, website codes you!

  4. #4
    Now available in Orange Tijmen's Avatar
    Join Date
    Jul 2004
    Location
    The Netherlands
    Posts
    1,469
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Things like this should be done different.

    Code:
    <div id="header">
    <a href="#"><img src="images/search.gif" alt="Search" style="float:right;" /></a>
    </div>
    You should write in your css file something like this. This prevents that you have to use inline code like you have done now. The same goes for the <p> in the #leftrc, and a couple of other things.

    Code:
    #header img {float:right;}
    Travel Photos on Flickr - Twitter

    “Never give up. Never surrender”

  5. #5
    I meant that to happen silver trophybronze trophy Raffles's Avatar
    Join Date
    Sep 2005
    Location
    Tanzania
    Posts
    4,662
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Looks like you've gone for a fixed 1024px-wide layout. It gives me a horizontal scrollbar since I'm at 1024x768, as are most of the browsing population I believe. Also, the "Painless Treatment" text has almost completely disappeared into the white box underneath with the doctor's picture.

    I don't see why you should use images in the navigation bar or in the top right of the main image (where there is a typo BTW) - it would be much better to just use text. Then you could also get rid of those unnecessary spans in your navigation and also slim the general page down by using fewer images. The same goes for the big blue "Click Here" link - why not just text? Not to mention that any links that say "click here" are bad practice.

    It would be nice if the site map, help and search links at the top used different colours on hover, as well as the links everywhere else. Using text for these would of course make this less of a hassle to implement.

    I'm a big fan of not using extra markup to clear elements. My favourite method, where it's practical, is to give the container of the floated elements overflow:auto and position:relative. Then there's also the ::after way which can be sufficient because IE often seems to self-clear floats for no apparent reason.

    Still, other than that I think it's a good-looking page and the source is quite nice too. It's refreshing not to see any <script> tags, though you really should get rid of the inline styles, they're not cool!

  6. #6
    Now available in Orange Tijmen's Avatar
    Join Date
    Jul 2004
    Location
    The Netherlands
    Posts
    1,469
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Another things is that whenever you decide to use images, its a good idea to specify the width and height of the image. That way the browser will know which size to expect when its going to start to load an image, and it doesn't have to figure it out itself after downloading the whole image.
    Travel Photos on Flickr - Twitter

    “Never give up. Never surrender”

  7. #7
    The I's for intelligent silver trophy iTechno's Avatar
    Join Date
    Jan 2006
    Location
    Yorkshire, UK
    Posts
    1,796
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Raffles View Post
    Looks like you've gone for a fixed 1024px-wide layout. It gives me a horizontal scrollbar since I'm at 1024x768, as are most of the browsing population I believe. Also, the "Painless Treatment" text has almost completely disappeared into the white box underneath with the doctor's picture.

    I don't see why you should use images in the navigation bar or in the top right of the main image (where there is a typo BTW) - it would be much better to just use text. Then you could also get rid of those unnecessary spans in your navigation and also slim the general page down by using fewer images. The same goes for the big blue "Click Here" link - why not just text? Not to mention that any links that say "click here" are bad practice.

    It would be nice if the site map, help and search links at the top used different colours on hover, as well as the links everywhere else. Using text for these would of course make this less of a hassle to implement.
    The reason is because the text is customized in a way that I can't replicate in a cross-browser friendly way. Also, I was under the opinion that writting <span></span> was good for SEO?

    I've implemented all the suggestions so thanks guys .

  8. #8
    Now available in Orange Tijmen's Avatar
    Join Date
    Jul 2004
    Location
    The Netherlands
    Posts
    1,469
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by iTechno View Post
    Also, I was under the opinion that writting <span></span> was good for SEO?
    I have never heard of that, and why would it be good for SEO? Your site should rank high based on your content, and not if you use the <span> tag in your code. It wouldn't make a lot of sense to me if it would matter, but maybe someone else has a good explanation for it.
    Travel Photos on Flickr - Twitter

    “Never give up. Never surrender”

  9. #9
    SitePoint Wizard silver trophybronze trophy Nadia P's Avatar
    Join Date
    Oct 2004
    Location
    NSW Australia
    Posts
    3,564
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    [quote=Tijmen;3349674]Your site should rank high based on your content, and not if you use the <span> tag in your code. /quote].

    I agree, I don't believe there's any reason to use the <span> tag either. The title attribute has been used and that should be enough... and anyway, the spiders will automatically follow the links in the <li>s. Just make sure there are relevant keywords scattered throughout the text copy and you make good use of the H1 tags.

    Nadia

  10. #10
    SitePoint Author silver trophybronze trophy

    Join Date
    Nov 2004
    Location
    Ankh-Morpork
    Posts
    12,158
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    FWIW, you've got an ALT text that says, 'Clich Here'. Not only bad spelling, but also bad for accessibility. Why should I click there? What good will it do me?

    The markup is not too good in the semantics department. You have DIVs with character data sloshing around in them without being marked up as paragraphs.

    You have contact information that is marked up as a paragraph, rather than using the appropriate ADDRESS element.

    There seem to be too many classes, and their names look more presentational than semantic.

    Inline CSS is rarely a good idea. Use a class instead (with a semantic name) and relegate the styling to an external style sheet.

    This code suffers from a bad case of spanitis. You don't have to wrap everything in a SPAN just to assign a class to it. You can set a class for practically any element type. including links.
    Birnam wood is come to Dunsinane

  11. #11
    I meant that to happen silver trophybronze trophy Raffles's Avatar
    Join Date
    Sep 2005
    Location
    Tanzania
    Posts
    4,662
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by iTechno View Post
    The reason is because the text is customized in a way that I can't replicate in a cross-browser friendly way. Also, I was under the opinion that writting <span></span> was good for SEO?
    I thought the spans were there to make image replacement in the header and navigation bar easier.

  12. #12
    SitePoint Guru
    Join Date
    Apr 2007
    Posts
    813
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I am not a big supporter of strict web XHTML standards. Anyway did anyone always enclose a text inside inline HTML elements such as p, h1-h6, span but not div.

  13. #13
    I meant that to happen silver trophybronze trophy Raffles's Avatar
    Join Date
    Sep 2005
    Location
    Tanzania
    Posts
    4,662
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Because div is supposed to be used to hold lots of different things, like p, h1, ul and whatnot. It's just a general container. Also, p and h1-h6 are NOT inline, they are block just like div and they are used to "just put text inside" because they are the most semantically sensible element to use.

    Still, your first statement clearly explains why this was all Greek to you and why it probably will never be otherwise.

  14. #14
    SitePoint Author silver trophybronze trophy

    Join Date
    Nov 2004
    Location
    Ankh-Morpork
    Posts
    12,158
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    What element type you enclose text in depends on what the text is. That is the whole idea with markup languages like HTML and XHTML.

    If it is a paragraph, use <p>.. A paragraph is one or more sentences that deal with a single thought or idea.

    If it is a top-level document heading, use <h1>. It it's a second-level heading use <h2>, an so on.

    A DIV may, syntactically, contain both block-level and inline-level children. But it shouldn't contain both at the same time. That doesn't make semantic sense.
    Birnam wood is come to Dunsinane

  15. #15
    SitePoint Enthusiast dmj1973's Avatar
    Join Date
    Feb 2007
    Location
    Somewhere between Geek and Total Geek
    Posts
    67
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Layout and design wise it looks nice. Can't comment on the coding as I am new to this myself.

    Having said that you do have a link that is entitled click here. This is not good practice.

    http://www.cs.tut.fi/~jkorpela/www/click.html

    Try using the "get back your freedom today." part of the paragraph as the link instead of a "click here". The same applys to the "Click here" for the "map of our location" text. The is also no Alt text for the link on that one.

    Talking of ALT text, there is none for the logo lilnk in the top left.

    The "About Dr.Surwit" alt text is there but in my opinion should be the same as the nav text.

    There is also no ALT text for any of the nav along the bottom

    Asthetically pleasing but i think it needs some tweaking.

  16. #16
    Robert Wellock silver trophybronze trophy xhtmlcoder's Avatar
    Join Date
    Apr 2002
    Location
    A Maze of Twisty Little Passages
    Posts
    6,316
    Mentioned
    60 Post(s)
    Tagged
    0 Thread(s)
    Having click here being the same colour as the subheadings wasn't too sensible and you'd almost certainly would get a lot smaller image filesize using PNG-8 instead of *.gif. Like others have said using "Click Here" as an image is rather strange.


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •