Repository navigation
proper CSS coding guidelines #308
Description
Activity
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!
@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?
@butonic looks good from first sight. What specific IE hacks were there to implement? Also, I thought IE8 was our lowest bar?
@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?
@butonic pull request would be awesome! You probably have way more experience about how to handle IE than me. ;)
@jancborchardt shouldn't we be having at-least the internal apps done according to the coding guidelines before OC 5 is shipped?
@raghunayyar no, it’s way too late now and avoiding possible breakage is more important than code uniformity.
Updated the issue a bit according to how we currently do it.
@jancborchardt Want to sit together and write the documentation
Yes, also cc @frankaellen on this. We should work on this tomorrow.
@jancborchardt any updates? We missed this for OC 5 too.
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
Maybe we could use something like this?
https://code.google.com/p/closure-stylesheets/Thought I would mention it just in case :)
4 remaining items
@jancborchardt Do you want to merge this with the coding guidelines in the documentation?
@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.
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?!
@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.phpThe 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#cssI 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?CSS coding guidelines belong into the coding guidelines, app design guidelines (HIG) belong into a separate section.
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.
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.
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.
- locked as resolved and limited conversation to collaborators
on Aug 3, 2019
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:
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