Re: [PATCH] Tables node (pgAdmin4)

From: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
To: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
Cc: Dave Page <dpage(at)pgadmin(dot)org>, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>, Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Tables node (pgAdmin4)
Date: 2016-05-24 18:09:24
Message-ID: CAG7mmoxJHcTrTFKJGmtVLz-Wix0xpqOmcpHxs141N_Bbx4X7VQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

On Mon, May 23, 2016 at 6:35 PM, Murtuza Zabuawala <
murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:

> Hi,
>
> PFA patch, which will fixes below mentioned issues,
>
Committed.

>
> - Fixed all the review comments given by Dave on tables & its child nodes.
>
Can you please list down, what has not yet been taken care in the review
comments?

>
> *Additional enhancements*
> - In Index node, We have updated the way columns were added,
> earlier it was using subnode control now we can insert/update
> values in-place using DepsCell functionality
>
> - In Type node, We have updated the way Composite types were added,
> earlier it was using subnode control now we can insert/update
> values in-place using DepsCell functionality
>
> - In Constraints nodes, Updated error messages handling earlier it was
> throwing error when we open create dialog and no input has been
> provided by user.
>
>
> *Affected nodes by this patch:*
>
>
> 1. Table
> 2. Column
> 3. Check constraint
> 4. Exclusion constraint
> 5. Foreign key
> 6. Primary key
> 7. Unique
> 8. Index
> 9. Trigger
> 10. Type
> 11. Materialized view
>
> --

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company
<http://www.enterprisedb.com/>

*http://www.linkedin.com/in/asheshvashi*
<http://www.linkedin.com/in/asheshvashi>

>
>
>
> --
> Regards,
> Murtuza Zabuawala
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Sat, May 21, 2016 at 2:45 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> I think that makes sense, yes.
>>
>> Sent from my iPad
>>
>> On 21 May 2016, at 04:12, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
>> wrote:
>>
>>
>> On Sat, May 21, 2016 at 12:01 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>> Hi
>>>
>>> I just started to take a look at the table dialogue and friends. Here
>>> are a few issues that we need to address - please take care of them:
>>>
>>> 1) Move columns to their own tab. Vertical scrolling is bad.
>>>
>> Should the 'inherit from table' part of columns tab?
>>
>>>
>>> 2) Similarly, move constraints to their own tab.
>>>
>>> 3) Ensure all labels only have a capital letter on the first word,
>>> except if following words are keywords or acronyms, e.g.
>>>
>>> With default values?
>>> Has OIDs?
>>>
>>> 4) s/System tabel?/System table?
>>>
>>> 5) Error messages on fields should not be shown unless the field loses
>>> focus and has an error (see Create Table)
>>>
>>> 6) The sections on the Properties view are not as they should be. As
>>> I've pointed out before, the "General" section should have a limited subset
>>> of information, e.g. name, oid, owner, tablespace, comment and "is system?"
>>> Other properties should be in other appropriate sections.
>>>
>>> 7) Variables grids should not be on the Security tab (as also mentioned
>>> previously).
>>>
>>> 8) Field labels that imply a question (e.g. usually those with a Yes/No
>>> switch for input) should end in a ? - e.g. "Deferrable?"
>>>
>>> 9) On the Trigger dialogue, "Fires" and following controls should move
>>> to a new tab.
>>>
>>> 10) On the MV dialogue, VACUUM settings should be on their own tab, as
>>> on the Table dialogue.
>>>
>>> 11) Privileges controls on the Properties lists should be in a
>>> "Security" group, not "General"
>>>
>>> I think there are a couple of basic principles to follow here:
>>>
>>> - Make properties lists and dialogues consistent with existing ones,
>>> from control grouping right down to spelling and case of labels.
>>>
>>> - Dialogs should never need vertical scrolling by default (e.g. for a
>>> new object with no columns/constraints/whatever yet defined). If you need
>>> to scroll, then things should be moved to a new tab, grouped as appropriate.
>>>
>>> Thanks.
>>>
>>> On Fri, May 20, 2016 at 7:57 AM, Akshay Joshi <
>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Thanks - Committed with minor changes.
>>>>
>>>> On Thu, May 19, 2016 at 10:47 PM, Harshal Dhumal <
>>>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> PFA updated patch for table and all it's child nodes (Version 9). This
>>>>> patch does not depend on any of existing table node patch.
>>>>>
>>>>> Major change in this patch: Unlike pgAdmin3 now in table create mode
>>>>> any constraint(s) created (but not saved) will listen to table column
>>>>> changes and adapt themselves accordingly.
>>>>>
>>>>> For e.g.
>>>>> In table create mode user adds column definition with name "col1" then
>>>>> adds constraint which includes column "col1". Now user changes column name
>>>>> to "col2" then constraint will listen to this change and adapt the column
>>>>> name from "col1" to "col2" in constraint definition. Also if column "col2"
>>>>> is removed then constraint will also remove the column "col2" from it's
>>>>> definition.
>>>>>
>>>>>
>>>>> --
>>>>> *Harshal Dhumal*
>>>>> *Software Engineer *
>>>>>
>>>>>
>>>>>
>>>>> EenterpriseDB <http://www.enterprisedb.com>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Sent via pgadmin-hackers mailing list (pgadmin-hackers(at)postgresql(dot)org)
>>>>> To make changes to your subscription:
>>>>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> *Akshay Joshi*
>>>> *Principal Software Engineer *
>>>>
>>>>
>>>>
>>>> *Phone: +91 20-3058-9517 <%2B91%2020-3058-9517>Mobile: +91 976-788-8246*
>>>>
>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Ashesh Vashi 2016-05-24 18:13:52 Re: [pgAdmin4][Patch]: [FileManager] ReferenceError: assignment to undeclared variable t_res
Previous Message Ashesh Vashi 2016-05-24 18:08:20 pgAdmin 4 commit: Fixed all the review comments from Dave.