Re: Enabling SQL text field in the SQL tab of object dialog - Mailing list pgadmin-hackers

Dave Page a écrit :
> On Tue, Jul 1, 2008 at 11:52 AM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
>
>> I'm at the PostgreSQL booth, for the RMLL in Mont-de-Marsan
>> (http://rmll.info). I'm trying the wifi system :)
>
> :-)
>
>> Here is the patch that fixes your issue on Win32. Works great on Win32 and
>> Linux... I'm not sure for Mac OS X though.
>
> Hmm, this still isn't looking so good I'm afraid:
>

:-/

> - If I open the properties dialogue on a database, click the SQL tab
> and then uncheck Read only, having changed nothing, I get asked if I
> want to cancel my edit. I believe this is because in the edit case,
> GetSQL() will return nothing, whilst the textbox may contain '--
> nothing to change'
>

Fixed.

> - The sizing is still wrong on Mac. Worse, the text boxes no longer
> resize on the resizeable dialogues (dlgFunction, dlgPackage,
> dlgTrigger and friends).
>
> I would suggest  avoiding trying to calculate the dimensions in code -

I like this idea.

> in my experience this almost never works well. Instead, I would
> suggest laying the two textboxes and the checkbox in a sizer and
> letting that do the work. Not only should that be easier to code (once
> you fully understand sizers!), but it should work properly on all
> platforms, and be resize-friendly.
>

OK, it wasn't easy to do. But it now works. I don't specify any size and
the rendering looks great... on Linux. On Windows, it doesn't. I spent
part of the last two days to try to understand the Windows behaviour but
I failed. Do you have any idea on the Windows behavior? is there
something Windows specific when we use the flex grid sizer?

> The same should probably be done for other standard tabs (eg,
> privileges, gucs) when you update all the XRC files as you discussed
> previously.
>
> Sorry, I know this isn't what you wanted to hear - but we'll get there :-)
>

Oh no problem with me on this. I'm just worried on the number of bad
patches.

The patch attached fixed some issues:
  * The "Are you sure..." message is displayed only when "Read Only" is
    checked.
  * No button is the default on the "Are you sure" message (which has a
    weird behaviour... hitting Esc will validate the dialog and the user
    will lose his changes... I'm not sure we should keep this).
  * the first issue you had.
  * Sizing problems fixed on Linux with a wxFlexGridSizer.

Remaining issues:
  * Objects' size on Windows (and Max OS X ?).


--
Guillaume.
  http://www.postgresqlfr.org
  http://dalibo.com
Index: pgadmin/include/dlg/dlgProperty.h
===================================================================
--- pgadmin/include/dlg/dlgProperty.h    (revision 7390)
+++ pgadmin/include/dlg/dlgProperty.h    (working copy)
@@ -60,6 +60,8 @@
     void EnableOK(bool enable);
     virtual bool IsUpToDate() { return true; };
     void ShowObject();
+
+    void FillSQLTextfield();

     void CheckValid(bool &enable, const bool condition, const wxString &msg);
     static dlgProperty *CreateDlg(frmMain *frame, pgObject *node, bool asNew, pgaFactory *factory=0);
@@ -86,6 +88,7 @@
     void OnChange(wxCommandEvent &ev);
     void OnChangeOwner(wxCommandEvent &ev);
     void OnChangeStc(wxStyledTextEvent& event);
+    void OnChangeReadOnly(wxCommandEvent& event);

 protected:
     void AddUsers(ctlComboBoxFix *cb1, ctlComboBoxFix *cb2=0);
@@ -97,7 +100,7 @@
     pgDatabase *database;

     frmMain *mainForm;
-    ctlSQLBox *sqlPane;
+    wxPanel *sqlPane;

     wxTextValidator numericValidator;

@@ -105,6 +108,10 @@
     wxTextCtrl *txtName, *txtOid, *txtComment;
     ctlComboBox *cbOwner;
     ctlComboBox *cbClusterSet;
+    wxCheckBox *chkReadOnly;
+    ctlSQLBox *sqlTextField1;
+    ctlSQLBox *sqlTextField2;
+    bool enableSQL2;

     int width, height;
     wxTreeItemId item, owneritem;
Index: pgadmin/dlg/dlgProperty.cpp
===================================================================
--- pgadmin/dlg/dlgProperty.cpp    (revision 7390)
+++ pgadmin/dlg/dlgProperty.cpp    (working copy)
@@ -59,8 +59,6 @@
 #include "schema/pgUser.h"


-
-
 class replClientData : public wxClientData
 {
 public:
@@ -72,6 +70,9 @@
 };


+#define CTRLID_CHKSQLTEXTFIELD 1000
+
+
 BEGIN_EVENT_TABLE(dlgProperty, DialogWithHelp)
     EVT_NOTEBOOK_PAGE_CHANGED(XRCID("nbNotebook"),  dlgProperty::OnPageSelect)

@@ -80,6 +81,8 @@
     EVT_COMBOBOX(XRCID("cbOwner"),                  dlgProperty::OnChange)
     EVT_TEXT(XRCID("txtComment"),                   dlgProperty::OnChange)

+    EVT_CHECKBOX(CTRLID_CHKSQLTEXTFIELD,            dlgProperty::OnChangeReadOnly)
+
     EVT_BUTTON(wxID_HELP,                           dlgProperty::OnHelp)
     EVT_BUTTON(wxID_OK,                             dlgProperty::OnOK)
     EVT_BUTTON(wxID_APPLY,                          dlgProperty::OnApply)
@@ -90,6 +93,8 @@
 {
     readOnly=false;
     sqlPane=0;
+    sqlTextField1=0;
+    sqlTextField2=0;
     processing=false;
     mainForm=frame;
     database=0;
@@ -117,6 +122,11 @@
     txtComment = CTRL_TEXT("txtComment");
     cbOwner = CTRL_COMBOBOX2("cbOwner");
     cbClusterSet = CTRL_COMBOBOX2("cbClusterSet");
+
+    wxString db = wxT("Database");
+    wxString ts = wxT("Tablespace");
+    enableSQL2 = db.Cmp(factory->GetTypeName()) == 0
+        || ts.Cmp(factory->GetTypeName()) == 0;

     wxNotebookPage *page=nbNotebook->GetPage(0);
     wxASSERT(page != NULL);
@@ -311,8 +321,41 @@

 void dlgProperty::CreateAdditionalPages()
 {
-    sqlPane = new ctlSQLBox(nbNotebook, CTL_PROPSQL, wxDefaultPosition, wxDefaultSize, wxTE_MULTILINE |
wxSUNKEN_BORDER| wxTE_READONLY | wxTE_RICH2); 
+    // create a panel
+    sqlPane = new wxPanel(nbNotebook);
+
+    // add panel to the notebook
     nbNotebook->AddPage(sqlPane, wxT("SQL"));
+
+    // create a flex grid sizer
+    wxFlexGridSizer *fgsizer = new wxFlexGridSizer(1, 5, 5);
+
+    // add checkbox to the panel
+    chkReadOnly = new wxCheckBox(sqlPane, CTRLID_CHKSQLTEXTFIELD, wxT("Read only"));
+    chkReadOnly->SetValue(true);
+    fgsizer->Add(chkReadOnly, 1, wxALL | wxALIGN_LEFT, 5);
+
+    // add the first text entry box
+    sqlTextField1 = new ctlSQLBox(sqlPane, CTL_PROPSQL,
+        wxDefaultPosition, wxDefaultSize,
+        wxTE_MULTILINE | wxSUNKEN_BORDER | wxTE_RICH2);
+    fgsizer->Add(sqlTextField1, 1, wxALL | wxEXPAND, 5);
+
+    // add the second text entry box (if needed)
+    if (enableSQL2)
+    {
+        sqlTextField2 = new ctlSQLBox(sqlPane, CTL_PROPSQL,
+            wxDefaultPosition, wxDefaultSize,
+            wxTE_MULTILINE | wxSUNKEN_BORDER | wxTE_RICH2);
+        fgsizer->Add(sqlTextField2, 1, wxALL | wxEXPAND, 5);
+    }
+
+    fgsizer->AddGrowableCol(0);
+    fgsizer->AddGrowableRow(1);
+    fgsizer->AddGrowableRow(2);
+
+    sqlPane->SetAutoLayout(true);
+    sqlPane->SetSizer(fgsizer);
 }


@@ -506,6 +549,85 @@
 }


+void dlgProperty::OnChangeReadOnly(wxCommandEvent &ev)
+{
+    size_t pos;
+    bool showmessage;
+
+    showmessage = chkReadOnly->GetValue()
+        && ! (!enableSQL2 && GetSql().Length() == 0 && sqlTextField1->GetText().Cmp(_("-- nothing to change")) == 0)
+        && ! (!enableSQL2 && GetSql().Length() == 0 && sqlTextField1->GetText().Cmp(_("-- definition incomplete")) ==
0)
+        && ! (enableSQL2 && GetSql().Length() == 0 && GetSql2().Length() == 0 && sqlTextField1->GetText().Cmp(_("--
nothingto change")) == 0 && sqlTextField2->GetText().Length() == 0) 
+        && ! (enableSQL2 && GetSql().Length() == 0 && GetSql2().Length() == 0 && sqlTextField1->GetText().Cmp(_("--
definitionincomplete")) == 0 && sqlTextField2->GetText().Length() == 0) 
+        && (sqlTextField1->GetText().Cmp(GetSql()) != 0 || (enableSQL2 && sqlTextField2->GetText().Cmp(GetSql2()) !=
0));
+
+    if (showmessage)
+    {
+        if (wxMessageBox(_("Are you sure you wish to cancel your edit?"), _("SQL editor"), wxYES_NO|wxNO_DEFAULT) ==
wxNO)
+        {
+            chkReadOnly->SetValue(false);
+            return;
+        }
+    }
+
+    sqlTextField1->SetReadOnly(chkReadOnly->GetValue());
+    if (enableSQL2)
+    {
+        sqlTextField2->SetReadOnly(chkReadOnly->GetValue());
+    }
+    for (pos = 0; pos < nbNotebook->GetPageCount() - 1; pos++)
+    {
+        nbNotebook->GetPage(pos)->Enable(chkReadOnly->GetValue());
+    }
+
+    if (chkReadOnly->GetValue())
+    {
+        FillSQLTextfield();
+    }
+}
+
+
+void dlgProperty::FillSQLTextfield()
+{
+    // create a function because this is a duplicated code
+    sqlTextField1->SetReadOnly(false);
+    if (enableSQL2)
+    {
+        sqlTextField2->SetReadOnly(false);
+    }
+    if (btnOK->IsEnabled())
+    {
+        wxString tmp;
+        if (cbClusterSet && cbClusterSet->GetSelection() > 0)
+        {
+            replClientData *data=(replClientData*)cbClusterSet->GetClientData(cbClusterSet->GetSelection());
+            tmp.Printf(_("-- Execute replicated using cluster \"%s\", set %ld\n"), data->cluster.c_str(),
data->setId);
+        }
+        sqlTextField1->SetText(tmp + GetSql());
+        if (enableSQL2)
+        {
+            sqlTextField2->SetText(GetSql2());
+        }
+    }
+    else
+    {
+        if (GetObject())
+            sqlTextField1->SetText(_("-- nothing to change"));
+        else
+            sqlTextField1->SetText(_("-- definition incomplete"));
+        if (enableSQL2)
+        {
+            sqlTextField2->SetText(wxT(""));
+        }
+    }
+    sqlTextField1->SetReadOnly(true);
+    if (enableSQL2)
+    {
+        sqlTextField2->SetReadOnly(true);
+    }
+}
+
+
 bool dlgProperty::tryUpdate(wxTreeItemId collectionItem)
 {
     ctlTree *browser=mainForm->GetBrowser();
@@ -608,7 +730,7 @@
             mainForm->GetSqlPane()->SetReadOnly(true);
         }
     }
-    else if (item)
+    else if (item && chkReadOnly->GetValue())
     {
         wxTreeItemId collectionItem=item;

@@ -753,8 +875,25 @@
         return;
     }

-    wxString sql=GetSql();
-    wxString sql2=GetSql2();
+    wxString sql;
+    wxString sql2;
+    if (chkReadOnly->GetValue())
+    {
+        sql = GetSql();
+        sql2 = GetSql2();
+    }
+    else
+    {
+        sql = sqlTextField1->GetText();
+        if (enableSQL2)
+        {
+            sql2 = sqlTextField2->GetText();
+        }
+        else
+        {
+            sql2 = wxT("");
+        }
+    }

     if (!apply(sql, sql2))
     {
@@ -768,27 +907,10 @@

 void dlgProperty::OnPageSelect(wxNotebookEvent& event)
 {
-    if (sqlPane && event.GetSelection() == (int)nbNotebook->GetPageCount()-1)
+    if (sqlTextField1 && chkReadOnly->GetValue() &&
+        event.GetSelection() == (int)nbNotebook->GetPageCount()-1)
     {
-        sqlPane->SetReadOnly(false);
-        if (btnOK->IsEnabled())
-        {
-            wxString tmp;
-            if (cbClusterSet && cbClusterSet->GetSelection() > 0)
-            {
-                replClientData *data=(replClientData*)cbClusterSet->GetClientData(cbClusterSet->GetSelection());
-                tmp.Printf(_("-- Execute replicated using cluster \"%s\", set %ld\n"), data->cluster.c_str(),
data->setId);
-            }
-            sqlPane->SetText(tmp + GetSql() + GetSql2());
-        }
-        else
-        {
-            if (GetObject())
-                sqlPane->SetText(_("-- nothing to change"));
-            else
-                sqlPane->SetText(_("-- definition incomplete"));
-        }
-        sqlPane->SetReadOnly(true);
+        FillSQLTextfield();
     }
 }