Re: extending functionality strategy

["Dave Page" <dpage(at)pgadmin(dot)org>]

On Mon, Oct 20, 2008 at 11:59 PM, Gevik Babakhani <pgdev(at)xs4all(dot)nl> wrote:
> Hereby a v1.0 patch implementing basic ordering for a wxListCtrl:
>
> - The patch is developed using MSVC 2008.
> - 2 new source files ctl\ListItemArranger.cpp and include\ListItemArranger.h
> are added.
> - 4 new image file are added in include\images (wxListItemArranger*.xpm)
> - For demo the dlgType.xrc and dlgType.cpp are modified.
> - 2 mk files modified manually.
> - After patching run embed-xrc
> - For compiling with MSVC the two source files must be added manually to ctl
> and include filters inside VS2005/2008.
> - For testing: Goto "add new composite type" then the definition tab.
>
> Please let me know what you think.

Hi Gevik,

Sorry for the delay in looking at this. A few (minor comments)

- The buttons should not become active when viewing an existing type.

- The background of the button area needs to be transparent. See the
attached screenshot and you'll see why.

- Please follow existing naming conventions - eg. use
ctlListItemArranger, per the other custom controls.

- Please retain the comments and grouping on the includes - it helps
prevent people mistakenly re-ordering things they shouldn't - eg.

// Main app header
#include "pgAdmin3.h"

// wxWindows headers
#include <wx/wx.h>
#include <wx/listctrl.h>

// App headers
#include "ctl/ListItemArranger.h"

They should almost always be in that order (one exception being
wx/regex.h which seems to need to be last usually). I appreciate that
we have some inonsistencies at the moment which do need improvement...

Thanks, Dave.

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

Attachment: ss.png
Description: PNG image