[Phpmyadmin-devel] Dynamically increasing Create table dialog's size

Hi, please comment on my commit 554c1df307b47516dc0249fc75bdec2ad5c5e5a1. -- Marc Delisle http://infomarc.info

2011/10/9 Marc Delisle <marc@infomarc.info>:
Hi, please comment on my commit 554c1df307b47516dc0249fc75bdec2ad5c5e5a1.
I think size should be computed when AJAX response comes, and with some reasonable min sizes. Also, I don't like the idea of referencing frames (top.frame_content) when it can be easily avoided. Something like that should do: + height: $(window.document.body).innerHeight()-16, + width: $(window.document.body).innerWidth()-16, -- Regards, Piotr Przybylski

Piotr Przybylski a écrit :
2011/10/9 Marc Delisle <marc@infomarc.info>:
Hi, please comment on my commit 554c1df307b47516dc0249fc75bdec2ad5c5e5a1.
I think size should be computed when AJAX response comes, and with some reasonable min sizes. Also, I don't like the idea of referencing frames (top.frame_content) when it can be easily avoided. Something like that should do:
+ height: $(window.document.body).innerHeight()-16, + width: $(window.document.body).innerWidth()-16,
Thanks Piotr; however, using .innerHeight() returns too small a value. $(window.document.body).innerHeight() returns 443 $(top.frame_content).height() returns 791 -- Marc Delisle http://infomarc.info

2011/10/9 Marc Delisle <marc@infomarc.info>:
Piotr Przybylski a écrit :
2011/10/9 Marc Delisle <marc@infomarc.info>:
Hi, please comment on my commit 554c1df307b47516dc0249fc75bdec2ad5c5e5a1.
I think size should be computed when AJAX response comes, and with some reasonable min sizes. Also, I don't like the idea of referencing frames (top.frame_content) when it can be easily avoided. Something like that should do:
+ height: $(window.document.body).innerHeight()-16, + width: $(window.document.body).innerWidth()-16,
Thanks Piotr; however, using .innerHeight() returns too small a value.
$(window.document.body).innerHeight() returns 443
$(top.frame_content).height() returns 791
Oops, I forgot that <body>'s height gets special treatment. You are right, height() is the correct function to use. -- Piotr Przybylski

It still caused scrollbars to appear, so I tried to change Marc's approach - now window's scrollbars should be inactive, only dialog's content should have them [1]. While at it, I also upgraded jQuery UI to the newest stable version [2]. I hope I didn't break anything in the process, and that new dialog works better. [1] http://phpmyadmin.git.sourceforge.net/git/gitweb.cgi?p=phpmyadmin/phpmyadmin... [2] http://phpmyadmin.git.sourceforge.net/git/gitweb.cgi?p=phpmyadmin/phpmyadmin... -- Piotr Przybylski

On Mon, 2011-10-10 at 22:42 +0200, Piotr Przybylski wrote:
It still caused scrollbars to appear, so I tried to change Marc's approach - now window's scrollbars should be inactive, only dialog's content should have them [1]. While at it, I also upgraded jQuery UI to the newest stable version [2].
I'm not convinced that this is better. Try the following scenario: * Open a somewhat small browser window. * Click Create table. * Maximise the browser window. The dialog remains the same size, is padded with a lot of white space and there is no way to resize it, so the user has to close and reopen it. Same goes for making the browser window smaller after opening the dialog: you can't resize it, so you end up with two sets of scrollbars anyway.
I hope I didn't break anything in the process, and that new dialog works better.
[1] http://phpmyadmin.git.sourceforge.net/git/gitweb.cgi?p=phpmyadmin/phpmyadmin... [2] http://phpmyadmin.git.sourceforge.net/git/gitweb.cgi?p=phpmyadmin/phpmyadmin...
Also I'm not sure why you committed the following: --- a/js/functions.js +++ b/js/functions.js @@ -1433,6 +1433,8 @@ function PMA_ajaxRemoveMessage($this_msgbox) // due to a bug in qtip's implementation we can // only hide it without throwing JS errors. $this_msgbox.qtip('hide'); + } else { + $this_msgbox.remove(); } } } This change looks irrelevant to your work and besides the messages are destroyed at line 1388 via a callback, so this change may cause conflicts though I haven't verified this. Let's see what others have to say, but personally I'd like to see that changeset reverted. Rouslan

2011/10/11 Rouslan Placella <rouslan@placella.com>:
On Mon, 2011-10-10 at 22:42 +0200, Piotr Przybylski wrote:
It still caused scrollbars to appear, so I tried to change Marc's approach - now window's scrollbars should be inactive, only dialog's content should have them [1]. While at it, I also upgraded jQuery UI to the newest stable version [2].
I'm not convinced that this is better. Try the following scenario: * Open a somewhat small browser window. * Click Create table. * Maximise the browser window. The dialog remains the same size, is padded with a lot of white space and there is no way to resize it, so the user has to close and reopen it. Same goes for making the browser window smaller after opening the dialog: you can't resize it, so you end up with two sets of scrollbars anyway.
What browser are you using? There is a callback that should be reacting to window's resize.
I hope I didn't break anything in the process, and that new dialog works better.
[1] http://phpmyadmin.git.sourceforge.net/git/gitweb.cgi?p=phpmyadmin/phpmyadmin... [2] http://phpmyadmin.git.sourceforge.net/git/gitweb.cgi?p=phpmyadmin/phpmyadmin...
Also I'm not sure why you committed the following:
--- a/js/functions.js +++ b/js/functions.js @@ -1433,6 +1433,8 @@ function PMA_ajaxRemoveMessage($this_msgbox) // due to a bug in qtip's implementation we can // only hide it without throwing JS errors. $this_msgbox.qtip('hide'); + } else { + $this_msgbox.remove(); } } }
This change looks irrelevant to your work and besides the messages are destroyed at line 1388 via a callback, so this change may cause conflicts though I haven't verified this.
Unless I misunderstood something this should destroy message completely, and I needed this change because the way in which I am hiding page's content makes recent AJAX message to come back when I close create table dialog. It's in the same commit as dialog's change for easy rollback. -- Piotr Przybylski

On Tue, 2011-10-11 at 01:57 +0200, Piotr Przybylski wrote:
2011/10/11 Rouslan Placella <rouslan@placella.com>:
On Mon, 2011-10-10 at 22:42 +0200, Piotr Przybylski wrote:
It still caused scrollbars to appear, so I tried to change Marc's approach - now window's scrollbars should be inactive, only dialog's content should have them [1]. While at it, I also upgraded jQuery UI to the newest stable version [2].
I'm not convinced that this is better. Try the following scenario: * Open a somewhat small browser window. * Click Create table. * Maximise the browser window. The dialog remains the same size, is padded with a lot of white space and there is no way to resize it, so the user has to close and reopen it. Same goes for making the browser window smaller after opening the dialog: you can't resize it, so you end up with two sets of scrollbars anyway.
What browser are you using? There is a callback that should be reacting to window's resize.
So far I tried (under linux): Firefox 7, Opera 11 and Chromium 12. The issue is present in all of them. I also tried IE8, where it doesn't seem to work at all. Clicking the create table icon opens an unusable dialog. See attached screenshot.
I hope I didn't break anything in the process, and that new dialog works better.
[1] http://phpmyadmin.git.sourceforge.net/git/gitweb.cgi?p=phpmyadmin/phpmyadmin... [2] http://phpmyadmin.git.sourceforge.net/git/gitweb.cgi?p=phpmyadmin/phpmyadmin...
Also I'm not sure why you committed the following:
--- a/js/functions.js +++ b/js/functions.js @@ -1433,6 +1433,8 @@ function PMA_ajaxRemoveMessage($this_msgbox) // due to a bug in qtip's implementation we can // only hide it without throwing JS errors. $this_msgbox.qtip('hide'); + } else { + $this_msgbox.remove(); } } }
This change looks irrelevant to your work and besides the messages are destroyed at line 1388 via a callback, so this change may cause conflicts though I haven't verified this.
Unless I misunderstood something this should destroy message completely, and I needed this change because the way in which I am hiding page's content makes recent AJAX message to come back when I close create table dialog. It's in the same commit as dialog's change for easy rollback.
Sorry about that comment, I see now what that does. Rouslan

On Tue, 2011-10-11 at 10:26 +0100, Rouslan Placella wrote:
On Tue, 2011-10-11 at 01:57 +0200, Piotr Przybylski wrote:
2011/10/11 Rouslan Placella <rouslan@placella.com>:
On Mon, 2011-10-10 at 22:42 +0200, Piotr Przybylski wrote:
It still caused scrollbars to appear, so I tried to change Marc's approach - now window's scrollbars should be inactive, only dialog's content should have them [1]. While at it, I also upgraded jQuery UI to the newest stable version [2].
I'm not convinced that this is better. Try the following scenario: * Open a somewhat small browser window. * Click Create table. * Maximise the browser window. The dialog remains the same size, is padded with a lot of white space and there is no way to resize it, so the user has to close and reopen it. Same goes for making the browser window smaller after opening the dialog: you can't resize it, so you end up with two sets of scrollbars anyway.
What browser are you using? There is a callback that should be reacting to window's resize.
So far I tried (under linux): Firefox 7, Opera 11 and Chromium 12. The issue is present in all of them. I also tried IE8, where it doesn't seem to work at all. Clicking the create table icon opens an unusable dialog. See attached screenshot.
I see a commit that fixes the resize issue, so after pulling from master this issue is gone. The IE problem still applies though.
I hope I didn't break anything in the process, and that new dialog works better.
[1] http://phpmyadmin.git.sourceforge.net/git/gitweb.cgi?p=phpmyadmin/phpmyadmin... [2] http://phpmyadmin.git.sourceforge.net/git/gitweb.cgi?p=phpmyadmin/phpmyadmin...
Also I'm not sure why you committed the following:
--- a/js/functions.js +++ b/js/functions.js @@ -1433,6 +1433,8 @@ function PMA_ajaxRemoveMessage($this_msgbox) // due to a bug in qtip's implementation we can // only hide it without throwing JS errors. $this_msgbox.qtip('hide'); + } else { + $this_msgbox.remove(); } } }
This change looks irrelevant to your work and besides the messages are destroyed at line 1388 via a callback, so this change may cause conflicts though I haven't verified this.
Unless I misunderstood something this should destroy message completely, and I needed this change because the way in which I am hiding page's content makes recent AJAX message to come back when I close create table dialog. It's in the same commit as dialog's change for easy rollback.
Sorry about that comment, I see now what that does.
Rouslan
------------------------------------------------------------------------------ All the data continuously generated in your IT infrastructure contains a definitive record of customers, application performance, security threats, fraudulent activity and more. Splunk takes this data and makes sense of it. Business sense. IT sense. Common sense. http://p.sf.net/sfu/splunk-d2d-oct _______________________________________________ Phpmyadmin-devel mailing list Phpmyadmin-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel

2011/10/11 Rouslan Placella <rouslan@placella.com>:
On Tue, 2011-10-11 at 01:57 +0200, Piotr Przybylski wrote:
2011/10/11 Rouslan Placella <rouslan@placella.com>:
On Mon, 2011-10-10 at 22:42 +0200, Piotr Przybylski wrote:
It still caused scrollbars to appear, so I tried to change Marc's approach - now window's scrollbars should be inactive, only dialog's content should have them [1]. While at it, I also upgraded jQuery UI to the newest stable version [2].
I'm not convinced that this is better. Try the following scenario: * Open a somewhat small browser window. * Click Create table. * Maximise the browser window. The dialog remains the same size, is padded with a lot of white space and there is no way to resize it, so the user has to close and reopen it. Same goes for making the browser window smaller after opening the dialog: you can't resize it, so you end up with two sets of scrollbars anyway.
What browser are you using? There is a callback that should be reacting to window's resize.
So far I tried (under linux): Firefox 7, Opera 11 and Chromium 12. The issue is present in all of them. I also tried IE8, where it doesn't seem to work at all. Clicking the create table icon opens an unusable dialog. See attached screenshot.
... fail on my side, resize handler was broken. Fixed in master, I will take a look at IE when I get home.
Also I'm not sure why you committed the following:
--- a/js/functions.js +++ b/js/functions.js @@ -1433,6 +1433,8 @@ function PMA_ajaxRemoveMessage($this_msgbox) // due to a bug in qtip's implementation we can // only hide it without throwing JS errors. $this_msgbox.qtip('hide'); + } else { + $this_msgbox.remove(); } } }
This change looks irrelevant to your work and besides the messages are destroyed at line 1388 via a callback, so this change may cause conflicts though I haven't verified this.
Unless I misunderstood something this should destroy message completely, and I needed this change because the way in which I am hiding page's content makes recent AJAX message to come back when I close create table dialog. It's in the same commit as dialog's change for easy rollback.
Sorry about that comment, I see now what that does.
Better to ask than later wonder about some line's purpose ;) -- Piotr Przybylski

Le 2011-10-11 05:40, Piotr Przybylski a écrit :
2011/10/11 Rouslan Placella <rouslan@placella.com>:
On Tue, 2011-10-11 at 01:57 +0200, Piotr Przybylski wrote:
2011/10/11 Rouslan Placella <rouslan@placella.com>:
On Mon, 2011-10-10 at 22:42 +0200, Piotr Przybylski wrote:
It still caused scrollbars to appear, so I tried to change Marc's approach - now window's scrollbars should be inactive, only dialog's content should have them [1]. While at it, I also upgraded jQuery UI to the newest stable version [2].
I'm not convinced that this is better. Try the following scenario: * Open a somewhat small browser window. * Click Create table. * Maximise the browser window. The dialog remains the same size, is padded with a lot of white space and there is no way to resize it, so the user has to close and reopen it. Same goes for making the browser window smaller after opening the dialog: you can't resize it, so you end up with two sets of scrollbars anyway.
What browser are you using? There is a callback that should be reacting to window's resize.
So far I tried (under linux): Firefox 7, Opera 11 and Chromium 12. The issue is present in all of them. I also tried IE8, where it doesn't seem to work at all. Clicking the create table icon opens an unusable dialog. See attached screenshot.
... fail on my side, resize handler was broken. Fixed in master, I will take a look at IE when I get home.
Piotr, with IE 9 (Vista) I get a tiny create table window. -- Marc Delisle http://infomarc.info

On Tue, 2011-10-11 at 05:59 -0400, Marc Delisle wrote:
Le 2011-10-11 05:40, Piotr Przybylski a écrit :
2011/10/11 Rouslan Placella <rouslan@placella.com>:
On Tue, 2011-10-11 at 01:57 +0200, Piotr Przybylski wrote:
2011/10/11 Rouslan Placella <rouslan@placella.com>:
On Mon, 2011-10-10 at 22:42 +0200, Piotr Przybylski wrote:
It still caused scrollbars to appear, so I tried to change Marc's approach - now window's scrollbars should be inactive, only dialog's content should have them [1]. While at it, I also upgraded jQuery UI to the newest stable version [2].
I'm not convinced that this is better. Try the following scenario: * Open a somewhat small browser window. * Click Create table. * Maximise the browser window. The dialog remains the same size, is padded with a lot of white space and there is no way to resize it, so the user has to close and reopen it. Same goes for making the browser window smaller after opening the dialog: you can't resize it, so you end up with two sets of scrollbars anyway.
What browser are you using? There is a callback that should be reacting to window's resize.
So far I tried (under linux): Firefox 7, Opera 11 and Chromium 12. The issue is present in all of them. I also tried IE8, where it doesn't seem to work at all. Clicking the create table icon opens an unusable dialog. See attached screenshot.
... fail on my side, resize handler was broken. Fixed in master, I will take a look at IE when I get home.
Piotr, with IE 9 (Vista) I get a tiny create table window.
On demo server, I getting the below error in firebug console when clicking create table and the dialog never appears. attempt to run compile-and-go script on a cleared scope function(a){if(a.cache===v)a.cache=fal...ll;d&&b.parentNode&&d.removeChild(b); jquery...8335404 (line 154) Rouslan

Window size problems in IE are fixed, I forgot that IE < 9 doesn't have window.inner* properties. JS bug on demo server may be fixed, I'm just not sure because this error doesn't appear locally. It may be related to Firefox bug [1], I haven't looked into that yet [1] https://bugzilla.mozilla.org/show_bug.cgi?id=635548 -- Piotr Przybylski

Piotr Przybylski a écrit :
Window size problems in IE are fixed, I forgot that IE < 9 doesn't have window.inner* properties.
It works fine now, I tested in IE 8 at work. -- Marc Delisle http://infomarc.info

On Oct 11, 2011, at 4:17 PM, Piotr Przybylski <piotr.prz@gmail.com> wrote:
Window size problems in IE are fixed, I forgot that IE < 9 doesn't have window.inner* properties.
JS bug on demo server may be fixed, I'm just not sure because this error doesn't appear locally. It may be related to Firefox bug [1], I haven't looked into that yet
I had been getting a JavaScript error in IE which also caused the menu tabs to not collapse under "more" but that was fixed a day or two ago. IE seems to work right for me now.
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=635548
-- Piotr Przybylski
------------------------------------------------------------------------------ All the data continuously generated in your IT infrastructure contains a definitive record of customers, application performance, security threats, fraudulent activity and more. Splunk takes this data and makes sense of it. Business sense. IT sense. Common sense. http://p.sf.net/sfu/splunk-d2d-oct _______________________________________________ Phpmyadmin-devel mailing list Phpmyadmin-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel

On Wed, 2011-10-12 at 13:21 -0400, Isaac Bennetch wrote:
On Oct 11, 2011, at 4:17 PM, Piotr Przybylski <piotr.prz@gmail.com> wrote:
Window size problems in IE are fixed, I forgot that IE < 9 doesn't have window.inner* properties.
JS bug on demo server may be fixed, I'm just not sure because this error doesn't appear locally. It may be related to Firefox bug [1], I haven't looked into that yet
I had been getting a JavaScript error in IE which also caused the menu tabs to not collapse under "more" but that was fixed a day or two ago. IE seems to work right for me now.
Broken and fixed by me. I introduced that issue in the new implementation of the sprites. I had no idea that 'class' is a reserved word in IE'S javascript and can't be used for object attributes :S Rouslan
participants (4)
-
Isaac Bennetch
-
Marc Delisle
-
Piotr Przybylski
-
Rouslan Placella