Thread: Follow-up on "Query tool layout issues"

Follow-up on "Query tool layout issues"

From
Guillaume Lelarge
Date:
Hi,

Dave found some issues on the layout of the query tool. They are on a
few mails, so I thought I'll answer to all of them in a new thread.

Here is the list of issues, and my answers:

> I just went to add a server in a pgAdmin build from SVN trunk and
> noticed that the new colour selector doesn't work so well on Mac.

This is an issue with the wxColourPickerCtrl widget I used. So, after
some discussions, I went on to code my own colour picker widget. You'll
find it in the 1_ctlColourPicker.patch file. It adds the new widget and
replace every wxColourPickerCtrl with a ctlColourPicker.

Special thanks to Ashesh who gave me the right solution to an issue. I
was ready to stop working on this.

> The connection drop-down has become rather ugly (the button doesn't
> line up vertically), and requires scrolling even for a tiny list.

Not yet worked on it. I also have the issue on Linux and Mac OS X.

> The left-hand margin is the same colour as the text area now. Can we
> make it gray again please?

Already send a patch for this. I updated it to work with the new
ctlColourPicker widget. See 2_margin.patch.

> The buttons to the right of the recent queries combo are not lined up with it.

Already fixed (revision 8199, http://code.pgadmin.org/trac/changeset/8199).

> The Query tool looks good now, but I get a crash if I try to open the
> Options dialogue:

I suppose this is fixed since your xrcDialogs.cpp commit?

> - Only add a query to the list is it executes without an error,
> otherwise it will get spammed with non-functional queries and typos.
> - You can probably remove the 'Current' from 'Delete Current' and save
> some space. I think current is implied by the fact that the other says
> 'all'.

See 3_miscquery.patch. It didn't need an update since last time.

> BTW, on OSX, the tabset overflows the width of the dialogue now. Can
> we widen the dialogue, and perhaps move all the colour options onto
> one tab?

I need to add 50% of the size of the dialog to get something good
enough. You'll find another patch for this, but I'll need to do better.
The dialog is usable with this patch, but it's not pretty. I'll work
more on this by adding sizers, but I should probably work on more
important items right now (exclusion constraint and other 9.0 new stuff).

To test my code, don't forget to update xrcDialogs.cpp. I don't do it to
keep low the size of my patches.

All the patches were checked on Linux, Mac OS X (obviously), and Windows.


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com

Attachment

Re: Follow-up on "Query tool layout issues"

From
Dave Page
Date:
On Sat, Mar 6, 2010 at 5:22 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> Hi,
>
> Dave found some issues on the layout of the query tool. They are on a
> few mails, so I thought I'll answer to all of them in a new thread.
>
> Here is the list of issues, and my answers:
>
>> I just went to add a server in a pgAdmin build from SVN trunk and
>> noticed that the new colour selector doesn't work so well on Mac.
>
> This is an issue with the wxColourPickerCtrl widget I used. So, after
> some discussions, I went on to code my own colour picker widget. You'll
> find it in the 1_ctlColourPicker.patch file. It adds the new widget and
> replace every wxColourPickerCtrl with a ctlColourPicker.

OK, sounds good.

>> The connection drop-down has become rather ugly (the button doesn't
>> line up vertically), and requires scrolling even for a tiny list.
>
> Not yet worked on it. I also have the issue on Linux and Mac OS X.

OK.

>> The left-hand margin is the same colour as the text area now. Can we
>> make it gray again please?
>
> Already send a patch for this. I updated it to work with the new
> ctlColourPicker widget. See 2_margin.patch.

Yup.

>> The buttons to the right of the recent queries combo are not lined up with it.
>
> Already fixed (revision 8199, http://code.pgadmin.org/trac/changeset/8199).

:-)

>> The Query tool looks good now, but I get a crash if I try to open the
>> Options dialogue:
>
> I suppose this is fixed since your xrcDialogs.cpp commit?

Yes.

>> - Only add a query to the list is it executes without an error,
>> otherwise it will get spammed with non-functional queries and typos.
>> - You can probably remove the 'Current' from 'Delete Current' and save
>> some space. I think current is implied by the fact that the other says
>> 'all'.
>
> See 3_miscquery.patch. It didn't need an update since last time.

Yup., that was working fine.

>> BTW, on OSX, the tabset overflows the width of the dialogue now. Can
>> we widen the dialogue, and perhaps move all the colour options onto
>> one tab?
>
> I need to add 50% of the size of the dialog to get something good
> enough. You'll find another patch for this, but I'll need to do better.
> The dialog is usable with this patch, but it's not pretty. I'll work
> more on this by adding sizers, but I should probably work on more
> important items right now (exclusion constraint and other 9.0 new stuff).

I wonder if the time has come to redesign it to use a listbox full of
checkboxes/strings like some other programs do. Or replace the tabs
with a treeview-like page browser. Or a combination of the two, like
Visual Studio (iirc).

> To test my code, don't forget to update xrcDialogs.cpp. I don't do it to
> keep low the size of my patches.

I'm sure it's fine - please go ahead and commit if you're happy. I
don't have too many free cycles right now, and don't want to delay
your work.

Thanks!

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com
PG East Conference: http://www.enterprisedb.com/community/nav-pg-east-2010.do

Re: Follow-up on "Query tool layout issues"

From
Guillaume Lelarge
Date:
Le 08/03/2010 11:24, Dave Page a écrit :
> On Sat, Mar 6, 2010 at 5:22 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
> [...]
>>> BTW, on OSX, the tabset overflows the width of the dialogue now. Can
>>> we widen the dialogue, and perhaps move all the colour options onto
>>> one tab?
>>
>> I need to add 50% of the size of the dialog to get something good
>> enough. You'll find another patch for this, but I'll need to do better.
>> The dialog is usable with this patch, but it's not pretty. I'll work
>> more on this by adding sizers, but I should probably work on more
>> important items right now (exclusion constraint and other 9.0 new stuff).
>
> I wonder if the time has come to redesign it to use a listbox full of
> checkboxes/strings like some other programs do. Or replace the tabs
> with a treeview-like page browser. Or a combination of the two, like
> Visual Studio (iirc).
>

I'll try to do something based on that. My first idea was something like
wxPropertyGrid
(http://wxpropgrid.sourceforge.net/cgi-bin/index?page=about), I suppose
also this is what you wanted to say with the VisualStudio stuff.

>> To test my code, don't forget to update xrcDialogs.cpp. I don't do it to
>> keep low the size of my patches.
>
> I'm sure it's fine - please go ahead and commit if you're happy. I
> don't have too many free cycles right now, and don't want to delay
> your work.
>

OK, commit done. I did one commit per patch because I find it easier to
understand the different steps.


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com

Re: Follow-up on "Query tool layout issues"

From
Dave Page
Date:
On Mon, Mar 8, 2010 at 6:04 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:

> I'll try to do something based on that. My first idea was something like
> wxPropertyGrid
> (http://wxpropgrid.sourceforge.net/cgi-bin/index?page=about), I suppose
> also this is what you wanted to say with the VisualStudio stuff.

Yeah, thats the sort of thing. Shame it's another external library
though :-(. Think I'd prefer to avoid that.

> OK, commit done. I did one commit per patch because I find it easier to
> understand the different steps.

Absolutely the correct approach. Never mix patches in one commit. :-)

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com
PG East Conference: http://www.enterprisedb.com/community/nav-pg-east-2010.do

Re: Follow-up on "Query tool layout issues"

From
Guillaume Lelarge
Date:
Le 08/03/2010 21:05, Dave Page a écrit :
> On Mon, Mar 8, 2010 at 6:04 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
>
>> I'll try to do something based on that. My first idea was something like
>> wxPropertyGrid
>> (http://wxpropgrid.sourceforge.net/cgi-bin/index?page=about), I suppose
>> also this is what you wanted to say with the VisualStudio stuff.
>
> Yeah, thats the sort of thing. Shame it's another external library
> though :-(. Think I'd prefer to avoid that.
>

So do I.

There's another/simpler way to do it. We just need to change the
wxNotebook's style to wxNB_LEFT (one line to add), and we have something
fine. See the screenshot attached (but keep in mind I can't check right
now on Windows and Mac OS X).

Yet another way would be to replace the wxNotebook with a wxAUINotebook.

In fact, my proposal would be to:
 * use wxNB_LEFT for 1.12 ;
 * think about replacing it with something else for 1.14.

>> OK, commit done. I did one commit per patch because I find it easier to
>> understand the different steps.
>
> Absolutely the correct approach. Never mix patches in one commit. :-)
>

Sure. But it was a lot of commits at the same time.


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com

Attachment

Re: Follow-up on "Query tool layout issues"

From
Dave Page
Date:
On Mon, Mar 8, 2010 at 9:48 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:

> So do I.
>
> There's another/simpler way to do it. We just need to change the
> wxNotebook's style to wxNB_LEFT (one line to add), and we have something
> fine. See the screenshot attached (but keep in mind I can't check right
> now on Windows and Mac OS X).

Hmm, not sure if wxNB_LEFT will work on a Mac. Worth a try though.

> Yet another way would be to replace the wxNotebook with a wxAUINotebook.
>
> In fact, my proposal would be to:
>  * use wxNB_LEFT for 1.12 ;
>  * think about replacing it with something else for 1.14.

Sounds good to me.

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com
PG East Conference: http://www.enterprisedb.com/community/nav-pg-east-2010.do

Re: Follow-up on "Query tool layout issues"

From
Guillaume Lelarge
Date:
Le 09/03/2010 09:44, Dave Page a écrit :
> On Mon, Mar 8, 2010 at 9:48 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
>
>> So do I.
>>
>> There's another/simpler way to do it. We just need to change the
>> wxNotebook's style to wxNB_LEFT (one line to add), and we have something
>> fine. See the screenshot attached (but keep in mind I can't check right
>> now on Windows and Mac OS X).
>
> Hmm, not sure if wxNB_LEFT will work on a Mac. Worth a try though.
>

It works, but it's not pretty at all.

>> Yet another way would be to replace the wxNotebook with a wxAUINotebook.
>>
>> In fact, my proposal would be to:
>>  * use wxNB_LEFT for 1.12 ;
>>  * think about replacing it with something else for 1.14.
>
> Sounds good to me.
>


--
Guillaume.
 http://www.postgresqlfr.org
 http://dalibo.com