Re: [PATCH] Tables node (pgAdmin4)

From: Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>, 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-23 13:05:59
Message-ID: CAKKotZS9MbvFpSkYtRfcLZ8_2drrMpL8T-btH4QXf-BPL0cYzQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi,

PFA patch, which will fixes below mentioned issues,

- Fixed all the review comments given by Dave on tables & its child nodes.

*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

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

Attachment Content-Type Size
fixed_review_comments.patch application/octet-stream 77.9 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Susan Douglas 2016-05-23 15:26:53 Patch for documentation update
Previous Message Ashesh Vashi 2016-05-23 11:09:30 Re: [pgAdmin4][Patch]: Auto redirect to page on opening FileManager when used in Fieldset