Re: [pgAdmin4] [Patch]: Grant Wizard

Hi Dave,

Please find updated patch with suggested changes.

On Thu, Mar 10, 2016 at 5:26 PM, Dave Page <dpage@pgadmin.org> wrote:

On Thu, Mar 10, 2016 at 9:50 AM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> Please apply Khusboo's patch for "Privileges macros under Schema" before
> using grant wizard patch.

Thanks, that works. Some (hopefully final) feedback:

- Can we add a side-image? Not sure what yet - just a placeholder for
now until I come up with something.

Already a placeholder for left side image.

- Why is the closed button in an odd position (see File -> Test Alert
for comparison)

Fixed 

- Why are we using a scrolling list AND pagination? I think a
scrolling list alone should be fine.

Pagination is removed. 

- The grid sizing is wrong. See how the scrollbar on the right in the
screenshot is off the edge of the dialogue, and there's a horizontal
scrollbar?

Fixed.

- We shouldn't truncate object names as that can be ambiguous. The
column should extend as necessary, and there should be a horizontal
scrollbar on the grid itself (not at the bottom of the dialogue
content).

I have fixed it and scrollbar is now on the grid itself.

- Function names should include the parameters, as they are part of
the identifier. Without, it can be ambigous - e.g. do_stuff(int) vs.
do_stuff(text).

Fixed. 

- If I select only functions (for example), the Privileges panel
should only list privileges available for functions. If I select
multiple object types, it should show the available options for only
those object types.

Implemented this feature. 

- If I select functions and tables, and then choose (for example),
usage and truncate, it will attempt to set usage on tables and
truncate on functions. It should only attempt to set privileges on the
objects for which they are appropriate.

Done 

- On the last page, the Next button is disabled. It is turned a
marginally darker blue, but also highlights on mouse-over. The change
in shade is so subtle it's hard to see, and the highlight implies the
button is active when it isn't.

Fixed. 

- The buttons appear to have a smaller corner radius than those on the
main browser, or in other dialogues.

Fixed. 

- Button labels should have an &nbsp; before them to properly space
the label from the icon (or better yet, this should be done in CSS,
though that would also need to be done elsewhere).

done with css. 

- Why do the URLs have a /wizard prefix? I think that should be removed.

Removed prefix. 

- The available privileges for each object type seem to be defined in
both grant_wizard.js and allowed_acl.json. Can we just use
allowed_acl.json?

Yes, allowed_acl.json is now used in grant_wizard.js.

Thanks.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company