Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.2.0-dev
    • Fix Version/s: 3.1.0-RC1
    • Component/s: Styles
    • Labels:
      None

      Description

      The practice of using IDs as selectors in the css is a serious resource problem. IDs are more strictly scoped then classes and as such do not allow for modularization as classes do. Further more they are more resource costly than classes.

      We need to replace all of the ids in the theme to use classes instead. Wether its all accomplished in this PR or in several separate ones I will leave up to discussion.

      For now I replaced the logo to be a class.

        Activity

        Hide
        VSE Matt Friedman added a comment -

        Where do you get that? Because that flies in the face of what we were all brought up on, which is IDs are meant to be used first and foremost, followed by classes for re-usable elements, because targeting ID's is supposed to be more efficient as there can only be one, while classes can occur many times and require the parsing of the entire DOM to find every one.

        Show
        VSE Matt Friedman added a comment - Where do you get that? Because that flies in the face of what we were all brought up on, which is IDs are meant to be used first and foremost, followed by classes for re-usable elements, because targeting ID's is supposed to be more efficient as there can only be one, while classes can occur many times and require the parsing of the entire DOM to find every one.
        Hide
        VSE Matt Friedman added a comment -

        Look at this jsPerf, shows that IDs are light years better (when used with jQuery, which phpBB is doing a lot of now)
        http://jsperf.com/id-vs-class-vs-tag-selectors/2

        Show
        VSE Matt Friedman added a comment - Look at this jsPerf, shows that IDs are light years better (when used with jQuery, which phpBB is doing a lot of now) http://jsperf.com/id-vs-class-vs-tag-selectors/2
        Hide
        Hanakin Michael Miday added a comment -

        IDs are better for JS but when it comes to css and traversing the DOM using multiple levels of selectors ie '#id .class a' they are more resource intensive than a class and limiting as you can not have multiple ID on the same HTML entity nor reusable code.

        It is considered a best practice to only use IDs for JS and not for css this drastically reduces the amount of IDs which load slower as well as provides for a cleaner more understandable code base when it comes to your JS as well as if an ID is present it is more descriptive as its sole purpose is to either to provide a hook for JS or to provide better semantics to a undescriptive element such as a div

        have a look at any professional framework and you will not see a single ID for this vary reason. give these a read https://github.com/csswizardry/CSS-Guidelines http://www.smashingmagazine.com/2011/12/12/an-introduction-to-object-oriented-css-oocss/

        Show
        Hanakin Michael Miday added a comment - IDs are better for JS but when it comes to css and traversing the DOM using multiple levels of selectors ie '#id .class a' they are more resource intensive than a class and limiting as you can not have multiple ID on the same HTML entity nor reusable code. It is considered a best practice to only use IDs for JS and not for css this drastically reduces the amount of IDs which load slower as well as provides for a cleaner more understandable code base when it comes to your JS as well as if an ID is present it is more descriptive as its sole purpose is to either to provide a hook for JS or to provide better semantics to a undescriptive element such as a div have a look at any professional framework and you will not see a single ID for this vary reason. give these a read https://github.com/csswizardry/CSS-Guidelines http://www.smashingmagazine.com/2011/12/12/an-introduction-to-object-oriented-css-oocss/
        Hide
        VSE Matt Friedman added a comment -

        This ticket is misleading.

        What it should want is to stop using ID's in CSS, not stop using IDs as selectors.

        You should revise your ticket to indicate that you want all CSS selectors to be based around classes, while still retaining existing IDs for the benefit of phpBB 3.1's many javascript events.

        So what you should instead be aiming for is adding classes, not dropping IDs. Additionally, trying to remove IDs from the HTML may have serious unintended consequences of breaking javascript functionality.

        IMO, changes like this are too drastic, bad for backwards compatibility and extensions, and for negligible benefit. Probably better suited for the "next-gen" phpBB theme instead.

        Show
        VSE Matt Friedman added a comment - This ticket is misleading. What it should want is to stop using ID's in CSS, not stop using IDs as selectors. You should revise your ticket to indicate that you want all CSS selectors to be based around classes, while still retaining existing IDs for the benefit of phpBB 3.1's many javascript events. So what you should instead be aiming for is adding classes, not dropping IDs. Additionally, trying to remove IDs from the HTML may have serious unintended consequences of breaking javascript functionality. IMO, changes like this are too drastic, bad for backwards compatibility and extensions, and for negligible benefit. Probably better suited for the "next-gen" phpBB theme instead.
        Hide
        Hanakin Michael Miday added a comment -

        I changed the title although selectors is a css term no a JS one. As to the rest of the statement. Yes I agree it needs to be done carefully in order not to break any JS functionality.

        But its not really negligible as you do see an added performance boost by converting to classes within the css. Not to mention you gain better specificity and lose the need for scope lock ie a#logo:hover WTF to that

        Also as I stated it was up for discussion on how best to handle it. I would suggest going block by block each in its own PR to completely refactor the theme from scratch.

        Show
        Hanakin Michael Miday added a comment - I changed the title although selectors is a css term no a JS one. As to the rest of the statement. Yes I agree it needs to be done carefully in order not to break any JS functionality. But its not really negligible as you do see an added performance boost by converting to classes within the css. Not to mention you gain better specificity and lose the need for scope lock ie a#logo:hover WTF to that Also as I stated it was up for discussion on how best to handle it. I would suggest going block by block each in its own PR to completely refactor the theme from scratch.
        Hide
        VSE Matt Friedman added a comment - - edited

        That's good, but unlike in the commit you submitted, you should be leaving existing IDs in place. People may be relying on them. Just add new CSS classes (with the same name as the ID as much as possible).

        EDIT: I see you updated the commit and put the ID back in

        Show
        VSE Matt Friedman added a comment - - edited That's good, but unlike in the commit you submitted, you should be leaving existing IDs in place. People may be relying on them. Just add new CSS classes (with the same name as the ID as much as possible). EDIT: I see you updated the commit and put the ID back in

          People

          • Assignee:
            nickvergessen Joas Schilling
            Reporter:
            Hanakin Michael Miday
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development