SitePoint Sponsor

User Tag List

Results 1 to 17 of 17
  1. #1
    SitePoint Zealot
    Join Date
    Apr 2003
    Location
    New Zealand
    Posts
    168
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Code Review Request

    Over the last couple of days, I have been working on a proof of concept (www.webme.co.nz/poc) for some site components produced using CSS.

    As I have pretty much stretched beyond my knowledge and experience on this, I'd appreciate some people more experienced at CSS having a look at what I have done and offering advice on how it could be improved.

    Things in particular that I am interested in are:
    1. Ways in which the CSS could be better optimized for greater efficiency.
    2. Any bugs in specific browsers and how they may be fixed (I have only tested in Chrome 5, FF3.5, IE7) however, as it will be used in an administrative application for a private company, older or more obscure browsers are unlikely to be used. IE6 may be used though.
    3. Anything I have overlooked that may cause issues for some users (I have not spent much time considering accessibility issues, so that is one area I would be interested in receiving advice on)
    4. How to line up the left/right images with the numbers in the pagination.
    5. How to vertically align the form labels so they are level with the middle of the fields in IE7
    6. Any other comments (please be gentle ;o) )

    The Components
    1. Fixed header with flexible main content section. I based this on the solution described in Test your CSS skills Quiz 1.

    2. Multi-level drop menu (see "Tools" for third level). Based on a solution I picked up some time ago. I'm not sure that display:none is the best way to hide the drop-downs, so would like to hear comments on that.

    3. Scrolling table with fixed headers. Another one I picked up from the Test your CSS skills series, this time Quiz 22

    4. Tabbed menu. This is simply a formatted unordered list, but I'd be interested in hearing a method of fixing the 1px gap where it meets the main panel.

    Thanks for your interest.

  2. #2
    billycundiff{float:left;} silver trophybronze trophy RyanReese's Avatar
    Join Date
    Oct 2008
    Location
    Whiteford, Maryland, United States
    Posts
    13,760
    Mentioned
    12 Post(s)
    Tagged
    0 Thread(s)
    Hi, while this mmay be a CSS related question. I'd ask any design/accessibilty/any other questions not related to cross browser issues in a different part of hte forum. I'll ask this to be moved. I'll answer some questions while I'm still typing.

    Dropdowns should be hidden via a huge left margin (margin-left:-999em) and hten on hover bring it back to 0 (margin-left:0)
    Coordinates shouolld be used as always (you don't seem like a beginner)
    Always looking for web design/development work.
    http://www.CodeFundamentals.com

  3. #3
    SitePoint Zealot
    Join Date
    Apr 2003
    Location
    New Zealand
    Posts
    168
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Thanks for responding, Ryan
    Quote Originally Posted by RyanReese View Post
    Hi, while this mmay be a CSS related question. I'd ask any design/accessibilty/any other questions not related to cross browser issues in a different part of hte forum. I'll ask this to be moved. I'll answer some questions while I'm still typing.
    I apologize for mixing question types. My main interest is in producing good quality, efficient CSS, which is why I posted it here.

    I'm not quite sure what you mean by "any design/accessibilty/any other questions not related to cross browser issues". Is the CSS forum only for posts related to cross-browser issues?

    Quote Originally Posted by RyanReese View Post
    Dropdowns should be hidden via a huge left margin (margin-left:-999em) and hten on hover bring it back to 0 (margin-left:0)
    Coordinates shouolld be used as always
    Yes, I believe I read that elsewhere. Thanks for the confirmation.

    Quote Originally Posted by RyanReese View Post
    (you don't seem like a beginner)
    Thank you for the compliment. I am not a beginner, but also not an expert by any means. However, I am reasonably adept at adapting the work of others for my needs. I guess you could say I am experienced enough to know where I lack expertise which is why I appreciate peer review of my work.

  4. #4
    Ripe Tomatos silver trophybronze trophy Rayzur's Avatar
    Join Date
    Jun 2007
    Location
    Texas
    Posts
    4,174
    Mentioned
    4 Post(s)
    Tagged
    0 Thread(s)
    Hi,
    You have posted in the right forum as your questions are CSS related.

    4. How to line up the left/right images with the numbers in the pagination.
    As for vertical alignment of the arrow images the vertical-align needs to be set on the inline elements and not the p tag itself. Vertical-align only works on inline elements (including inline-block) and tables.

    Your images have about 2px worth of whitespace at the top of them which throws the true alignment off slightly.

    Just set vertical-align:midlle; on the image and remove vertical-align:bottom; from the anchor.

    Code:
    #tabSection p.pagination {
        border-top: 1px solid #91a7b4;
        clear: both;
        padding-top: 6px;
        text-align: center;
        /*vertical-align: bottom;*/
    }
    #tabSection p.pagination a {
        text-decoration: none;
        /*vertical-align: bottom;*/
    }
    #tabSection p.pagination img {
        vertical-align:middle;
        background:cyan;/*test BG color*/
        padding-bottom:2px;/*compensate image offset*/
    }

  5. #5
    SitePoint Zealot
    Join Date
    Apr 2003
    Location
    New Zealand
    Posts
    168
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Rayzur View Post
    Hi,
    You have posted in the right forum as your questions are CSS related.

    As for vertical alignment of the arrow images the vertical-align needs to be set on the inline elements and not the p tag itself. Vertical-align only works on inline elements (including inline-block) and tables.
    Aha! That's where I was thrown off. I was working on the supposition that vertical-align operated on the contents of the element (like text-align) whereas it operates on the element itself. No wonder I have had so much trouble getting it to work

    Quote Originally Posted by Rayzur View Post
    Your images have about 2px worth of whitespace at the top of them which throws the true alignment off slightly.

    Just set vertical-align:midlle; on the image and remove vertical-align:bottom; from the anchor.
    Perfect! Thanks for your help.

  6. #6
    SitePoint Zealot
    Join Date
    Apr 2003
    Location
    New Zealand
    Posts
    168
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Rayzur View Post
    As for vertical alignment of the arrow images the vertical-align needs to be set on the inline elements and not the p tag itself. Vertical-align only works on inline elements (including inline-block) and tables.
    This guided me to a solution for aligning the form labels as well.

    Thanks again!

  7. #7
    Ripe Tomatos silver trophybronze trophy Rayzur's Avatar
    Join Date
    Jun 2007
    Location
    Texas
    Posts
    4,174
    Mentioned
    4 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by KiwiJohn View Post
    This guided me to a solution for aligning the form labels as well.

    Thanks again!
    I expected that you might find the solution for the labels after I explained how v-a works with inline elements.

  8. #8
    SitePoint Zealot
    Join Date
    Apr 2003
    Location
    New Zealand
    Posts
    168
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by RyanReese View Post
    Dropdowns should be hidden via a huge left margin (margin-left:-999em) and hten on hover bring it back to 0 (margin-left:0)
    Coordinates shouolld be used as always
    Thanks again for the tip. I've managed to put this into effect.

  9. #9
    Ripe Tomatos silver trophybronze trophy Rayzur's Avatar
    Join Date
    Jun 2007
    Location
    Texas
    Posts
    4,174
    Mentioned
    4 Post(s)
    Tagged
    0 Thread(s)
    4. Tabbed menu. This is simply a formatted unordered list, but I'd be interested in hearing a method of fixing the 1px gap where it meets the main panel.
    You should be able to fix that gap with bottom padding and a negative margin to match. I was able to eliminate the bottom border by using 2px, you might want to set that active class on another list item and see how it works there.

    Code:
    #tabSection ul.tabs li.active {
        background-color: #f0ecde;
        background-image: url("tab-left-active.gif");
        /*border-bottom: 1px solid #f0ecde;*/
        padding-bottom:2px;
        margin-bottom:-2px
    }

  10. #10
    SitePoint Zealot
    Join Date
    Apr 2003
    Location
    New Zealand
    Posts
    168
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Rayzur View Post
    You should be able to fix that gap with bottom padding and a negative margin to match. I was able to eliminate the bottom border by using 2px, you might want to set that active class on another list item and see how it works there.

    Code:
    #tabSection ul.tabs li.active {
        background-color: #f0ecde;
        background-image: url("tab-left-active.gif");
        /*border-bottom: 1px solid #f0ecde;*/
        padding-bottom:2px;
        margin-bottom:-2px
    }
    Fantastic! That works great.

    I'd tried using a negative margin on the .inner div which closed the gap, but also covered the bottom-border on the .tabs ul

    Thanks for taking the time to help me figure these things out. I really appreciate it.

    Your solution gave me some ideas about the gap at the other end too and I managed to solve that as well! Woohoo!

    The issue I has was that the bottom border on the .tabs ul ended 2px short of the right hand corner. Adding a 1px border each side of the ul fixed the alignment, but I didn't want borders on the ends. But that got me thinking - how else can I add those 2px? Adding 2px padding-left to the ul fixed the bottom-border issue, but left the first tab 2px to the right. So I gave the first tab a class of "first" and applied a -2px margin-left to it. Solved!

    It even works in IE7

  11. #11
    SitePoint Zealot
    Join Date
    Apr 2003
    Location
    New Zealand
    Posts
    168
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Oh, by the way, both our fixes work great no matter which tab is active.

  12. #12
    The CSS Clinic is open silver trophybronze trophy
    Paul O'B's Avatar
    Join Date
    Jan 2003
    Location
    Hampshire UK
    Posts
    40,549
    Mentioned
    183 Post(s)
    Tagged
    6 Thread(s)
    Quote Originally Posted by kiwiJohn
    I'm not quite sure what you mean by "any design/accessibilty/any other questions not related to cross browser issues". Is the CSS forum only for posts related to cross-browser issues?
    What Ryan meant was that we have a specific forum for Reviews and as you titled your thread "Code Review Request" then it could be perceived that you were looking for general reviews on how the sites works, looks and feels. Those types of questions must be asked in the review forum so that we don't have people posting "does my site look ok" type of questions and cluttering up the forums

    However, questions about bugs and fixes and css problems are fine here It's just the review type of questions that you shold avoid but any other fixes you need looked at then fire away

    Usually it's better to ask one or two questions at a time as a whole list frightens people away

  13. #13
    SitePoint Zealot
    Join Date
    Apr 2003
    Location
    New Zealand
    Posts
    168
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Paul O'B View Post
    What Ryan meant was that we have a specific forum for Reviews and as you titled your thread "Code Review Request" then it could be perceived that you were looking for general reviews on how the sites works, looks and feels. Those types of questions must be asked in the review forum so that we don't have people posting "does my site look ok" type of questions and cluttering up the forums

    However, questions about bugs and fixes and css problems are fine here It's just the review type of questions that you shold avoid but any other fixes you need looked at then fire away

    Usually it's better to ask one or two questions at a time as a whole list frightens people away
    Thanks for the clarification, Paul. I see your point. I guess I did throw quite a bit into the pot at once.

  14. #14
    SitePoint Zealot
    Join Date
    Apr 2003
    Location
    New Zealand
    Posts
    168
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Paul O'B View Post
    What Ryan meant was that we have a specific forum for Reviews and as you titled your thread "Code Review Request" then it could be perceived that you were looking for general reviews on how the sites works, looks and feels. Those types of questions must be asked in the review forum so that we don't have people posting "does my site look ok" type of questions and cluttering up the forums
    Hi Paul

    I just checked out the review forums you linked to and they all seem to be focused on design, layout, usability, etc. In this post, I was not after a critique of my design or content. I was after a review of the CSS I used to create it.

    I accept that I may have been unclear and it may have appeared that I was after a review of the design. I also accept that I put too many questions in one post. I apologize.

    My primary interests when I started this thread were:

    1. have a more experienced CSSer review the CSS and let me know if it could be improved and/or if there is anything that might break in a certain browser (most significantly IE6). Things like "____ will break in IE6 so you should add ___ (hack) to prevent that" or "If you change ____, you can reduce your CSS file by X lines" or, for a specific example, "To hide the drop menus, it is better to use negative margin than display:none."

    2. receive advice on the specific issues I had (i.e. vertical alignments, gaps in tab menu, etc)

    Thank you again to those who helped.

  15. #15
    The CSS Clinic is open silver trophybronze trophy
    Paul O'B's Avatar
    Join Date
    Jan 2003
    Location
    Hampshire UK
    Posts
    40,549
    Mentioned
    183 Post(s)
    Tagged
    6 Thread(s)
    Quote Originally Posted by KiwiJohn View Post


    My primary interests when I started this thread were:

    1. have a more experienced CSSer review the CSS and let me know if it could be improved and/or if there is anything that might break in a certain browser (most significantly IE6). Things like "____ will break in IE6 so you should add ___ (hack) to prevent that" or "If you change ____, you can reduce your CSS file by X lines" or, for a specific example, "To hide the drop menus, it is better to use negative margin than display:none."
    If you want to support IE6 then you need to provide additional rules for any of the following.

    input[type="submit"]

    ul.nav li:hover>a


    IE6 doesn't understand attribute selectors or the child selector so you would need to make alternative arrangements using a class or similar.

    This code won't work:

    Code:
    * html #content {
        background: #fff;
        height: 100%;
        margin:20px;
        overflow: scroll;
        padding-bottom: 20px;
        padding-top: 20px;
        position: relative;
        z-index: 100;
    }
    height:100% + 40px margin + 40px padding = much to big.

    You have the double margin float bug in IE6. Add display:inline to floats that have margins and are first, last or only floats in a row.

    Code:
    #tabSection .panel {
        background: #f0ecde;
        border-bottom: 1px solid #91a7b4;
        border-left: 1px solid #91a7b4;
        border-right: 1px solid #91a7b4;
        float: left;
        height: 300px; /*must be same as in #tabSection .panel .inner */
        margin: 0px 0 0 2%; /*must be same as in #tabSection ul.tabs */
        overflow: hidden;
        padding: 57px 0 30px 0; /*sets space above and below .inner div */
        position: relative;
        width: 95%; /*must be same as in #tabSection ul.tabs */
        display:inline;
    }
    I assume this is taken from one of my fixed header demos but you don't seem to be utilising it correctly (although I know you haven't finished yet).

    Code:
    * html body {height: 100%; margin: 0; padding: 180px 0 20px 0}
    The padding for IE6 is to make room for fixed header and footer elements so needs to match whatever your requirements are.

    If an element has negative margins in iE6 it will most likely need position:relative to show the negative portion.

    It is needed here along width display:inline for the double margin bug.

    Code:
    #tabSection ul.tabs li.first {
        margin-left: -2px;
        position:relative;
        display:inline;
    }
    You will find your tabs will jog in and out by 1px in ie6 due to rounding errors and the percentage margins used.

    The header needs to be placed absolutely at the top for ie6.
    Code:
    * html body {height: 100%; margin: 0; padding: 45px 0 0px 0}
    * html #header{width:100%;position:absolute;top:0;left:0}
    IE6 needs a lot more work but I've run out of time today



    [/code]

  16. #16
    SitePoint Zealot
    Join Date
    Apr 2003
    Location
    New Zealand
    Posts
    168
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Paul O'B View Post
    IE6 needs a lot more work but I've run out of time today
    Paul, thank you very much for taking the time you have. I really appreciate your advice.

    Looking at the IE6 fixes you have already given and noting that you say even more needs to be done helps me understand why so many people hate IE6

    I will check with my employer as to whether we actually need to support IE6. As it would only be used in an administrative backend for a private company, we will probably have some control over what browsers are used.

    I appreciate your time and don't want you to waste it finding further IE6 solutions for this if we don't need to support it, so unless you think it would be a worthwhile exercise in order to help others learn IE6 fixes, please don't bother spending any more time on it.

  17. #17
    The CSS Clinic is open silver trophybronze trophy
    Paul O'B's Avatar
    Join Date
    Jan 2003
    Location
    Hampshire UK
    Posts
    40,549
    Mentioned
    183 Post(s)
    Tagged
    6 Thread(s)
    No Problem - just shout if you do need to support IE6 and we'll go through the fixes needed


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
  •