New less organization - Githubissues

bouks commented 9 years ago

I suggest we organize less files better, then we can change and/or optimize easily each part.

one folder for each component (base, formedit, inlineedit, modal, subgrid, treegrid...)
in each folder, separation of colors, fonts, positionning and sizes rules.

Here's a proposal (it can be improved)

https://github.com/bouks/jsgrid/tree/master/less_proposal

meh-uk commented 9 years ago

Doesn't this add complexity due to more files?

bouks commented 9 years ago

On the contrary, it is simpler when working css due to separation... maybe like Single Responsability or MVC patterns in coding.

meh-uk commented 9 years ago

That's a good point, moving to 2.0 as I'd quite like to make a release and this can wait :).

bouks commented 9 years ago

I think if we agree with this, we can implement now. There is no changes in the rules, on separation. But need to adapt the build and why not the download builder to reduce css file size.

bouks commented 9 years ago

Fast and simple optimization : https://github.com/bouks/jsgrid/commit/e9fed03be8fb67bf0948d5c6ec4af843c02f78f8

flack commented 9 years ago

I must admit I've never been a fan of the philosophy of separating CSS rules into different "roles" like colors, positioning etc. I've seen this used on some (mostly quite old) projects, but it always ended up making things more confusing.

Usually, people say that this makes e.g. theming easier, because all the font stuff and all the colors are in one place and so on. While this may be true, it does not apply to our situation for a number of reasons:

we do not want people to customize the look by changing our CSS file. They should do their customizations in a separate file, to avoid problems when updating
the standard CSS file we ship does not really define colors, fonts etc. anyways, but mostly uses inherit to get the correct ones from the jquery ui theme
grouping rules by function is only necessary in CSS because it doesn't support variables. So you put all places that have the same color next to each other so that you don't miss one when changing it. With LESS, we don't need that workaround, because we can simply define the color (or font size, etc.) in a variable. Usually, all variables are in a file called variables.less (for example: https://github.com/twbs/bootstrap/blob/master/less/variables.less), which gives you one convenient place to change everything at once.

It is a matter of taste of course, but for me, having five different files named pos.less and potentially four different files where each selector is defined ({color|font|pos|size}.less) does not sound simpler to maintain

flack commented 9 years ago

To give you an example:

CSS separated by roles:

/* color */
a
{
    color: red;
}
h1, p, button
{
    color: black
}
button
{
    border-color: black;
}

/* font */
a
{
    text-decoration: none;
}
h1, p, button
{
    font-family: Arial
}

The same in LESS with variables:

/* define variables */
@link-color: red;
@default-color: black;
@default-font: Arial;

/* apply rules */
a
{
    color: @link-color;
    text-decoration: none;
}
h1, p, button
{
    color: @default-color;
    font-family: @default-font;
}
button
{
    border-color: @default-color;
}

bouks commented 9 years ago

we do not want people to customize the look by changing our CSS file. They should do their customizations in a separate file, to avoid problems when updating

But... people could contribute by proposing themes Some of them would like to change color or only fonts stuffs. It's, then, easier to do.

the standard CSS file we ship does not really define colors, fonts etc. anyways, but mostly uses inherit to get the correct ones from the jquery ui theme

We talked about abandon of jqueryui theming, so we'll have to implement colors and fonts. Also separation would easily permit to use others css frameworks for all OR only "one stuff". For example, jqueryui for colors and bootstrap for placing... or other combination.

About grouping rules and less variables, it's not the same usage. I talk about readability and independency.

bouks commented 9 years ago

These are principles we apply in real coding (not really in this project but...), why not apply these in css ?

bouks commented 9 years ago

With this organization, i could fastly detect some redondances in css rules and remove them. Here :

https://github.com/bouks/jsgrid/commit/e9fed03be8fb67bf0948d5c6ec4af843c02f78f8

So it can avoid easily useless overcharging rules.

flack commented 9 years ago

But... people could contribute by proposing themes Some of them would like to change color or only fonts stuffs. It's, then, easier to do.

But it would be even easier if they can do all their changes in one file, i.e. variables.less

We talked about abandon of jqueryui theming, so we'll have to implement colors and fonts. Also separation would easily permit to use others css frameworks for all OR only "one stuff". For example, jqueryui for colors and bootstrap for placing... or other combination.

AFAIK, no other css framework uses the separation you propose. For example jquery ui and bootstrap both have one file per component, not separated colors/fonts/etc.

About grouping rules and less variables, it's not the same usage. I talk about readability and independency.

But don't you think that reading @default-color gives you a better explanation of what it does than black in my example? It is also easier to grep, and adds some semantic information.

These are principles we apply in real coding (not really in this project but...), why not apply these in css ?

I'm not sure that the MVC paradigm is a good model for CSS, since this is already inside the "V" layer. I would go more with the Single Responsibility Principle, i.e. one "class" (i.e. file) contains one piece of functionality (e.g. subgrid.less) and exposes an "interface" (i.e. variables) with which the functionality can be customized. With your separation proposal, the implementation of each piece of functionality will be spread out across multiple files, and have no interface.

I think reducing redundancies is a good thing, and I know that the files are currently not very well structured (because they still correspond a lot to Tony's CSS structure), but I'm still not convinced multiple files per module is the answer.

bouks commented 9 years ago

But it would be even easier if they can do all their changes in one file, i.e. variables.less

Sure, but what i propose doesn't exclude variables items. The conjonction of the two stuff is good.

AFAIK, no other css framework uses the separation you propose.

I'm not conducted by others doing or not doing a thing. Hope there's people thinking and doing different. I'm not a sheep. I'm not praying lords of frameworks. :) Today we talk about abandon jqueryui, so is it a good example ? Bootstrap is fashionable today, but tomorrow ?

But don't you think that reading @default-color gives you a better explanation of what it does than black in my example? It is also easier to grep, and adds some semantic information.

Like i said above : "what i propose doesn't exclude variables items". It's not the same purpose.

the implementation of each piece of functionality will be spread out across multiple files, and have no interface

Yes. But it permits to have a global and large view of the project (in css part of course)

bouks commented 9 years ago

about "I'm not conducted by others doing or not doing a thing..." I'm conducted by best practices, not "edicted rules" or "what this popular project do".

I've done this type of separation in my projects for years after doing it "normally". And, by this experience, i'm totally satisfied. It may be a litlle bit longer in the start dev, but really cool in maintenance and updates.

flack commented 9 years ago

Sure, but what i propose doesn't exclude variables items. The conjonction of the two stuff is good.

I'd rather say one makes the other superfluous

I'm not conducted by others doing or not doing a thing. Hope there's people thinking and doing different. I'm not a sheep. I'm not praying lords of frameworks. :) Today we talk about abandon jqueryui, so is it a good example ? Bootstrap is fashionable today, but tomorrow ?

The only projects I have seen using this kind of separation are the ones from yesterday (a client of mine had that for 6 years or so. It became an intolerable mess, and is being replaced right now), and your example was about using positioning from bootstrap and colors from jquery ui. I just pointed out that both are not structured in a way to allow this kind of usage, so your example is moot.

Also, even if it worked, you would still have to touch multiple color.less files, which is suboptimal

Yes. But it permits to have a global and large view of the project (in css part of course)

But that only means that it is a good format for reverse engineering. Structuring LESS files so that they look good in CSS is a bit like writing Javascript the way you would write PHP. It may make it easier for "switchers" to get started, but the code will always be suboptimal in structure.

bouks commented 9 years ago

If you define a variable and some selectors use the same variable in a rule.

If you don't separate the code, the ruleset's selectors may be too long to see redondancy. If you separate, you can fastly see the "variabled rule" redondancy.

It may also avoid creating new selectors and rules by easily see refactoring possibilities.

bouks commented 9 years ago

It became an intolerable mess

The mess can result in bad human implementation of a thing. Some people uses good frameworks and can do crap things. :)

flack commented 9 years ago

There are also tools to detect redundant CSS. I don't think this has to be done manually all the time.

The variabled redundancy can also be spotted by grep for example.

And I don't think we will need to do very many refactorings. For backwards compatibility, we should avoid breaking existing custom stylesheets anyways, and the entire CSS has only 11KB, so there is not so much potential for optimizations.

flack commented 9 years ago

Also, I think the "separation of concerns" is a bit artificial. For example, if you use a certain font size, you will need a padding that works with it. Or if your icons are 20px high, then the line-height of the neighboring text needs to match. With the separation, you would have to modify multiple (theoretically independent) files at the same time

bouks commented 9 years ago

If you have tools that can detect redundancy in mixed-nonredundant-rules, please, tell me where.

Backwards compatibility is associated to js files. No one will update the css and not the js...

bouks commented 9 years ago

For example, if you use a certain font size, you will need a padding that works with it. Or if your icons are 20px high, then the line-height of the neighboring text needs to match. With the separation, you would have to modify multiple (theoretically independent) files at the same time

Yes, but you will see easily if you need to add or refactor a rule. Sure you open two files. What a big deal.

In a PHP project, for example, if you change (adding, removing...) properties of an entity, then you maybe have to open entity file, controller file(s), view file(s)...

flack commented 9 years ago

If you have tools that can detect redundancy in mixed-nonredundant-rules, please, tell me where.

I've googled quickly, and these two sound promising:

Backwards compatibility is associated to js files. No one will update the css and not the js...

Sure, but lots of people override the bundled styles in their own CSS

flack commented 9 years ago

In a PHP project, for example, if you change (adding, removing...) properties of an entity, then you maybe have to open entity file, controller file(s), view file(s)...

Well, yeah, but that is because there are three different concerns (model, view, controller) that have three independent functions (persistence, presentation, business logic). In CSS, everything is presentation. So why make it unnecessarily complicated?

flack commented 9 years ago

Sure you open two files. What a big deal.

The two files I want to have open are subgrid.js (business logic) and subgrid.less (presentation). And maybe backend.php (persistence)

bouks commented 9 years ago

You can define as much layers you want. Some use MVC, others use MVTC. But most of MVTC will say they use MVC. Also you can consider model (M) to three different layers -> DBAL, ORM and entity layer. M is just convenience call.

For me there are four different major concerns/layers. Positionning, sizing, fonting and coloring. :)

flack commented 9 years ago

I've had an idea: Since we obviously won't convince each other any time soon, how about meeting in the middle? We could leave one file per module, and then group the rules inside the file by "concerns". I'm still not really convinced it's useful, but it might be a good compromise: It saves me the hassle of opening more files than I have to, we don't need to adapt the builder or grunt, and you still have the grouping you wanted

bouks commented 9 years ago

MVC is not the best example, that it is vertical layers. For horizontal layers, there is, for example, Strategy or Chain-of-responsibility.

bouks commented 9 years ago

If my proposition is not adopted, sure i accept your compromise proposal. :)

meh-uk commented 9 years ago

I like the compromise.

bouks commented 9 years ago

Commited :

https://github.com/bouks/jsgrid/commit/4f38e0fa6c7aa86a056cf6b834abe457274bb773

In my PR.

openpsa / jsgrid

New less organization #116