Skip site navigation (1) Skip section navigation (2)

Re: Dialogs Review new patch

From: Guillaume Lelarge <guillaume(at)lelarge(dot)info>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: pgadmin-hackers(at)postgresql(dot)org
Subject: Re: Dialogs Review new patch
Date: 2008-08-22 14:06:07
Message-ID: 48AEC7CF.3040403@lelarge.info (view raw or flat)
Thread:
Lists: pgadmin-hackers
Dave Page a écrit :
> [...]
> On Mon, Aug 18, 2008 at 11:09 PM, Guillaume Lelarge
> <guillaume(at)lelarge(dot)info> wrote:
>>
>> My new patch fixes some issues. Some dialogs seem ready to commit:
> 
> Wow - they look excellent. Just some minor thoughts:
> 
> - dlgManageMacros - if the size is reduced to minimum, a horizontal
> scrollbar is show in the list control, due to the width of the name
> column.
> 

I don't have this issue. Perhaps do you mean dlgManageFavorites? or is
it on Mac OS X?

> - dlgTrigger, dlgSequence and dlgView are quite tall. The latter two
> are currently constrained by the height of the privs pane, but perhaps
> also by the comments textbox which certainly could be smaller. Not
> sure if it would be enough to get them to the standard size though
> (perhaps we need two or three standard sizes)?
> 

dlgTrigger is smaller. The four checkboxes are now in a 2x2
wxFlexGridSizer. If we want to make it smaller once again, we need to
replace the wxRadioBox with a wxComboBox.

dlgSequence will need another tab, so we can reduce its size.

dlgView is smaller too because of the resizeable Properties tab.

The biggest one is now dlgSequence.

> - The privileges panes doesn't size yet.
> 

It does now. It took me five hours to get something working. Glad it's
finished :)

> - Most dialogues seem to have an unused status bar at the bottom. I
> think we should either remove it universally, or make it work
> properly. What do you think?
> 

Make them work properly would be better. The status bar offers important
informations when a user deals with an unusual object (conversion for
example).

It doesn't work well on Mac?

>> I don't think I should play with:
>>  * dlgConnect.xrc
>>  * dlgSelectConnection.xrc
> 
> It may be useful to do them, if only so they size properly with
> non-standard fonts, or in other languages.
> 

Done.

>> The txtValue/chkValue problem is fixed. The wxListCtrl problem is fixed
>> too, but with a dirty hack. I don't really like what I did but it works.
>> If someone has a better idea, I welcome it.
> 
> I've seen a lot worse. I think a comment explaining the purpose of the
> function is in order though.

OK, will do.

> I think you should probably commit once
> you're at a convenient point, to minimise the risk of bitrot or future
> conflicts.
> 

Not sure if we really want this now. It's difficult to know what I
should commit. Obviously, the "remaining-to-be-fixed" should not :)

But can I commit the ctlSecurityPanel.cpp which takes care of the
Privileges tab without commiting dlgTable? dlgTable still needs a fix
but I don't know how the old dltTable will behave with the new
resizeable Privileges tab.

> Nice work :-)
> 

Thanks.

To do:
 * dlgFindReplace

Remains to be fixed:
 * dlgDatabase
 * dlgFunction
 * dlgLanguage
 * dlgTable
 * dlgType


New patch is available here:
 http://developer.pgadmin.org/~guillaume/dialogreview_20080822.patch.bz2


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

In response to

Responses

pgadmin-hackers by date

Next:From: Dave PageDate: 2008-08-22 14:15:52
Subject: Re: Dialogs Review new patch
Previous:From: svnDate: 2008-08-22 11:20:43
Subject: SVN Commit by dpage: r7419 - in trunk/pgadmin3: . docs/en_US pgadmin/ctl pgadmin/include/ctl

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group