Skip to content

proper CSS coding guidelines #308

Description

@jancborchardt

Once I get to it, I’ll rework the CSS guidelines and also check all the files. Thoughts for now:

No damn discussions about using Sass / SCSS / Compass. Doesn’t solve any of our problems and will only introduce new ones, plus a barrier to entry.

Generally speaking, we follow Google’s CSS coding guidelines.

No single-line. Every rule gets a line. It’s not sorted alphabetically though but by relation:

  • position and metrics
  • font and color
  • effects like box-shadow and border-radius

Use tabs everywhere for indentation of course, no spaces (as per the global coding guidelines)
Indent blocks depending on the DOM structure so it’s easier to see
As soon as 2 apps use the same CSS, move it into core.
No inline styles, ever.
No leading zeros for decimal values.
No unit when value is 0.
No _underscores, instead use -dashes.
Use only single 'quotes'.
We should look into using prefixfree, also see #201.
Instead of using !important, look into fixing the underlying CSS specificity issue.

Also, I guess we have to say something about ids and classes. What’s the latest reason to not use ids in CSS apart from that you might use the same style again for another element (which is moot in our case of using it for »list« [ex leftcontent] and »detail« [ex rightcontent]).

cc @Raydiation

Activity

  1. DeepDiver1975 commented on Nov 8, 2012

    @DeepDiver1975
    Contributor

    What about line length?
    While viewing code here on github I often face the issue of scrolling - which make code reviews unnecessarily inconvenient.

    Overall 👍 & 🚀 for the rest!

  2. butonic commented on Feb 14, 2013

    @butonic
    Contributor

    @jancborchardt I had to introduce some IE8 / 9 hacks for a customer and found this little nugget which helped me a lot: http://misteroneill.com/journal/improved-internet-explorer-targeting-through-body-classes/
    The basic idea is to use conditional CSS to add classes to the html tag:

    <!DOCTYPE html>
    <!--[if lt IE 7]> <html class="ie ie6 lte9 lte8 lte7"> <![endif]-->
    <!--[if IE 7]> <html class="ie ie7 lte9 lte8 lte7"> <![endif]-->
    <!--[if IE 8]> <html class="ie ie8 lte9 lte8"> <![endif]-->
    <!--[if IE 9]> <html class="ie ie9 lte9"> <![endif]-->
    <!--[if gt IE 9]> <html class="ie"> <![endif]-->
    <!--[if !IE]><!--> <html> <!--<![endif]-->
        <head>
    ...

    This allow to keep IE CSS hacks in one place. A short example for CSS:

    p { color:red; }
    .ie p { color:green; }
    .lte7 p { color:red; }

    what do you think?

  3. jancborchardt commented on Feb 15, 2013

    @jancborchardt
    ContributorAuthor

    @butonic looks good from first sight. What specific IE hacks were there to implement? Also, I thought IE8 was our lowest bar?

  4. butonic commented on Feb 15, 2013

    @butonic
    Contributor

    @jancborchardt Yes I only needed the IE8 part, but I found the switch quite nice so I took it all. Just in case I needed IE9 or IE10 specific css. I had to replace the css based round corners with a background image for the search bar because IE8 cannot do that. I might need to use images for the other input fields as well but as of now they only wanted the search bar themed because they have similar search fields in other parts of the webapp where owncloud is embedded.

    should I create a PR with this or do you want to toy around with it first?

  5. jancborchardt commented on Feb 15, 2013

    @jancborchardt
    ContributorAuthor

    @butonic pull request would be awesome! You probably have way more experience about how to handle IE than me. ;)

  6. raghunayyar commented on Feb 15, 2013

    @raghunayyar
    Contributor

    @jancborchardt shouldn't we be having at-least the internal apps done according to the coding guidelines before OC 5 is shipped?

  7. jancborchardt commented on Feb 15, 2013

    @jancborchardt
    ContributorAuthor

    @raghunayyar no, it’s way too late now and avoiding possible breakage is more important than code uniformity.

  8. jancborchardt commented on Aug 7, 2013

    @jancborchardt
    ContributorAuthor

    Updated the issue a bit according to how we currently do it.

  9. MorrisJobke commented on Aug 17, 2013

    @MorrisJobke
    Contributor

    @jancborchardt Want to sit together and write the documentation

  10. jancborchardt commented on Aug 17, 2013

    @jancborchardt
    ContributorAuthor

    Yes, also cc @frankaellen on this. We should work on this tomorrow.

  11. raghunayyar commented on Oct 13, 2013

    @raghunayyar
    Contributor

    @jancborchardt any updates? We missed this for OC 5 too.

  12. tripflex commented on Dec 4, 2013

    @tripflex
    Contributor

    Yeah i've noticed some issues between apps and the core CSS in public viewing, and has been that way since OC5.

    For instance right now, share an empty folder and allow public uploads, div positioning is all messed up.

    I started going through the CSS and there were def some things I was wondering why they were done that way, especially with margins, padding, and position.

    Would be good to have some real drawn out guidelines to make sure everything works smoothly together :P

  13. MorrisJobke commented on Dec 5, 2013

    @MorrisJobke
    Contributor
  14. tripflex commented on Jan 23, 2014

    @tripflex
    Contributor

    Maybe we could use something like this?
    https://code.google.com/p/closure-stylesheets/

    Thought I would mention it just in case :)

  15. 4 remaining items

  16. removed their assignment
    on Mar 6, 2014
  17. removed this from the milestone on Mar 6, 2014
  18. RobinMcCorkell commented on Mar 8, 2015

    @RobinMcCorkell
    Contributor

    @jancborchardt Do you want to merge this with the coding guidelines in the documentation?

  19. jancborchardt commented on Mar 17, 2015

    @jancborchardt
    ContributorAuthor

    @Xenopathic yes, this and owncloud-archive/documentation#160 as well should just be merged with the current stuff in the documentation as they are now. I didn’t come around to it yet – any help is appreciated.

  20. Volker-E commented on Mar 18, 2015

    @Volker-E
    Contributor

    This seems like a collection of various CSS topics, not only guideline related. @jancborchardt it should definitely be merged with the coding guidelines/standards issue. Is there a coding-guidline.md or similar, where we could add our thoughts, issues and possible topics to address around?

    I've found that WordPress Coding Standards have increased in quality recently. A good one-document example is http://sass-guidelin.es/ - I think it should also feature some nice project inherit styling itself to be appealing to new project contributers.

    Best outcome IMHO would be a document that uses core's CSS and could be exported to an md file as well?!

  21. jancborchardt commented on Mar 18, 2015

    @jancborchardt
    ContributorAuthor

    @Volker-E so there are 2 pages on the website regarding that:

    One is the design page: https://owncloud.org/contribute/design/
    Source: https://github.com/owncloud/owncloud.org/blob/master/page-design.php

    The coding guidelines in the documentation: https://doc.owncloud.org/server/8.0/developer_manual/general/codingguidelines.html#user-interface
    https://github.com/owncloud/documentation/blob/master/developer_manual/general/codingguidelines.rst#user-interface
    There’s also a part about CSS which can be pretty much removed: https://github.com/owncloud/documentation/blob/master/developer_manual/general/codingguidelines.rst#css

    I would prefer to have it all the stuff regarding design in one place. Preferably on the design subpage. Or we just keep the design subpage to resources and the team, and all guidelines go into a separate »design guidelines« document in the docs.
    What do you think @Raydiation @owncloud/designers @Volker-E?

  22. BernhardPosselt commented on Mar 18, 2015

    @BernhardPosselt
    Contributor

    CSS coding guidelines belong into the coding guidelines, app design guidelines (HIG) belong into a separate section.

  23. Volker-E commented on Mar 18, 2015

    @Volker-E
    Contributor

    From my experience from other diverse (also open source projects) all those should go into one place and live as subpages, where different core contributers or teams can work on. Like a handbook. If they are living in different places, finding a bigger picture is in my experience much harder.

    Also there are times when for example back-end specialists want or will have to write frontend code and providing them with one address to go to definitely helps. And there's the momentum of contributors wanting to learn and to participate in other sections of the application, so a strict separation doesn't seem to have advantages to me.

  24. jancborchardt commented on Mar 18, 2015

    @jancborchardt
    ContributorAuthor

    Yeah, so I would say it might be better to split up the different languages for the coding guidelines. Otherwise it’s one really long document you quickly lose overview of.

  25. modified the milestone: backlog on Mar 21, 2015
  26. ghost closed this as completedon Oct 6, 2016
  27. lock commented on Aug 3, 2019

    @lock

    This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

  28. locked as resolved and limited conversation to collaborators on Aug 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions