Re: [PATCH] Tables node (pgAdmin4) - Mailing list pgadmin-hackers

From Ashesh Vashi
Subject Re: [PATCH] Tables node (pgAdmin4)
Date
Msg-id CAG7mmoxJHcTrTFKJGmtVLz-Wix0xpqOmcpHxs141N_Bbx4X7VQ@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] Tables node (pgAdmin4)  (Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com>)
Responses Re: [PATCH] Tables node (pgAdmin4)  (Thom Brown <thom@linux.com>)
List pgadmin-hackers

On Mon, May 23, 2016 at 6:35 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.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


 


--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


On Sat, May 21, 2016 at 2:45 PM, Dave Page <dpage@pgadmin.org> wrote:
I think that makes sense, yes.

Sent from my iPad

On 21 May 2016, at 04:12, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:


On Sat, May 21, 2016 at 12:01 AM, Dave Page <dpage@pgadmin.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.joshi@enterprisedb.com> wrote:
Thanks - Committed with minor changes.

On Thu, May 19, 2016 at 10:47 PM, Harshal Dhumal <harshal.dhumal@enterprisedb.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 






--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers




--
Akshay Joshi
Principal Software Engineer 


Phone: +91 20-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



pgadmin-hackers by date:

Previous
From: Ashesh Vashi
Date:
Subject: pgAdmin 4 commit: Fixed all the review comments from Dave.
Next
From: Ashesh Vashi
Date:
Subject: Re: [pgAdmin4][Patch]: [FileManager] ReferenceError: assignment to undeclared variable t_res