free-jqgrid / jqGrid

jQuery grid plugin
https://github.com/free-jqgrid/jqGrid
Other
480 stars 196 forks source link

Simplifying CSS for better support for icon fonts #1

Closed smartcorestudio closed 9 years ago

smartcorestudio commented 9 years ago

Hi! Here are 2 tests, based on your demos: http://smartcore.ru/test/ - with FA icons http://smartcore.ru/test/index2.htm - with UI icons 1) You can easily change size of the base text font or FA icons font via Firebug/Developer tools to check how it looks in different situations. 2) I deleted all lines about modal dialog buttons from your FA integrations CSS. 3) JS needs to be changed to make different button structure (see my original comment in grid.js https://github.com/openpsa/grid.js/issues/71#issuecomment-71226197). In this demo JS isn't changed, so there is no space between text and icon.

OlegKi commented 9 years ago

Hi!

Thank you very much. I hope that I understand now what you mean, but I think that one will still probably need the -icon-left and -icon-right classes at least to have be able to use direction: "rtl" option (compare the demo with this one, in the same way should RTL dialog have some elements in another order). I will examine the changes morning.

I posted last tame many changes in jqGrid. So it would be better if you would use copy of jquery.jqGrid.src.js, ui.jqgrid.css, jQuery.jqGrid.fontAwesome4.css and jQuery.jqGrid.fontAwesome4.js for next demos. In the case one will have less side effects of the latest changes on your demo made with previous versions of the files.

OlegKi commented 9 years ago

Hi @smartcorestudio !

I posted the changes of ui.jqgrid.css, the form editing module (grid.formedit.js) and fontAwesome4 plugin which I made based on your suggestions.

Thank you for the demo and your comments! I hold fm-button-icon-left and fm-button-icon-right classes to have better support of existing options of editGridRow, viewGridRow and delGridRow which allow to specify parameters like saveicon, closeicon, delicon, cancelicon. The values of the parameters are arrays like [true, "left", "ui-icon-disk"] or [true, "right", "ui-icon-disk"] where the second parameter specify the position ("left" or "right") of icon relative to the text. I don't know how many users really uses the option, but I implemented the feature just to be mostly compatible to old versions of jqGrid.

I made many other changes in jqGrid last days, so I think that I do move the code of fontAwesome4 from plugin to the main code of jqGrid. In the way one can just specify fontAwesomeIcons: true option to inform jqGrid that fontAwesome4 icons should be used instead on jQuery UI Icons.

smartcorestudio commented 9 years ago

Thank you for your work Oleg! I didn't know about possibility to customize the alignment of icons as you described above, so I didn't see any reasons of using right and left classes. Now I see it. Do you have any plans about release? Milestones?

OlegKi commented 9 years ago

I improved the code posted yesterday and reduced some parts of CSS files. I posted results to my repository.

I plan to publish the first release of my fork in February (probably already in the middle of the February), but some other things which I need to do can change a little my plans.

OlegKi commented 9 years ago

@smartcorestudio: Could you try the current implementation of Font Awesome. You need just add iconSet: "fontAwesome" option to use new set of icons. Probably you have suggestion about using other icons as I included in the set? Probably you can some suggestions about the names of properties inside of $.jgrid.icons.fontAwesome (and other)? I changed the look of sorting icons. One can use

$.extend($.jgrid.icons.fontAwesome.sort, {
    common: "fa-lg",
    asc: "fa-sort-asc",
    desc: "fa-sort-desc"
});

and set showOneSortIcon: false together with iconSet: "fontAwesome" to have the look of sorting icons close to jQuery UI icons.

Any feedback from you side is welcome. It would be wonderful if you could create some other set of icons. Probably one will see new requirement and I will have to make some changes in my current implementation to be able to use another set of icons.

Best regards Oleg P.S. You can write me in Russian if it's easier for you.

smartcorestudio commented 9 years ago

OK, Oleg! I'll try to test your current implementation this weekend in my project. Are you going to split iconsets into separate files (similar to locales) in the future? Of course it's much easier for me to write in Russian, but since GitHub is international I think it will be better if it will be understandable for everyone. Also it's a good practice for me in English :-)

OlegKi commented 9 years ago

Fine! I thought about separation iconsets into separate files, but I don't want to do this in the next version (which I want to publish this month). The reason: I want to point to new feature - replacing old style icons to Font Awesome icons. I though even about making iconSet: "fontAwesome" as default. If somebody prefers old style of icons he can just add iconSet: "jQueryUI" jqGrid option or one can change defaults using $.extend(true, $.jgrid.defaults, {iconSet: "jQueryUI"}).

I want to include support of AMD (mostly requirejs) in the later version (in March or April). I want to try to make the implementation as dynamical as possible, so that some dependent files (TreeGrid, iconset) will be loaded (if there not have been loaded before) depend on the options used in jqGrid. I wanted to make some experiments with declaration CSS dependencies (at least one theme of jQuery UI CSS without jQuery UI itself). Only if one would use specific options sortable: true or specific methods (sortableRows, gridResize and other) then the corresponding modules of jQuery should be loaded. After having implemented such behavior one could make very good separation of the code in parts.

By the way I plan to publish the my jqGrid version on free CDNs (cdnjs.cloudflare.com for example) direct after publishing and will change URLs of all demos to URLs from CDN. In the way the loading of files will be not so important problem as before.

I have many other ideas what can be implemented in jqGrid more, but I want to make the first cut and create the first version and some documentation which I want to improve permanently.

smartcorestudio commented 9 years ago

Hi Oleg!

I've tested your current jqGrid version with FA icons in my project. It works OK out of the box! But of course I have some suggestions and some questions. The suggestions reflects my personal taste. Maybe I'm not a gread coder, but I'm rather good designer and I pay much attention to the visual component of UI. So I had to make some overrides of your default styles. 2015-02-15 14_25_55- dekonadzor ru 2 1) I'd like default size of FA icons to be 14px. It's designed to be pixel perfect at this size 2) In general all jqGrid elements look too small nowadays. It looks like it's not 2015 now, but 00s. I'd like to increase base font size from 11 to 12px and slightly increase paddings. 3) Is it possible to change a particular icon via options? In my case I have to change some icons for the consistency with other parts of my application. I think that this possibitity should be documented. 4) Is it possible to add custom classes to buttons? For example, I'd like to add Bootstrap classes "btn-danger" to delete button, and "btn-primary" to save button for the consistency with other parts of my application. 5) Maybe it's better to place search icon before the text in the "Find" button, to make it look similar to other buttons. 6) I've added some margin between "Save" and "Cancel" buttons 7) I prefer old style sort icons in the table headers. To my mind it looks clearer than the new ones. You can ask the other people of course. The only thing is the old icons may require additional paddings for vertical alignment with the header text 8) To my mind using "Undo" icon for "Cancel" button looks weird in some cases, especially in "View" modal. It doesn't "Undo" anything in this case. Maybe it's better to use "fa-ban" button. Also you can think about replacing "floppy" to simple "check" for save button. 9) I don't like that inputs in the filter toolbar always occupies 100% width of the cell. And "Clear Search value" button has fixed width. Now it looks inaccurate with the different column widths. It will be much better if we'll have something tike Bootstrap input groups http://getbootstrap.com/components/#input-groups-buttons or clear search will be just absolutely positioned inside the input, near it's right side. Also maybe you can think about replacing text "X" for appropriate icon for this button. 2015-02-15 14_29_27- dekonadzor ru 2

And the last part, not about CSS. Do you have any plans about "Download builder", similar to original Tony's or Grid.js? I find it very convenient to have everything you need, but nothing that you don't in one minified js file. Simple and efficient.

I'm going to add an iconset for those who use standard Bootstrap Glyphicons.

P.S. Sorry for my poor English...

OlegKi commented 9 years ago

@smartcorestudio : Thank you very much for your feedback.

I'm not a designer at all. So your point of view is very interesting for me. I'm far from the subject so I'm not full understand the problem with the font size. All my customers are corporations which use web sites with jqGrid in internal LAN from desktop computers or notebooks. The web browsers like IE10 or Chrome saves the last zoom factor automatically. Chrome saves the zoom factor for every site separately. The users who have large display (or two displays) use mostly the standard 1:1 zoom factor. Other (like me who have not so sharp eyes) just uses 125-200% zoom. It will be either set automatically for the web site (last used zoom factor for the site) or one need to make 1-2 clicks (or use scroll rad of mouse or press Ctrl+) at the beginning of working with the site and the problem will be solved. The only problem for me was because the old jQuery UI icons looks not good enough after scaling. The Font Awesome icons don't have the problem more. So I'm not sure where there problem with the icons exist. Probably you use some specific destination devices where zooming is not so easy or you mean some another scenario? Could you explain the problem for me more clear (for dummies)?

I understand that the new version of jqGrid (which I will publish) could not be 100% compatible with jqGrid 4.7, but I don't want to makes too much incompatibility. All old versions works with font-size: 11px. So I plan don't change the factor at least for the first version which I'll publish. I removed from ui.jqgrid.css many places (but not all) where height were fixed and replaced there to height: auto;. Moreover I removed a lot of inline CSS settings which set styles directly in JavaScript code before. So I hope that one could relatively easy to overwrite the font-size and other settings without adjustment a lot of other settings. One need just add some additional CSS rules. To change the size of navigator icons one can just add .navtable .fa { font-size: 14px; } CSS rule or example. One can use

.ui-jqgrid .fa { font-size: 14px; }
.ui-jqgrid .ui-jqgrid-htable .ui-jqgrid-sortable { height: 18px; padding-top: 2px; }

to increase the font size icons in the whole grid to 14px. I full agree with your that the font size 14px for navigator icons .ui-jqgrid .fa { font-size: 14px; } looks for me better too, but I'm not sure that one should do the same for inline icons from the formatter: "action". The results looks like 14px The sorting icon and the actions icons looks for me too large. But I'm not designer and I'm not sure... If one would add .ui-jqgrid > .ui-jqgrid-view { font-size: 12px; } to change the default font size than one would see 14px12px which seems not so good. One should increase padding and so on. So I'm afraid that one would have to make many additional changes in the CSS of the grid to make all be look good with all 14px icons.

My main goal was to provide the version of jqGrid which could be very easy to customize (with small set of new CSS rules) without changing JavaScript code. I still want to provide possible good default settings. If you would say that .navtable .fa { font-size: 14px; } or some another changes would be better as the current one after reading of my above arguments I would make the changes in jqGrid. Do you think that common setting .ui-jqgrid .fa { font-size: 14px; } are good too for the inline icons (I mean formatter: "actions")?

The changing of default font-size from 11px to 12px seems me a little danger from the migration point of view. I'm afraid that the usage of 12px will required changing of a lot of width properties of colModel of grids which are fixed coded in a lot of existing code over the world. I don't want to change the setting at least for the first version of jqGrid which I would publish.

So I ask you to consider some changes in icon size which don't required changing the font-size in the grid. Please could you look at the problem once more?

The answers on your other questions are relatively easy:

3) Is it possible to change a particular icon via options?*

Yes. You can use for example $.extend($.jgrid.nav, { addicon: "fa-cart-plus" }); (old style of setting Add icon) or $.extend($.jgrid.icons.fontAwesome.nav, { add: "fa-cart-plus" }); (in new style).

The structure of icons from $.jgrid.icons is very easy. You can see there here. So you can overwrite any from the icons with one line of the code

4) Is it possible to add custom classes to buttons? For example, I'd like to add Bootstrap classes "btn-danger" to delete button, and "btn-primary" to save button for the consistency with other parts of my application.

Yes. You can use for example

$.extend($.jgrid.icons.fontAwesome.form, {
    save: "btn-danger fa-floppy-o",
    del: "btn-danger fa-trash-o"
});

5) Maybe it's better to place search icon before the text in the "Find" button, to make it look similar to other buttons.

I agree. I changed the code. I'll publish the changes soon.

6) I've added some margin between "Save" and "Cancel" buttons

Do you mean something like .EditButton a.fm-button {margin-left: .2em;}? In the case the same margins will be in Delete/Search dialog too. Delete dialog contains currently   between the buttons. I can include  , as the alternative to margin solution, between "Save" and "Cancel" button and between "Query" and "Find" in the search dialog too (if one use showQuery: true searching option then "Query" button will be displayed). I would make the changes in the code of jqGrid if you send me the exact suggestions of CSS.

7) I prefer old style sort icons in the table headers. To my mind it looks clearer than the new ones. You can ask the other people of course. The only thing is the old icons may require additional paddings for vertical alignment with the header text

OK, I'll change back to large "fa-sort-asc" and "fa-sort-desc" as default settings. I will just include an example how to configure "fa-sort-amount-asc" and "fa-sort-amount-desc" instead and to show only one icon from two sorting icons at the same time.

8) To my mind using "Undo" icon for "Cancel" button looks weird in some cases, especially in "View" modal. It doesn't "Undo" anything in this case. Maybe it's better to use "fa-ban" button. Also you can think about replacing "floppy" to simple "check" for save button.

I agree with you in case of usage "fa-ban" button in View. I've made the corresponding changes in my code already. I'm not sure about replacing "floppy" to "check" button. The same "Save" icon will be used in fomatter: "actions" (inline save icon) and in the navigator icons of inlineNav (see the demo or this one). I use currently only one icon for all the cases and the usage of "check" button in other cases seems me not good.

9) I don't like that inputs in the filter toolbar always occupies 100% width of the cell. And "Clear Search value" button has fixed width. Now it looks inaccurate with the different column widths. It will be much better if we'll have something tike Bootstrap input groups http://getbootstrap.com/components/#input-groups-buttons or clear search will be just absolutely positioned inside the input, near it's right side. Also maybe you can think about replacing text "X" for appropriate icon for this button.

I think that the changes of Searching Toolbar I will do in the next release (I mean one release later). I want now just preparing the next release with already existing changes. It would be fine if you would open separate issue where you describe the problem and your exact suggestions more detailed. I'll use the suggestion later.

Another question to you: do you tried to use Font Awesome icon with TreeGrid? See the demo as an example. I didn't found good icon for the leaf. I use currently "fa-dot-circle-o" instead of "ui-icon-radio-off", but I don't like the choice. Do you have probably a better idea?

Best regards Oleg

smartcorestudio commented 9 years ago

Hi Oleg!

1), 2) FA is designed by Dave Gandy "... to be razor sharp at Bootstrap's default 14px". I think that the most people use the default 100% zoom. And in this case 14px icons looks much better than the smaller ones, because Dave specially optimized them for using at this size. You can look at the recycle bin icon for example and compare how it looks @12px and @14px. You asked me to describe my use case. OK. I use jqGrid on some pages of my CMS. Of course my CMS is optimized for the default 100% zoom. So it's not a good idea to switch the zoom on every page which contain jqGrid. And of course I had to override the default jqGrid sizes and paddings. It's not a problem for me, I've already done this. I totally understand your point of view about compatibility with the earlier versions, I also thought about this and I respect this. It just depends on your main goal. It's OK for the existing users to use the same font size as in earlier versions. But I think that the most of the new users don't like how it looks out of the box, so they'll have to write their custom CSS. So if you want to keep the life of the existing customers as smooth as possible of course it's better to keep the size, If you want to attract new users, to make their life easier, maybe it's better to refresh the style for better meeting the modern realities To resume: I think that it will be better to use 14px FA icons by default at least on nav and modal buttons. Also you sholud remove "bold" weight for inline FA icons 3) Is it possible to change several icons from the different sections using just one $.extend ? 4) I tried your code but I had no luck, because it applies this class to the icons, not to the buttons 2015-02-16 09_29_29- dekonadzor ru 2 6) I think it doesn't matter how you'll do this but there should be some space between buttons. I used margins for this. 8) "floppy" icon - OK, I've got your idea. Everyone can easily change the icons which he don't like 9) OK, I'll make a separate issue for this. Do you have any public "todo list" for the next milestone?

Tree grid. I think that your current icon is acceptable. Also I think that by default leaf items don't need any icons at all, it makes no sence. It's here maybe just for the historical reasons. More important that users can always use their custom icons for the leaves if they want. If the leaves are files user can use "file" icons, if the leaves are users - than it will be logical to use "user" icons etc.

OlegKi commented 9 years ago

@smartcorestudio Could you post a link which shows an example how you use jqGrid? Which jQuery UI Theme you use? The standard jqGrid is oriented only on jQuery UI CSS. If you look on the demo which uses the original jQuery UI icons (look at the demo) you will see that the icons are relatively small, but the text size corresponds the size of the icons. Any integration it together with Bootstrap or other CSS framework required many customization steps. I really want to understand the problem from 1) and 2), but I still can't. I discuss with customers about many changes which the customer want, but I never hear before about increasing the font size. So I'm sure that we have different testing environments. Which DPI have your destination environment? Which size of monitor you use? I personally can't good read no from the sites, which you reference on http://smartcore.ru/, with 100% zoom. I use typically 150% or 125% for someone. I use zooming on practically all sites since years and I do this absolutely automatically without think about it.

In any way I added fa-lg class to $.jgrid.fontAwesome.nav.common and separated setting for inline and navigator icons. I made 14px icons as default for navigator icons and 12px (instead of 11px previously) for inline icons. I hope that all looks now better as before. I removed "bold" weight from inline FA icons. Thank you!

About 3).One can set all the changes which you need using one $.extend. The jQuery method can be used co combine property of multiple objects. It's really easy to use. If you would post my an example of what you do I can post you the corresponding modified example.

About 4). the results looks good for me. One can see that the CSS for are changed. The problem is just I don't have any code example or any demo. I have no idea which jQuery UI Theme you use and what exactly results you expect by adding of new CSS. I suppose that you have some problems with combinations of jQuery UI CSS classes with Bootstrap classes, but without an example I can't solve the problem. Could you post the corresponding demo and to explain more details what you want to achieve?

About 6). I can repeat that I'm not designer and if you write "it doesn't matter how you'll do this but there should be some space between buttons" it gives me no information. You wrote before about "some margin". My problem is in the exact value. Should it be in em, px etc? Which exact numeric value it should have? First of all I included   which I personally not like, because other people can't set his custom CSS rule to change the size of gap between the buttons. If you would post me the exact suggestion I would change the code.

I don't have any public "TODO list" for the next milestone till now. I plan to create it soon. My current goal is preparing the the current version of jqGrid. I'll post my current changes which I made today later. I'm making some changes in all language files. After that I'll make mostly bug fixes rewriting README as release notes, including more detailed documentation of new features, more examples and so on.

Thank you once more for your feedback.

Best regards Oleg

smartcorestudio commented 9 years ago

1) 2) I'll send you screenshots to oleg.kiriljuk@ok-soft-gmbh.com and try to explain there in Russian 4) I just want the WHOLE BUTTON, not just an icon was blue with the white text. Like this: http://getbootstrap.com/css/#buttons-options So I asked about the ability to add custom classes to the BUTTONS, not to the icons.

smartcorestudio commented 9 years ago

3) Just FOR EXAMPLE. Let's imagine that I want to change "del" icon BOTH in "form" and "nav" to "fa-times". Actually I want to change more icons from different categories, but let's take this for example.

OlegKi commented 9 years ago

3) Let's imagine that I want to change "del" icon BOTH in "form" and "nav" to "fa-times".

It's very easy. You need to use the code like

$.extend(true, $.jgrid.icons.fontAwesome, {
    form: {
        del: "fa-times"
    },
    nav: {
        del: "fa-times"
    }
});

It's clear that you can use different icons in both cases:

$.extend(true, $.jgrid.icons.fontAwesome, {
    form: {
        del: "fa-times"
    },
    nav: {
        del: "fa-thumbs-o-down"
    }
});

It's just very important to use deep extend ($.extend(true, $.jgrid.icons.fontAwesome, {...}) and not just $.extend($.jgrid.icons.fontAwesome, {...})) if you don't want to overwrite all properties of the corresponding objects. The usage of

$.extend($.jgrid.icons.fontAwesome, {
    form: {
        del: "fa-times"
    },
    nav: {
        del: "fa-times"
    }
});

for example will replace all settings for form and nav and will remove existing settings for other form and nav properties (nav.common, nav.add and so on).

You need just examine the structure of properties under $.jgrid.icons (see here). In the way you can either overwrite any from the existing icons of jQueryUI or fontAwesome part or to create one more set of icons by usage

$.extend(true, $.jgrid.icons, {
    mySet: $.extend(true, {
        form: {
            del: "fa-times"
        },
        nav: {
            del: "fa-times"
        }
    },
    $.jgrid.icons.fontAwesome)
});

The last example uses $.extend(true, {.../*modifications*/...}, $.jgrid.icons.fontAwesome) to generate object with the same properties as $.jgrid.icons.fontAwesome, but with small modification. The part $.extend(true, $.jgrid.icons, {mySet: ...}); saves the object as new property of $.jgrid.icons under the name mySet on the same level like jQueryUI or fontAwesome. So one can use now iconSet: "mySet" option in the same way like iconSet: "fontAwesome" or iconSet: "jQueryUI" available out of the box.

So you can see that the model of iconSet is very simple and relatively flexible. One have of cause a lot of jQuery UI classes over the icons (in outer HTML elements) and the classes will be applied. Nevertheless one can do much more things as in jqGrid 4.7.

4) I just want the WHOLE BUTTON, not just an icon was blue with the white text. Like this: http://getbootstrap.com/css/#buttons-options So I asked about the ability to add custom classes to the BUTTONS, not to the icons.

It's not so simple. At least you can't do this in the same way like by configuring of icons, because you want to change more as just an icon. I use Bootstrap only in minimal way, so I can't write the corresponding changes immediately. I would suggest you to use beforeShowForm callback for such modifications. In new version of jqGrid you can include

$("#grid").jqGrid({
    // any well-known option of jqGrid
    formEditing: {
        beforeShowForm: function ($form) {
            // add new button or modify the existing one
        }
    }
});

See the answer for an example of such modifications. An example could be something like the following

formEditing: {
    afterShowForm: function ($form) {
        $form.next(".EditTable")
            .find("#sData")
            .unbind("mouseenter mouseleave") // remove standard hovering
            .removeClass("ui-state-default")
            .addClass("btn btn-danger")
            .html("Some Text");
    }
}

and the results look like on the picture below button. Probably it's not exactly what you want, but in the way you can do the required changes.

smartcorestudio commented 9 years ago

:+1:

It's just very important to use deep extend

Thanks for making this clear, @OlegKi! I tried to do this without "true" parameter and I had to overwrite all the properties. It seemed that there should be more convenient way, so I asked you.

Probably it's not exactly what you want

The final result is exactly what I want, I just thought that maybe there is more simple way to do this. It could be useful for many users, not only those who use Bootstrap.

OlegKi commented 9 years ago

I plan to include more possibilities to configure jqGrid in the future. It will allow to change the style of buttons, the style of title and so on. It's just important to make the changes in clear steps. Scaleless icons had for me the highest priority. Last days I made changes which allows to load multiple locales at once and set locale: "ru" for one grid on the page and locale: "en-US" for another one.

I need now to create the documentation and fix the bugs. After I'll make some first pages I'll ask you for some tips in design the pages. Is it OK?

If you will find any bugs or unclear things, please let me known about there.

smartcorestudio commented 9 years ago

OK!