Refactor Zulip CSS to be split into components #731

Open
timabbott opened this Issue Apr 28, 2016 · 7 comments

Projects

None yet

2 participants

@timabbott
Member

Possibly as part of this project, we should also use some fancy CSS preprocessing technology; but the overall goals are (1) to make things easier to find and understand and (2) enable projects like #301 and #267.

@timabbott timabbott added this to the 2016 roadmap milestone Apr 29, 2016
@showell
Contributor
showell commented Jul 18, 2016 edited

I spent a little bit of time today trying to "rationalize" our CSS. I didn't get too far, party for good reasons (there's nothing drastically wrong with our CSS) and partly for reasons that are easy to overcome (I tried to use dirty parsing on the CSS to figure out possible re-factorings, and there were some formatting issues in our current zulip.css that prevented this from being as easy as I thought originally.)

In addition to splitting out CSS into files based on subsystems like settings/administration, which I think is clearly a good idea, there is also the notion of splitting out CSS on different styling dimensions, which might be slightly more unorthodox.

One simple example is to have a CSS file purely dedicated to colors and fonts. The rationale for doing that is that our CSS addresses some concerns that are very brittle (alignment/line spacing/etc.), and some concerns that are probably more about implementation than pure styling (elements that need to be hidden until jQuery activates the elements, etc.), and then finally some concerns that are actually just about visual polish (color, font type, etc.). It might be good to separate those concerns.

A subtle benefit of splitting our CSS out into the three dimensions that I mentioned above is that we might be able to find more semantic commonalities between different HTML elements and re-work our HTML classes to some degree. To give an example, you might have two widgets that have very similar alignment properties, which are related to their semantic commonality, but they get rendered in different colors, because they are in fact two separate widgets. Right now the structure of our CSS makes it difficult to infer where those commonalities might exist. If we separated the concerns of alignment, visibility, and color/font in our CSS, we might be able to more easily refactor our CSS to be more concise and consistent with automated tools or brute force human analysis.

@timabbott
Member
timabbott commented Jul 18, 2016 edited

For constants like colors, a common way to handle that which we might want to consider using for Zulip is an extended CSS language like: http://sass-lang.com/guide

@showell
Contributor
showell commented Jul 18, 2016

Yeah, I'm pretty ambivalent about SASS/SCSS for Zulip. The preprocessors definitely have some nice features, but for the deeper structural issues of how you organize your CSS, it's still possible to have confusing structure in SASS, and it's not really that hard to have sensible structure in raw CSS. (There are a few things lacking in raw CSS, like being able to do arithmetic computations, but the actual language is flexible in terms of being able to nest styles, apply the same styles to multiple selectors, etc.)

Do we have a ticket that addresses moving to SASS (or SCSS)? I've used both in various projects. They introduce a little conceptual overhead, and you have to make sure your asset pipeline is robust, but generally I think they're worth it. I've never used either in an open-source context, though, and in an open-source context, it's possible that just using raw CSS lowers frictions for some possible contributors.

@timabbott
Member
timabbott commented Jul 18, 2016 edited

I think this is the most relevant ticket for that topic. I agree we should try to make our CSS as nice as we can directly before investing in / choosing SASS/SCSS.

This was referenced Aug 3, 2016
@showell
Contributor
showell commented Sep 10, 2016

In #1530 @vikas-parashar suggests that we have a naming convention that tells us which HTML classes are not to be used for styling and should be reserved for DOM manipulation. One possible prefix is js-, but I think that might be slightly confusing, since JS often manipulates HTML classes related to styling. I can't think of a great alternative--you could say something non-css-dom-location-foo, but that is really verbose. In this page somebody suggests using simply - as a prefix.

https://coderwall.com/p/qktuzw/decouple-javascript-classes-from-css-ones-by-using-prefix

Somebody there also makes the counter argument that it's perfectly reasonable to use the same class in both contexts, but I don't really agree with them for web apps. HTML was designed for documents, and semantic purity makes sense there, but for web apps, I think it's more practical to separate out styling from manipulation.

@timabbott
Member

Might be good to have a list of what tags we're thinking about doing this with as examples in the discussion. I think it's hard to know what's correct in the abstract.

@showell
Contributor
showell commented Sep 15, 2016

A good reference for type of HTML classes/ids that get handled by JS in the Zulip codebase is here:

https://github.com/zulip/zulip/blob/master/static/js/click_handlers.js

You'll see that they're kind of all over the map in terms of naming. Some of the names are kind of verb-ish, which I think is good: topic_edit_save, topic_edit_cancel, narrows_by_recipient.

A good rule of thumb for code review going forward might be to be suspicious of any CSS selector that has a verb-like name in it.

@timabbott timabbott added the area/misc label Oct 14, 2016
@timabbott timabbott modified the milestone: Zulip roadmap, Old roadmap Nov 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment