Обсуждение: Dialogs Review new patch

Поиск
Список
Период
Сортировка

Dialogs Review new patch

От
Guillaume Lelarge
Дата:
Hi,

My new patch fixes some issues. Some dialogs seem ready to commit:
 * dlgAddFavourite.xrc
 * dlgAggregate.xrc
 * dlgCast.xrc
 * dlgCheck.xrc
 * dlgColumn.xrc
 * dlgConversion.xrc
 * dlgDomain.xrc
 * dlgForeignKey.xrc
 * dlgHbaConfig.xrc
 * dlgIndexConstraint.xrc
 * dlgIndex.xrc
 * dlgMainConfig.xrc
 * dlgManageFavourites.xrc
 * dlgManageMacros.xrc
 * dlgOperator.xrc
 * dlgPgpassConfig.xrc
 * dlgReassignDropOwned.xrc
 * dlgRule.xrc
 * dlgSchema.xrc
 * dlgSequence.xrc
 * dlgTablespace.xrc
 * dlgTextSearchConfiguration.xrc
 * dlgTextSearchDictionary.xrc
 * dlgTextSearchParser.xrc
 * dlgTextSearchTemplate.xrc
 * dlgTrigger.xrc
 * dlgView.xrc

I don't think I should play with:
 * dlgConnect.xrc
 * dlgSelectConnection.xrc

Still to fix:
 * dlgDatabase.xrc
 * dlgFunction.xrc
 * dlgLanguage.xrc (strange bug with Mac OS X)
 * dlgTable.xrc (Columns Tab)

To do:
 * dlgDirectDbg.xrc
 * dlgEditGridOptions.xrc
 * dlgFindReplace.xrc
 * dlgGroup.xrc
 * dlgJob.xrc
 * dlgPackage.xrc (EDB stuff, don't know how to check this)
 * dlgSynonym.xrc (EDB stuff, don't know how to check this)
 * dlgRepClusterUpgrade.xrc
 * dlgRepCluster.xrc
 * dlgRepListen.xrc
 * dlgRepNode.xrc
 * dlgRepPath.xrc
 * dlgRepSequence.xrc
 * dlgRepSetMerge.xrc
 * dlgRepSetMove.xrc
 * dlgRepSet.xrc
 * dlgRepSubscription.xrc
 * dlgRepTable.xrc
 * dlgRole.xrc
 * dlgSchedule.xrc
 * dlgServer.xrc
 * dlgStep.xrc
 * dlgType.xrc
 * dlgUser.xrc

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.

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

Regards.


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

Re: Dialogs Review new patch

От
"Dave Page"
Дата:
Hi

On Mon, Aug 18, 2008 at 11:09 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> Hi,
>
> 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.

- 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)?

- The privileges panes doesn't size yet.

- 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?

> 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.

> 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. I think you should probably commit once
you're at a convenient point, to minimise the risk of bitrot or future
conflicts.

Nice work :-)


--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

Re: Dialogs Review new patch

От
Guillaume Lelarge
Дата:
Dave Page a écrit :
> [...]
> On Mon, Aug 18, 2008 at 11:09 PM, Guillaume Lelarge
> <guillaume@lelarge.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

Re: Dialogs Review new patch

От
"Dave Page"
Дата:
On Fri, Aug 22, 2008 at 3:06 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> Dave Page a écrit :
>> - 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?

Possibly - and no, Windows.

>> - 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.

OK, cool. I guess we could just move the rarely-used sequence options
to a second tab.

>> - The privileges panes doesn't size yet.
>>
>
> It does now. It took me five hours to get something working. Glad it's
> finished :)

Yikes. I think I owe you more than a few beers at PGDay this year :-)

>> - 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?

Never seems to display anything on Windows.

> 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 :)

Well, I'll leave it to you to decide what you think is most appropriate.

> 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.

Probably fine - it just takes a minute to find out :-)

Don't worry if you temporarily break the odd dialog. SVN code isn't
supposed to be stable, and I think that's probably a small price to
pay to avoid bitrot.

/D

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

Re: Dialogs Review new patch

От
Guillaume Lelarge
Дата:
Dave Page a écrit :
> On Fri, Aug 22, 2008 at 3:06 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
>> Dave Page a écrit :
>>> - 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?
>
> Possibly - and no, Windows.
>

OK, I see what you mean.

>>> - 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.
>
> OK, cool. I guess we could just move the rarely-used sequence options
> to a second tab.
>

OK, fixed.

>>> - The privileges panes doesn't size yet.
>>>
>> It does now. It took me five hours to get something working. Glad it's
>> finished :)
>
> Yikes. I think I owe you more than a few beers at PGDay this year :-)
>

:)

I'm not sure I'll go to pgDay this year.

>>> - 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?
>
> Never seems to display anything on Windows.
>

Fixed. The status bar is really oddly used.

>> 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 :)
>
> Well, I'll leave it to you to decide what you think is most appropriate.
>

I needed to fix the last Privileges issue (wxListCtrl issue on Mac OS
X). Now that it is fixed, I'll commit a part of my patch.

>> 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.
>
> Probably fine - it just takes a minute to find out :-)
>
> Don't worry if you temporarily break the odd dialog. SVN code isn't
> supposed to be stable, and I think that's probably a small price to
> pay to avoid bitrot.
>

I'll only commit some pieces of my patch. Here are the dialogs that
won't be commited:

 * dlgFunction (status bar, OK and cancel buttons don't display
   correctly on Mac)
 * dlgLanguage (strange error about wxChoice::GetString on Mac...
   strange because there's no wxChoice widget on this dialog)
 * dlgTable (wxListCtrl issue on Columns tab, on Mac)
 * dlgType (on Mac and Windows)
 * dlgRole (no chkValue on Variables tab, on Linux)
 * dlgSchedule (Days and Times tabs, on all platforms)

New patch is available on:
 http://developer.pgadmin.org/~guillaume/dialogreview_20080825.patch.bz2

It'll be commited tomorrow afternoon (if no one objects).


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

Re: Dialogs Review new patch

От
Guillaume Lelarge
Дата:
Guillaume Lelarge a écrit :
> [...]
> I'll only commit some pieces of my patch. Here are the dialogs that
> won't be commited:
>
>  * dlgFunction (status bar, OK and cancel buttons don't display
>    correctly on Mac)
>  * dlgLanguage (strange error about wxChoice::GetString on Mac...
>    strange because there's no wxChoice widget on this dialog)
>  * dlgTable (wxListCtrl issue on Columns tab, on Mac)
>  * dlgType (on Mac and Windows)
>  * dlgRole (no chkValue on Variables tab, on Linux)
>  * dlgSchedule (Days and Times tabs, on all platforms)
>
> New patch is available on:
>  http://developer.pgadmin.org/~guillaume/dialogreview_20080825.patch.bz2
>
> It'll be commited tomorrow afternoon (if no one objects).
>

Done.


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

Re: Dialogs Review new patch

От
"Dave Page"
Дата:
On Tue, Aug 26, 2008 at 3:22 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> Guillaume Lelarge a écrit :

>> New patch is available on:
>>  http://developer.pgadmin.org/~guillaume/dialogreview_20080825.patch.bz2
>>
>> It'll be commited tomorrow afternoon (if no one objects).
>>
>
> Done.

:-)

--
Dave Page
EnterpriseDB UK:   http://www.enterprisedb.com