openpsa / jsgrid

Fork of last jqGrid version before license change
http://openpsa.github.io/jsgrid/
Other
28 stars 12 forks source link

Rendering problems with confirmation dialog #85

Closed flack closed 9 years ago

flack commented 9 years ago

This is a follow-up to #79

Currently, the delete confirmation dialog looks like this:

bildschirmfoto 2015-01-26 um 20 41 03

At least three things should be fixed here:

bouks commented 9 years ago

For me the popup is always on the top left corner of the grid. The problem of centering to the grid is if the grid is smaller and near the window border (for example with form edit).

flack commented 9 years ago

Could be that some of the problems are related to bootstrap's CSS which is used on the demo site. But we should still try to make it look good by default with Bootstrap, I hear it's quite popular these days :-)

For centering, I think grid size is not a problem. The popup is modal, so it doesn't matter if the dialog covers all of it. As a rule, I would say that the center of the dialog should be at the center of the grid. If this causes any part of the dialog to be outside of the current viewport, the calculation should be adjusted accordingly. For example:

...and so on. It would be nice if we could use position from jquery UI, because it supports the above out of the box, but I guess we don't want to increase our filesize (it is 6 KB minified and can be used standalone).

flack commented 9 years ago

P.S: A simpler solution might be to use position: fixed and always display it at the center of the screen. But this could theoretically be far away from the grid

bouks commented 9 years ago

I think we should try to make it look good by default with a standard css. Bootstrap is popular but there are other popular css grids (960, foundation...) and some people just don't use these grids. Bootstrap is mainly used for front-office ("designed" website), rarely for back-office that is the main environment use of the grid i think.

flack commented 9 years ago

Sure, I'm not saying we should make it look good only in boostrap and bad everywhere else. But jqGrid's default CSS should also look good when it is used with bootstrap (especially on our demo site)

flack commented 9 years ago

I've debugged it a little, and it seems the problem comes from here:

https://github.com/openpsa/grid.js/blob/master/js/grid.common.js#L116

p.left, p.top and p.overlay are all undefined for some reason, so $.jgrid.findPos(posSelector); (which would set the popup position to the top left of the grid) is never called, and instead it sets undefinedpx as top and undefined as left

flack commented 9 years ago

OK, I've figured it out now. The main problem came from a merge (funnily enough it was in the same line where @bouks already fixed another problem: https://github.com/openpsa/grid.js/commit/23426c43d98076f7136144ba787f4f589402f896#diff-382b3649283911e1a5c64f3ec00e4968R1595). This fixes the incorrect position and the empty popup header. The padding for the buttons came from an error in the less conversion, which is now also fixed ==> closing