Skip to content
This repository has been archived by the owner on Jan 1, 2020. It is now read-only.

Is converting of ui.jqgrid.css from CSS to LESS good? #67

Closed
OlegKi opened this issue Jan 16, 2015 · 13 comments
Closed

Is converting of ui.jqgrid.css from CSS to LESS good? #67

OlegKi opened this issue Jan 16, 2015 · 13 comments
Labels
Milestone

Comments

@OlegKi
Copy link
Contributor

OlegKi commented Jan 16, 2015

I don't see sense in converting ui.jqgrid.css from one CSS file to multiple LESS files which have no logic internally. The dist folder don't contains ui.jqgrid.css file. For me personally the information devided in multiple files https://github.com/openpsa/grid.js/tree/master/less is more difficult to read, to compare and to analyse. In any way one need to have static ui.jqgrid.css files which can be loaded from static HTML file and just displays the jqGrid.

To analyse ui.jqgrid.css I personally use always an example of the grid because jqGrid have a lot of jQuery UI classes. In the same way I test the grid together wit Bootsrap. So one have see ui.jqgrid.css *in combination with jQuery UI and other classes from common frameworks. Only analyzing the conflicts with the frameworks allows to set correct CSS rule in ui.jqgrid.css. The topp level jqGrid component with the class .ui-jqgrid (known under the name gbox), another important part .ui-jqgrid-view (gview) and some other will be applied on other CSS rules. The separation in the LESS files makes (at least for me) more difficult to see all classes which will be applied and all looks not like it is in reallity.

It is very difficult to compare the resulting CSS with original ui.jqgrid.css or with ui.jqgrid.css from my fork.

For example look like ui-jqgrid-pager, .ui-pager-control, .ui-jqgrid-toppager and even td select are exist directly under .ui-jqgrid. Such rules on .ui-jqgrid td select seems be applied on the whole grid inclusive jqGrid body (btable) with the main data of the grid. Is it so? Where I can see the resulting ui.jqgrid.css? It would be better to have the file in more compact form so that one can see multiple CSS rules without scrolling vertical, which I need to do permanently if I look at the most LESS files.

It's my main problems in understanding of new `ui.jqgrid.css in LESS formaz...

@flack
Copy link
Contributor

flack commented Jan 16, 2015

Like I've written in my other post: The current LESS files are not very organized. Basically what I did was to take the old CSS file and split it into multiple files according to the comments (like this one: https://github.com/OlegKi/jqGrid/blob/master/css/ui.jqgrid.css#L14). I then converted the CSS selectors into nested LESS statements, but without changing the logic (of course I may have changed some logic without noticing it). so what you write about ui-jqgrid-pager and such actually comes from Tony's original code.

But this is only the first version. I fully plan to do the file more properly later on, and I want to introduce variables, so that on can control e.g. the border color by changing only one line of code.

LESS building is integrated into grunt watch, so the workflow for developing is to run grunt watch in a terminal. When you modify a less file, the CSS file (in the build/ folder) will automatically be regenerated. You can then symlink this file into an existing project (to test integration with other frameworks e.g.), or you can just use the demos in the documentation for reference (they automatically get refreshed by grunt watch, too)

@flack
Copy link
Contributor

flack commented Jan 16, 2015

P.S.: The generated CSS in the build/ folder is uncompressed, so it is somewhat easier to compare with the original. But honestly, I can't think of many reasons to compare them at all. Tony's original CSS is pretty simple (and badly written), so I don't know what insights can be gained by comparing to it

@OlegKi
Copy link
Contributor Author

OlegKi commented Jan 16, 2015

@flack Sorry, but one can't see the file on https://github.com/openpsa/grid.js. Compare with the folder structure here for example.

@flack
Copy link
Contributor

flack commented Jan 16, 2015

@OlegKi That's because the build/ directory is not commited (see https://github.com/openpsa/grid.js/blob/master/.gitignore#L3). build/ is the working directory for grunt, so it will only appear on local checkouts

@OlegKi
Copy link
Contributor Author

OlegKi commented Jan 16, 2015

@flack It's absolutely clear for me. I miss the file ui.jqgrid.css like the file grid.js-0.1.0.js some time before. Why one should install the whole systems used by developers if one just want to use the results? I don't know such in any public available systems.

I think that dist should contain all files required for the usage of jqGrid? The CSS file is do required. So one have to find it in dist . Isn't so?

@flack
Copy link
Contributor

flack commented Jan 16, 2015

Well, I think end users should use the minified CSS, and that is available in dist:

https://github.com/openpsa/grid.js/blob/master/dist/grid.js-0.1.0.min.css

Of course we could add the uncompressed file as well, but IMO it would only invite the wrong usage: Because when users get the uncompressed file, they will be tempted to make their own local modifications, and then they will have problems when they update to a newer release later on

@OlegKi
Copy link
Contributor Author

OlegKi commented Jan 16, 2015

It seem that I understand now how to explain the problem which I have with LESS files.

The first case: I use github repository not only in a projects. If I wrote an answer on the stackoverflow or describe a bug in a forum or in some issue I permanently include references to the line of source code (.js file) of to the line of CSS file (like to the line). I can't reference minimized version of the file.

The next problem: I use minimized versions of JavaScript and CSS files almost only in solution installed in production. I used to use non-minimized versions. For example I have a page where I included bootstrap.css, jquery-ui.css, font-awesome.css and ui.jqgrid.css (in the order) and I see some CSS conflict. Then I can start the developer tools, examine which styles will be overwritten by which another CSS. I can see the line number of the corresponding rule in the CSS file in some web browsers (like Chrome) or I can click on the CSS file near the CSS rule (like on ui.jqgrid.css) to see the rule in the file. Even if the line number will be not shown I can search for the line text like .ui-jqgrid .ui-search-table and will find the line of CSS file which need be used with small modification. I tried to write the texts in ui.jqgrid.css in the same way like Developer Tools shows. Probably I need to fix some lines (include spaces) to fix the problem. I work permanently in the way to create demos like this one.

Moreover the the usage of LESS version of CSS is close to rewriting of some modules in TypeScript of CoffeeScript language. It would be very good for people, who uses the same language as his developer language, but if not, then one will have only problems.

In any way I still don't know any argument which shows some clear advantage for the end users of transformation ui.jqgrid.css to many LES files. There are exist a lot of existing users, who uses ui.jqgrid.css now, who includes his own custom CSS rules to solve conflicts with other CSS files which are included on the HTML pages. I see that providing uncompressed CSS file is really required. Some other arguments, like which files are better readable, could be the matter of taste.

@flack
Copy link
Contributor

flack commented Jan 16, 2015

Your development workflow can be enabled if we tell grunt to also compile a source map. Then, the inspector will point you to the source file line. See e.g.:

http://code.tutsplus.com/tutorials/working-with-less-and-the-chrome-devtools--net-36636

But if you start developing a bit with less, you will find that you really don't use it very much. The advantage of less to the developer is that it makes the CSS structure feel a lot the the DOM of the page, so after a while, you know pretty much without thinking where the selector you're looking for is. Of course, this is assuming that the less files are properly structured (which will happen gradually).

For the end user (using the compressed CSS), there is no advantage. But there is no disadvantage either. People who use less already, or who don't mind using it, an advantage is that we can provide them with an easy way to customize the layout. If users include our less files into their code, they can do for example this:

// Import default layout
@import 'grid.less';
// do some customizations
@grid-default-font-size: 13px;
@grid-border-color: #ccc;

This is just an example. Actually, there are no variables defined yet. But we can do it, and this will also make development easier (because you if you want to change e.g. the border-width globally, you would only have to modify one line of code).

If you think you need an uncompressed version of the CSS available on github, we can add it easily. But I really wonder: Wouldn't it be easier to just post screenshots from the inspector, or doesn't stackoverflow support that?

@OlegKi
Copy link
Contributor Author

OlegKi commented Jan 18, 2015

Thank you for adding uncompressed CSS!

@bouks
Copy link
Contributor

bouks commented Jan 18, 2015

@OlegKi try beautify css on google.

@OlegKi
Copy link
Contributor Author

OlegKi commented Jan 18, 2015

@bouks: Thank you! If the most other prefer such format, then I can live with it. I'll reformat ui.jqgrid.css from my fork in the way, then I hope we could compare it easier with grid.js-0.1.0.css.

OlegKi added a commit to free-jqgrid/jqGrid that referenced this issue Jan 18, 2015
see the discussion openpsa/jsgrid#67 for the
description of the reason.
@bouks
Copy link
Contributor

bouks commented Jan 18, 2015

You're welcome.

@meh-uk
Copy link
Contributor

meh-uk commented Jan 18, 2015

Ok closing.

@meh-uk meh-uk closed this as completed Jan 18, 2015
@meh-uk meh-uk added this to the 1.0 milestone Jan 18, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants