Обсуждение: A fix and a new functionnality for the colour patch

Hi,

The colour patch commited by Dave during pgCon2008 allows a user to add
a background color to the treeview for each specific registered server.

When a user deletes the colour in the server's property, pgAdmin will
use a black (#000000) background. The treeview node's text will be
unreadable. What the patch does is impliying white colour when the user
doesn't spécify a colour.

The new functionality adds a colour button that opens the standard
colour dialog. The user doesn't new to know the HTML colour coding.

Comments?

Regards.


--
Guillaume.
  http://www.postgresqlfr.org
  http://dalibo.com
Index: pgadmin/include/schema/pgServer.h
===================================================================
--- pgadmin/include/schema/pgServer.h    (revision 7373)
+++ pgadmin/include/schema/pgServer.h    (working copy)
@@ -110,7 +110,7 @@
     bool SetPassword(const wxString& newVal);
     wxDateTime GetUpSince() { return upSince; }
     void iSetUpSince(const wxDateTime &d) { upSince = d; }
-    void iSetColour(const wxString &s) { colour = wxColour(s); }
+    void iSetColour(const wxString &s) { colour = s.Length() == 0 ? wxColour(wxT("#ffffff")) : wxColour(s); }
     wxColour GetColour() { return colour; }

     bool HasPrivilege(const wxString &objTyp, const wxString &objName, const wxString &priv) { return
conn->HasPrivilege(objTyp,objName, priv); } 
Index: pgadmin/include/dlg/dlgServer.h
===================================================================
--- pgadmin/include/dlg/dlgServer.h    (revision 7373)
+++ pgadmin/include/dlg/dlgServer.h    (working copy)
@@ -42,6 +42,7 @@
     void OnChangeRestr(wxCommandEvent &ev);
     void OnChangeTryConnect(wxCommandEvent &ev);
     void OnPageSelect(wxNotebookEvent &event);
+    void OnChooseColor(wxCommandEvent &ev);

     DECLARE_EVENT_TABLE()
 };
Index: pgadmin/dlg/dlgServer.cpp
===================================================================
--- pgadmin/dlg/dlgServer.cpp    (revision 7373)
+++ pgadmin/dlg/dlgServer.cpp    (working copy)
@@ -11,6 +11,7 @@

 // wxWindows headers
 #include <wx/wx.h>
+#include <wx/colordlg.h>

 // App headers
 #include "pgAdmin3.h"
@@ -37,6 +38,7 @@
 #define txtPassword     CTRL_TEXT("txtPassword")
 #define txtDbRestriction CTRL_TEXT("txtDbRestriction")
 #define txtColour       CTRL_TEXT("txtColour")
+#define btnColor        CTRL_BUTTON("btnColor")


 BEGIN_EVENT_TABLE(dlgServer, dlgProperty)
@@ -53,6 +55,7 @@
     EVT_CHECKBOX(XRCID("chkRestore"),               dlgProperty::OnChange)
     EVT_CHECKBOX(XRCID("chkTryConnect"),            dlgServer::OnChangeTryConnect)
     EVT_TEXT(XRCID("txtColour"),                    dlgProperty::OnChange)
+    EVT_BUTTON(XRCID("btnColor"),                   dlgServer::OnChooseColor)
     EVT_BUTTON(wxID_OK,                             dlgServer::OnOK)
 END_EVENT_TABLE();

@@ -232,7 +235,7 @@
         chkStorePwd->SetValue(server->GetStorePwd());
         chkRestore->SetValue(server->GetRestore());
         txtDbRestriction->SetValue(server->GetDbRestriction());
-        txtColour->SetValue(server->GetColour().GetAsString(wxC2S_HTML_SYNTAX));
+        txtColour->SetValue(server->GetColour().GetAsString(wxC2S_HTML_SYNTAX));

         stPassword->Disable();
         txtPassword->Disable();
@@ -290,6 +293,16 @@
 }


+void dlgServer::OnChooseColor(wxCommandEvent &ev)
+{
+    wxColourDialog dlg( NULL );
+    if ( dlg.ShowModal() == wxID_OK )
+    {
+        txtColour->SetValue(dlg.GetColourData().GetColour().GetAsString(wxC2S_HTML_SYNTAX));
+    }
+}
+
+
 void dlgServer::CheckChange()
 {
     wxString name=GetName();
Index: pgadmin/ui/dlgServer.xrc
===================================================================
--- pgadmin/ui/dlgServer.xrc    (revision 7373)
+++ pgadmin/ui/dlgServer.xrc    (working copy)
@@ -122,6 +122,10 @@
             <pos>70,169d</pos>
             <size>75,-1d</size>
           </object>
+          <object class="wxButton" name="btnColor">
+            <label>Choose Color...</label>
+            <pos>150,169d</pos>
+          </object>
         </object>
         <selected>1</selected>
       </object>
@@ -144,4 +148,4 @@
     <size>218,225d</size>
     <style></style>
   </object>
-</resource>
\ No newline at end of file
+</resource>

On Sat, Jun 14, 2008 at 4:21 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> Hi,
>
> The colour patch commited by Dave during pgCon2008 allows a user to add a
> background color to the treeview for each specific registered server.
>
> When a user deletes the colour in the server's property, pgAdmin will use a
> black (#000000) background. The treeview node's text will be unreadable.
> What the patch does is impliying white colour when the user doesn't spécify
> a colour.

Hmm, thought I covered that cased. Thanks for catching.

> The new functionality adds a colour button that opens the standard colour
> dialog. The user doesn't new to know the HTML colour coding.

Is the colour dialog available on all platforms? I did look for one
when writing the original patch but couldn't find it. It's possible I
was hungover from the Jagermeister the night before though (EDB party
at pgCon :-) ).

> Comments?

You spelt colour wrong :-). Other than that (and the possible issue
above), looks good.

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

Dave Page a écrit :
> On Sat, Jun 14, 2008 at 4:21 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
>> Hi,
>>
>> The colour patch commited by Dave during pgCon2008 allows a user to add a
>> background color to the treeview for each specific registered server.
>>
>> When a user deletes the colour in the server's property, pgAdmin will use a
>> black (#000000) background. The treeview node's text will be unreadable.
>> What the patch does is impliying white colour when the user doesn't spécify
>> a colour.
>
> Hmm, thought I covered that cased. Thanks for catching.
>

At least, it didn't work on GTK+.

>> The new functionality adds a colour button that opens the standard colour
>> dialog. The user doesn't new to know the HTML colour coding.
>
> Is the colour dialog available on all platforms?

AFAIK, yes, it does.

According to the manual
(http://docs.wxwidgets.org/2.8.6/wx_commondialogsoverview.html), "Some
dialogs have both platform-dependent and platform-independent
implementations, so that if underlying windowing systems do not provide
the required functionality, the generic classes and functions can stand in."

> I did look for one
> when writing the original patch but couldn't find it. It's possible I
> was hungover from the Jagermeister the night before though (EDB party
> at pgCon :-) ).
>

hehe :)

>> Comments?
>
> You spelt colour wrong :-). Other than that (and the possible issue
> above), looks good.
>

Yeah, my first (unsent) mail used color. I changed all color with colour
because of the spelling in pgAdmin :)


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

Dave Page a écrit :
> On Sat, Jun 14, 2008 at 4:21 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
> [...]
>> Comments?
>
> You spelt colour wrong :-). Other than that (and the possible issue
> above), looks good.
>

Can I commit this patch?


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

On Mon, Jun 16, 2008 at 9:03 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> Dave Page a écrit :
>>
>> On Sat, Jun 14, 2008 at 4:21 PM, Guillaume Lelarge
>> <guillaume@lelarge.info> wrote:
>> [...]
>>>
>>> Comments?
>>
>> You spelt colour wrong :-). Other than that (and the possible issue
>> above), looks good.
>>
>
> Can I commit this patch?

Sure. As a general rule if there are no objections within a couple of
days I say go for it. You're experienced in the code now, and we can
always fix any problems that may arise.

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