Обсуждение: [pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool

Поиск
Список
Период
Сортировка

[pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool

От
Akshay Joshi
Дата:
Hi All

I have implemented the Data Grid and Query Tool as one component. 

Functionality working for Data Grid:
  • Add/Update/Delete rows if object is editable.
  • Copy row 
  • Paste row
  • Refresh
  • Client Filter provided by the Backgrid.
  • Filter (User specified, By cell Selection, Exclude Selection
  • Add Limit to the query result
  • Download the data as CSV.
Todo's for the Data Grid:
  • Find a way to select backgrid cell when rows are not editable. If backgrid rows are not editable then Filter by selection and Exclude selection won't work 
Functionality working for Query Tool:
  • Execute sql query.
  • Execute the Highlighted sql.
  • Cancel the running query. 
  • Download the data as CSV
Todo's for the Query Tool
  • Open as SQL file.
  • Save to SQL file.
  • Auto Commit/ Rollback.
  • Output Panel should be resizable, so that user can
Attached is the patch file. Please review it and let me know the review comments if any.
 
--
Akshay Joshi
Principal Software Engineer 


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246
Вложения

Re: [pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool

От
Dave Page
Дата:
Hi

On Tue, Apr 5, 2016 at 10:20 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi All

I have implemented the Data Grid and Query Tool as one component. 

Functionality working for Data Grid:
  • Add/Update/Delete rows if object is editable.
  • Copy row 
  • Paste row
  • Refresh
  • Client Filter provided by the Backgrid.
  • Filter (User specified, By cell Selection, Exclude Selection
  • Add Limit to the query result
  • Download the data as CSV.
Todo's for the Data Grid:
  • Find a way to select backgrid cell when rows are not editable. If backgrid rows are not editable then Filter by selection and Exclude selection won't work 
Functionality working for Query Tool:
  • Execute sql query.
  • Execute the Highlighted sql.
  • Cancel the running query. 
  • Download the data as CSV
Todo's for the Query Tool
  • Open as SQL file.
  • Save to SQL file.
  • Auto Commit/ Rollback.
  • Output Panel should be resizable, so that user can
Attached is the patch file. Please review it and let me know the review comments if any.

Initial feedback based on my playing with it, and discussion with Ashesh:

- The View Data menu option should be on the Object menu, which should mirror the Context menu, except options should be disabled when not applicable instead of hidden.

- The Query Tool menu icon should be a glyphicon, to match the others.

- Please merge the functionality of the Refresh and Execute buttons into one button. We shouldn't have two buttons that do essentially the same thing.

- In Edit Grid mode, the History panel should log all queries (SELECTs, UPDATEs, DELETEs etc) as it would in the Query Tool.

- In Edit Grid mode, the Messages panel should display any messages from the most recent action  as it would in the Query Tool.

- Please add an SQL button. This should show/hide the SQL panel in *both* Query Tool and Edit Grid modes. In Edit Grid mode, that textbox should be read-only, but should display the SQL used (including any LIMIT/FILTER clauses)

- Please remove the border from the SQL box, such that it fills all available space.

- The Filter box should be in a modal overlay over the top of the SQL box/Results tabs as required. Those elements should be grayed whilst it is open.

- Please adjust the height of the Delete icon in the Edit Grid, such that it doesn't force the row height to be higher than it should be.

- If a field has been edited, but not saved, can we highlight it somehow? Maybe make the text dark blue?

- I think the names of the tabs are far too long. Can we change them to "Query 1", "Query 2" etc, then rename them to the filename if the user saves/loads a file?

- Ashesh and I discussed changing the History tab to be a grid, showing: Date/Time, Query (first line only), Rows affected, Runtime and Status, in a row per query executed. Ashesh suggested using a sub-form that can be expanded for each row, which could show the full query and error details (SQL State etc). New rows should be added to the top of the list.

- We should add an "Edit" button, which opens a drop-down menu. This would eventually include options as found on the Edit menu in the pgAdmin3 query tool, such as the "Clear SQL" option.

- Errors should be highlighted in the SQL box - a marker in the margin to note the line, and spellcheck-style underlining for the error word.

- Query results should have spaces converted to "&nbsp;", so that proper indenting is maintained (for example, on EXPLAIN queries).

- The "Add Row" button only works if you're on the last page of the resultset.

- Can the "Copy Row" button also populate the clipboard with CSV data for the row?

- In Edit mode, we need to be able to represent/set values to NULL.

- If I shutdown my pgAdmin server, then execute a query, I get no error, 0 rows displayed, and a message in the messages panel saying:

Total query runtime: 46 msec
3 rows retrieved.

If I restart the server, the query will execute correctly, however I should see appropriate messages when it's not running.

- The layout of the result tabs should be maintained if new Query Tool or Edit Grid tabs are opened.

Thanks!

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

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

Re: [pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool

От
Akshay Joshi
Дата:
Hi 

On Wed, Apr 6, 2016 at 8:28 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Apr 5, 2016 at 10:20 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi All

I have implemented the Data Grid and Query Tool as one component. 

Functionality working for Data Grid:
  • Add/Update/Delete rows if object is editable.
  • Copy row 
  • Paste row
  • Refresh
  • Client Filter provided by the Backgrid.
  • Filter (User specified, By cell Selection, Exclude Selection
  • Add Limit to the query result
  • Download the data as CSV.
Todo's for the Data Grid:
  • Find a way to select backgrid cell when rows are not editable. If backgrid rows are not editable then Filter by selection and Exclude selection won't work 
Functionality working for Query Tool:
  • Execute sql query.
  • Execute the Highlighted sql.
  • Cancel the running query. 
  • Download the data as CSV
Todo's for the Query Tool
  • Open as SQL file.
  • Save to SQL file.
  • Auto Commit/ Rollback.
  • Output Panel should be resizable, so that user can
Attached is the patch file. Please review it and let me know the review comments if any.

Initial feedback based on my playing with it, and discussion with Ashesh:

- The View Data menu option should be on the Object menu  
  
   OK.
 
, which should mirror the Context menu, except options should be disabled when not applicable instead of hidden.
 
     Context menu code is generic code, to do this we need to change that code and it impacts other menu items like (Connect/Disconnect server, Connect/Disconnect Database etc ...)  

- The Query Tool menu icon should be a glyphicon, to match the others.
 
   There is no glyphicon available which match the Query Tool icon. I have found one like below which is "database-search" or can you please suggest some other icon. 
       Inline image 1 

- Please merge the functionality of the Refresh and Execute buttons into one button. We shouldn't have two buttons that do essentially the same thing.
   
    I have modified the button toolbar, we will show buttons on the toolbar specific to the module (QueryTool/EditGrid). Please refer the attached screenshots (QueryTool and EditGrid).


- In Edit Grid mode, the History panel should log all queries (SELECTs, UPDATEs, DELETEs etc) as it would in the Query Tool.

- In Edit Grid mode, the Messages panel should display any messages from the most recent action  as it would in the Query Tool.
 
   OK. 

- Please add an SQL button. This should show/hide the SQL panel in *both* Query Tool and Edit Grid modes. In Edit Grid mode, that textbox should be read-only, but should display the SQL used (including any LIMIT/FILTER clauses)

    I think we don't need an SQL button, because I have added a Splitter to split SQL panel and Output Panel, so user can any time resize the SQL/Output panel. Please refer the attached screenshots (QueryTool and EditGrid).     

- Please remove the border from the SQL box, such that it fills all available space.
 
   Done. Please refer the attached screenshots (QueryTool and EditGrid).

- The Filter box should be in a modal overlay over the top of the SQL box/Results tabs as required. Those elements should be grayed whilst it is open.
  
   Done. Please refer the attached screenshot (Filter). 

- Please adjust the height of the Delete icon in the Edit Grid, such that it doesn't force the row height to be higher than it should be.
 
   OK. 

- If a field has been edited, but not saved, can we highlight it somehow? Maybe make the text dark blue?
 
   OK, not sure right now but will try. 

- I think the names of the tabs are far too long. Can we change them to "Query 1", "Query 2" etc, then rename them to the filename if the user saves/loads a file?
 
   I personally feel it's been difficult for user to identify the tab if we will give names like "Query 1" . What we can do in case of edit grid we will only show the servername-objectname (remove database and schema) or only objectname, and in case of query tool we will show servername-databasename. What do you think?
 
- Ashesh and I discussed changing the History tab to be a grid, showing: Date/Time, Query (first line only), Rows affected, Runtime and Status, in a row per query executed. Ashesh suggested using a sub-form that can be expanded for each row, which could show the full query and error details (SQL State etc). New rows should be added to the top of the list.

- We should add an "Edit" button, which opens a drop-down menu. This would eventually include options as found on the Edit menu in the pgAdmin3 query tool, such as the "Clear SQL" option.

- Errors should be highlighted in the SQL box - a marker in the margin to note the line, and spellcheck-style underlining for the error word.
  
    OK. 

- Query results should have spaces converted to "&nbsp;", so that proper indenting is maintained (for example, on EXPLAIN queries).
  
    Instead of converting spaces to "&nbsp;" we can have css style "white-space: pre-wrap;". I have tested it and works fine.

- The "Add Row" button only works if you're on the last page of the resultset.
 
   OK. 

- Can the "Copy Row" button also populate the clipboard with CSV data for the row?
 
   This required some research, not sure at the moment.  

- In Edit mode, we need to be able to represent/set values to NULL.
   
   This will be taken care as part of multi-type rendering task. 

- If I shutdown my pgAdmin server, then execute a query, I get no error, 0 rows displayed, and a message in the messages panel saying:

Total query runtime: 46 msec
3 rows retrieved.

If I restart the server, the query will execute correctly, however I should see appropriate messages when it's not running.

- The layout of the result tabs should be maintained if new Query Tool or Edit Grid tabs are opened.

   OK. 

Thanks!

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

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



--
Akshay Joshi
Principal Software Engineer 


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246
Вложения

Re: [pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool

От
Dave Page
Дата:
Hi

On Thu, Apr 7, 2016 at 10:07 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

- The View Data menu option should be on the Object menu  
  
   OK.
 
, which should mirror the Context menu, except options should be disabled when not applicable instead of hidden.
 
     Context menu code is generic code, to do this we need to change that code and it impacts other menu items like (Connect/Disconnect server, Connect/Disconnect Database etc ...)  

I think you misunderstand. The Context menu is fine - but options that are on it, should also be on the main "Object" menu. The difference is that where options are hidden on the context menu, they should be disabled on the object menu.
 

- The Query Tool menu icon should be a glyphicon, to match the others.
 
   There is no glyphicon available which match the Query Tool icon. I have found one like below which is "database-search" or can you please suggest some other icon. 
       Inline image 1 

That one looks perfect.
 

- Please merge the functionality of the Refresh and Execute buttons into one button. We shouldn't have two buttons that do essentially the same thing.
   
    I have modified the button toolbar, we will show buttons on the toolbar specific to the module (QueryTool/EditGrid). Please refer the attached screenshots (QueryTool and EditGrid).

No - please leave all buttons visible in either mode, but disable them as appropriate. Then, merge the execute/refresh buttons into one.

The reason for doing it this way is that eventually we will add query parsing support to the client so that we can start enabling some of the Edit Grid features when running in Query Tool mode - e.g. if a Query Tool authored query is determined to be updatable, we can enable buttons like "Add Row", "Save" etc.
 


- In Edit Grid mode, the History panel should log all queries (SELECTs, UPDATEs, DELETEs etc) as it would in the Query Tool.

- In Edit Grid mode, the Messages panel should display any messages from the most recent action  as it would in the Query Tool.
 
   OK. 

Thanks.
 

- Please add an SQL button. This should show/hide the SQL panel in *both* Query Tool and Edit Grid modes. In Edit Grid mode, that textbox should be read-only, but should display the SQL used (including any LIMIT/FILTER clauses)

    I think we don't need an SQL button, because I have added a Splitter to split SQL panel and Output Panel, so user can any time resize the SQL/Output panel. Please refer the attached screenshots (QueryTool and EditGrid).     

Right, but I'd also like to be able to hide it entirely (which would be the default in Edit Grid mode).
 

- Please remove the border from the SQL box, such that it fills all available space.
 
   Done. Please refer the attached screenshots (QueryTool and EditGrid).

- The Filter box should be in a modal overlay over the top of the SQL box/Results tabs as required. Those elements should be grayed whilst it is open.
  
   Done. Please refer the attached screenshot (Filter). 

Cool.
 

- Please adjust the height of the Delete icon in the Edit Grid, such that it doesn't force the row height to be higher than it should be.
 
   OK. 

- If a field has been edited, but not saved, can we highlight it somehow? Maybe make the text dark blue?
 
   OK, not sure right now but will try. 

- I think the names of the tabs are far too long. Can we change them to "Query 1", "Query 2" etc, then rename them to the filename if the user saves/loads a file?
 
   I personally feel it's been difficult for user to identify the tab if we will give names like "Query 1" . What we can do in case of edit grid we will only show the servername-objectname (remove database and schema) or only objectname, and in case of query tool we will show servername-databasename. What do you think?

I think they're all ambiguous enough to not be useful for many users :-( - plus the tabs are so long, they look awful. 

We should perhaps add a small panel (below the toolbar?) that shows Server -> Database in QT mode, and Server -> Database -> Schema -> Table/View in EG mode.
 
 
- Ashesh and I discussed changing the History tab to be a grid, showing: Date/Time, Query (first line only), Rows affected, Runtime and Status, in a row per query executed. Ashesh suggested using a sub-form that can be expanded for each row, which could show the full query and error details (SQL State etc). New rows should be added to the top of the list.

- We should add an "Edit" button, which opens a drop-down menu. This would eventually include options as found on the Edit menu in the pgAdmin3 query tool, such as the "Clear SQL" option.

- Errors should be highlighted in the SQL box - a marker in the margin to note the line, and spellcheck-style underlining for the error word.
  
    OK. 

- Query results should have spaces converted to "&nbsp;", so that proper indenting is maintained (for example, on EXPLAIN queries).
  
    Instead of converting spaces to "&nbsp;" we can have css style "white-space: pre-wrap;". I have tested it and works fine.

- The "Add Row" button only works if you're on the last page of the resultset.
 
   OK. 

- Can the "Copy Row" button also populate the clipboard with CSV data for the row?
 
   This required some research, not sure at the moment.  

- In Edit mode, we need to be able to represent/set values to NULL.
   
   This will be taken care as part of multi-type rendering task. 

OK.
 

- If I shutdown my pgAdmin server, then execute a query, I get no error, 0 rows displayed, and a message in the messages panel saying:

Total query runtime: 46 msec
3 rows retrieved.

If I restart the server, the query will execute correctly, however I should see appropriate messages when it's not running.

- The layout of the result tabs should be maintained if new Query Tool or Edit Grid tabs are opened.

   OK. 

Cool - thanks! 

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Вложения

Re: [pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool

От
Akshay Joshi
Дата:
Hi

On Thu, Apr 7, 2016 at 6:01 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Thu, Apr 7, 2016 at 10:07 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

- The View Data menu option should be on the Object menu  
  
   OK.
 
, which should mirror the Context menu, except options should be disabled when not applicable instead of hidden.
 
     Context menu code is generic code, to do this we need to change that code and it impacts other menu items like (Connect/Disconnect server, Connect/Disconnect Database etc ...)  

I think you misunderstand. The Context menu is fine - but options that are on it, should also be on the main "Object" menu. The difference is that where options are hidden on the context menu, they should be disabled on the object menu.

    The same behaviour I have already added in "Tools" menu, so you want me to shift that menu from "Tools" to "Object"?? 
 

- The Query Tool menu icon should be a glyphicon, to match the others.
 
   There is no glyphicon available which match the Query Tool icon. I have found one like below which is "database-search" or can you please suggest some other icon. 
       Inline image 1 

That one looks perfect.
 

- Please merge the functionality of the Refresh and Execute buttons into one button. We shouldn't have two buttons that do essentially the same thing.
   
    I have modified the button toolbar, we will show buttons on the toolbar specific to the module (QueryTool/EditGrid). Please refer the attached screenshots (QueryTool and EditGrid).

No - please leave all buttons visible in either mode, but disable them as appropriate. Then, merge the execute/refresh buttons into one.

    In that case we will have to use either refresh icon or execute icon (play button) or any other, as a user's perspective it's not good to have play button works as "Refresh" in EG mode or refresh button works as "Execute" in QT mode. 

The reason for doing it this way is that eventually we will add query parsing support to the client so that we can start enabling some of the Edit Grid features when running in Query Tool mode - e.g. if a Query Tool authored query is determined to be updatable, we can enable buttons like "Add Row", "Save" etc.

    OK. 
 


- In Edit Grid mode, the History panel should log all queries (SELECTs, UPDATEs, DELETEs etc) as it would in the Query Tool.

- In Edit Grid mode, the Messages panel should display any messages from the most recent action  as it would in the Query Tool.
 
   OK. 

Thanks.
 

- Please add an SQL button. This should show/hide the SQL panel in *both* Query Tool and Edit Grid modes. In Edit Grid mode, that textbox should be read-only, but should display the SQL used (including any LIMIT/FILTER clauses)

    I think we don't need an SQL button, because I have added a Splitter to split SQL panel and Output Panel, so user can any time resize the SQL/Output panel. Please refer the attached screenshots (QueryTool and EditGrid).     

Right, but I'd also like to be able to hide it entirely (which would be the default in Edit Grid mode).

    To achieve this we need to hide the top half of the splitter, that I am not sure how we can do that. We can't remove the splitter, because it provide's ease to user to expand the SQL panel for reading the long SQL queries/functions.  
 

- Please remove the border from the SQL box, such that it fills all available space.
 
   Done. Please refer the attached screenshots (QueryTool and EditGrid).

- The Filter box should be in a modal overlay over the top of the SQL box/Results tabs as required. Those elements should be grayed whilst it is open.
  
   Done. Please refer the attached screenshot (Filter). 

Cool.
 

- Please adjust the height of the Delete icon in the Edit Grid, such that it doesn't force the row height to be higher than it should be.
 
   OK. 

- If a field has been edited, but not saved, can we highlight it somehow? Maybe make the text dark blue?
 
   OK, not sure right now but will try. 

- I think the names of the tabs are far too long. Can we change them to "Query 1", "Query 2" etc, then rename them to the filename if the user saves/loads a file?
 
   I personally feel it's been difficult for user to identify the tab if we will give names like "Query 1" . What we can do in case of edit grid we will only show the servername-objectname (remove database and schema) or only objectname, and in case of query tool we will show servername-databasename. What do you think?

I think they're all ambiguous enough to not be useful for many users :-( - plus the tabs are so long, they look awful. 

We should perhaps add a small panel (below the toolbar?) that shows Server -> Database in QT mode, and Server -> Database -> Schema -> Table/View in EG mode.

    OK. 
 
 
- Ashesh and I discussed changing the History tab to be a grid, showing: Date/Time, Query (first line only), Rows affected, Runtime and Status, in a row per query executed. Ashesh suggested using a sub-form that can be expanded for each row, which could show the full query and error details (SQL State etc). New rows should be added to the top of the list.

- We should add an "Edit" button, which opens a drop-down menu. This would eventually include options as found on the Edit menu in the pgAdmin3 query tool, such as the "Clear SQL" option.

- Errors should be highlighted in the SQL box - a marker in the margin to note the line, and spellcheck-style underlining for the error word.
  
    OK. 

- Query results should have spaces converted to "&nbsp;", so that proper indenting is maintained (for example, on EXPLAIN queries).
  
    Instead of converting spaces to "&nbsp;" we can have css style "white-space: pre-wrap;". I have tested it and works fine.

- The "Add Row" button only works if you're on the last page of the resultset.
 
   OK. 

- Can the "Copy Row" button also populate the clipboard with CSV data for the row?
 
   This required some research, not sure at the moment.  

- In Edit mode, we need to be able to represent/set values to NULL.
   
   This will be taken care as part of multi-type rendering task. 

OK.
 

- If I shutdown my pgAdmin server, then execute a query, I get no error, 0 rows displayed, and a message in the messages panel saying:

Total query runtime: 46 msec
3 rows retrieved.

If I restart the server, the query will execute correctly, however I should see appropriate messages when it's not running.

- The layout of the result tabs should be maintained if new Query Tool or Edit Grid tabs are opened.

   OK. 

Cool - thanks! 

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

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



--
Akshay Joshi
Principal Software Engineer 


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246
Вложения

Re: [pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool

От
Dave Page
Дата:


On Thu, Apr 7, 2016 at 2:03 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi

On Thu, Apr 7, 2016 at 6:01 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Thu, Apr 7, 2016 at 10:07 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

- The View Data menu option should be on the Object menu  
  
   OK.
 
, which should mirror the Context menu, except options should be disabled when not applicable instead of hidden.
 
     Context menu code is generic code, to do this we need to change that code and it impacts other menu items like (Connect/Disconnect server, Connect/Disconnect Database etc ...)  

I think you misunderstand. The Context menu is fine - but options that are on it, should also be on the main "Object" menu. The difference is that where options are hidden on the context menu, they should be disabled on the object menu.

    The same behaviour I have already added in "Tools" menu, so you want me to shift that menu from "Tools" to "Object"?? 

Just for the View/Edit options. The Query Tool is never shown on the Context menu, so it should be on Tools.
 
 

- The Query Tool menu icon should be a glyphicon, to match the others.
 
   There is no glyphicon available which match the Query Tool icon. I have found one like below which is "database-search" or can you please suggest some other icon. 
       Inline image 1 

That one looks perfect.
 

- Please merge the functionality of the Refresh and Execute buttons into one button. We shouldn't have two buttons that do essentially the same thing.
   
    I have modified the button toolbar, we will show buttons on the toolbar specific to the module (QueryTool/EditGrid). Please refer the attached screenshots (QueryTool and EditGrid).

No - please leave all buttons visible in either mode, but disable them as appropriate. Then, merge the execute/refresh buttons into one.

    In that case we will have to use either refresh icon or execute icon (play button) or any other, as a user's perspective it's not good to have play button works as "Refresh" in EG mode or refresh button works as "Execute" in QT mode. 

MySQL Workbench uses something like glyphicon-flash.
 

The reason for doing it this way is that eventually we will add query parsing support to the client so that we can start enabling some of the Edit Grid features when running in Query Tool mode - e.g. if a Query Tool authored query is determined to be updatable, we can enable buttons like "Add Row", "Save" etc.

    OK. 
 


- In Edit Grid mode, the History panel should log all queries (SELECTs, UPDATEs, DELETEs etc) as it would in the Query Tool.

- In Edit Grid mode, the Messages panel should display any messages from the most recent action  as it would in the Query Tool.
 
   OK. 

Thanks.
 

- Please add an SQL button. This should show/hide the SQL panel in *both* Query Tool and Edit Grid modes. In Edit Grid mode, that textbox should be read-only, but should display the SQL used (including any LIMIT/FILTER clauses)

    I think we don't need an SQL button, because I have added a Splitter to split SQL panel and Output Panel, so user can any time resize the SQL/Output panel. Please refer the attached screenshots (QueryTool and EditGrid).     

Right, but I'd also like to be able to hide it entirely (which would be the default in Edit Grid mode).

    To achieve this we need to hide the top half of the splitter, that I am not sure how we can do that. We can't remove the splitter, because it provide's ease to user to expand the SQL panel for reading the long SQL queries/functions.  

You could put the small panel (discussed below) that displays the name of the object in the splitter panel as well. Then, you wouldn't have to hide the whole thing. It might look a little weird unless you force the splitter to move when you hide the SQL box though.
 
 

- Please remove the border from the SQL box, such that it fills all available space.
 
   Done. Please refer the attached screenshots (QueryTool and EditGrid).

- The Filter box should be in a modal overlay over the top of the SQL box/Results tabs as required. Those elements should be grayed whilst it is open.
  
   Done. Please refer the attached screenshot (Filter). 

Cool.
 

- Please adjust the height of the Delete icon in the Edit Grid, such that it doesn't force the row height to be higher than it should be.
 
   OK. 

- If a field has been edited, but not saved, can we highlight it somehow? Maybe make the text dark blue?
 
   OK, not sure right now but will try. 

- I think the names of the tabs are far too long. Can we change them to "Query 1", "Query 2" etc, then rename them to the filename if the user saves/loads a file?
 
   I personally feel it's been difficult for user to identify the tab if we will give names like "Query 1" . What we can do in case of edit grid we will only show the servername-objectname (remove database and schema) or only objectname, and in case of query tool we will show servername-databasename. What do you think?

I think they're all ambiguous enough to not be useful for many users :-( - plus the tabs are so long, they look awful. 

We should perhaps add a small panel (below the toolbar?) that shows Server -> Database in QT mode, and Server -> Database -> Schema -> Table/View in EG mode.

    OK. 
 
 
- Ashesh and I discussed changing the History tab to be a grid, showing: Date/Time, Query (first line only), Rows affected, Runtime and Status, in a row per query executed. Ashesh suggested using a sub-form that can be expanded for each row, which could show the full query and error details (SQL State etc). New rows should be added to the top of the list.

- We should add an "Edit" button, which opens a drop-down menu. This would eventually include options as found on the Edit menu in the pgAdmin3 query tool, such as the "Clear SQL" option.

- Errors should be highlighted in the SQL box - a marker in the margin to note the line, and spellcheck-style underlining for the error word.
  
    OK. 

- Query results should have spaces converted to "&nbsp;", so that proper indenting is maintained (for example, on EXPLAIN queries).
  
    Instead of converting spaces to "&nbsp;" we can have css style "white-space: pre-wrap;". I have tested it and works fine.

- The "Add Row" button only works if you're on the last page of the resultset.
 
   OK. 

- Can the "Copy Row" button also populate the clipboard with CSV data for the row?
 
   This required some research, not sure at the moment.  

- In Edit mode, we need to be able to represent/set values to NULL.
   
   This will be taken care as part of multi-type rendering task. 

OK.
 

- If I shutdown my pgAdmin server, then execute a query, I get no error, 0 rows displayed, and a message in the messages panel saying:

Total query runtime: 46 msec
3 rows retrieved.

If I restart the server, the query will execute correctly, however I should see appropriate messages when it's not running.

- The layout of the result tabs should be maintained if new Query Tool or Edit Grid tabs are opened.

   OK. 

Cool - thanks! 

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

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



--
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
Вложения

Re: [pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool

От
Akshay Joshi
Дата:
Hi

On Thu, Apr 7, 2016 at 6:38 PM, Dave Page <dpage@pgadmin.org> wrote:


On Thu, Apr 7, 2016 at 2:03 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi

On Thu, Apr 7, 2016 at 6:01 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Thu, Apr 7, 2016 at 10:07 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

- The View Data menu option should be on the Object menu  
  
   OK.
 
, which should mirror the Context menu, except options should be disabled when not applicable instead of hidden.
 
     Context menu code is generic code, to do this we need to change that code and it impacts other menu items like (Connect/Disconnect server, Connect/Disconnect Database etc ...)  

I think you misunderstand. The Context menu is fine - but options that are on it, should also be on the main "Object" menu. The difference is that where options are hidden on the context menu, they should be disabled on the object menu.

    The same behaviour I have already added in "Tools" menu, so you want me to shift that menu from "Tools" to "Object"?? 

Just for the View/Edit options. The Query Tool is never shown on the Context menu, so it should be on Tools.
 
 

- The Query Tool menu icon should be a glyphicon, to match the others.
 
   There is no glyphicon available which match the Query Tool icon. I have found one like below which is "database-search" or can you please suggest some other icon. 
       Inline image 1 

That one looks perfect.

           We can't use this icon because it's not come with Bootstrap , I have picked this from "http://glyphicons.com/" and I am not sure we can use it as per the Licence.
 

- Please merge the functionality of the Refresh and Execute buttons into one button. We shouldn't have two buttons that do essentially the same thing.
   
    I have modified the button toolbar, we will show buttons on the toolbar specific to the module (QueryTool/EditGrid). Please refer the attached screenshots (QueryTool and EditGrid).

No - please leave all buttons visible in either mode, but disable them as appropriate. Then, merge the execute/refresh buttons into one.

    In that case we will have to use either refresh icon or execute icon (play button) or any other, as a user's perspective it's not good to have play button works as "Refresh" in EG mode or refresh button works as "Execute" in QT mode. 

MySQL Workbench uses something like glyphicon-flash.
 

The reason for doing it this way is that eventually we will add query parsing support to the client so that we can start enabling some of the Edit Grid features when running in Query Tool mode - e.g. if a Query Tool authored query is determined to be updatable, we can enable buttons like "Add Row", "Save" etc.

    OK. 
 


- In Edit Grid mode, the History panel should log all queries (SELECTs, UPDATEs, DELETEs etc) as it would in the Query Tool.

- In Edit Grid mode, the Messages panel should display any messages from the most recent action  as it would in the Query Tool.
 
   OK. 

Thanks.
 

- Please add an SQL button. This should show/hide the SQL panel in *both* Query Tool and Edit Grid modes. In Edit Grid mode, that textbox should be read-only, but should display the SQL used (including any LIMIT/FILTER clauses)

    I think we don't need an SQL button, because I have added a Splitter to split SQL panel and Output Panel, so user can any time resize the SQL/Output panel. Please refer the attached screenshots (QueryTool and EditGrid).     

Right, but I'd also like to be able to hide it entirely (which would be the default in Edit Grid mode).

    To achieve this we need to hide the top half of the splitter, that I am not sure how we can do that. We can't remove the splitter, because it provide's ease to user to expand the SQL panel for reading the long SQL queries/functions.  

You could put the small panel (discussed below) that displays the name of the object in the splitter panel as well. Then, you wouldn't have to hide the whole thing. It might look a little weird unless you force the splitter to move when you hide the SQL box though.
 
 

- Please remove the border from the SQL box, such that it fills all available space.
 
   Done. Please refer the attached screenshots (QueryTool and EditGrid).

- The Filter box should be in a modal overlay over the top of the SQL box/Results tabs as required. Those elements should be grayed whilst it is open.
  
   Done. Please refer the attached screenshot (Filter). 

Cool.
 

- Please adjust the height of the Delete icon in the Edit Grid, such that it doesn't force the row height to be higher than it should be.
 
   OK. 

- If a field has been edited, but not saved, can we highlight it somehow? Maybe make the text dark blue?
 
   OK, not sure right now but will try. 

- I think the names of the tabs are far too long. Can we change them to "Query 1", "Query 2" etc, then rename them to the filename if the user saves/loads a file?
 
   I personally feel it's been difficult for user to identify the tab if we will give names like "Query 1" . What we can do in case of edit grid we will only show the servername-objectname (remove database and schema) or only objectname, and in case of query tool we will show servername-databasename. What do you think?

I think they're all ambiguous enough to not be useful for many users :-( - plus the tabs are so long, they look awful. 

We should perhaps add a small panel (below the toolbar?) that shows Server -> Database in QT mode, and Server -> Database -> Schema -> Table/View in EG mode.

    OK. 
 
 
- Ashesh and I discussed changing the History tab to be a grid, showing: Date/Time, Query (first line only), Rows affected, Runtime and Status, in a row per query executed. Ashesh suggested using a sub-form that can be expanded for each row, which could show the full query and error details (SQL State etc). New rows should be added to the top of the list.

- We should add an "Edit" button, which opens a drop-down menu. This would eventually include options as found on the Edit menu in the pgAdmin3 query tool, such as the "Clear SQL" option.

- Errors should be highlighted in the SQL box - a marker in the margin to note the line, and spellcheck-style underlining for the error word.
  
    OK. 

- Query results should have spaces converted to "&nbsp;", so that proper indenting is maintained (for example, on EXPLAIN queries).
  
    Instead of converting spaces to "&nbsp;" we can have css style "white-space: pre-wrap;". I have tested it and works fine.

- The "Add Row" button only works if you're on the last page of the resultset.
 
   OK. 

- Can the "Copy Row" button also populate the clipboard with CSV data for the row?
 
   This required some research, not sure at the moment.  

- In Edit mode, we need to be able to represent/set values to NULL.
   
   This will be taken care as part of multi-type rendering task. 

OK.
 

- If I shutdown my pgAdmin server, then execute a query, I get no error, 0 rows displayed, and a message in the messages panel saying:

Total query runtime: 46 msec
3 rows retrieved.

If I restart the server, the query will execute correctly, however I should see appropriate messages when it's not running.

- The layout of the result tabs should be maintained if new Query Tool or Edit Grid tabs are opened.

   OK. 

Cool - thanks! 

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

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



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



--
Akshay Joshi
Principal Software Engineer 


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246
Вложения

Re: [pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool

От
Dave Page
Дата:


On Fri, Apr 8, 2016 at 7:43 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

- The Query Tool menu icon should be a glyphicon, to match the others.
 
   There is no glyphicon available which match the Query Tool icon. I have found one like below which is "database-search" or can you please suggest some other icon. 
       Inline image 1 

That one looks perfect.

           We can't use this icon because it's not come with Bootstrap , I have picked this from "http://glyphicons.com/" and I am not sure we can use it as per the Licence.

At the risk of annoying everyone immensely, on reflection I'm thinking we should use Font Awesome as our default generic icons, and fall back to Bootstrap's Glyphicons. I really hadn't realised how much larger the FA set is.

For this particular issue, could we use FA's stacking? e.g. something like: https://jsfiddle.net/pa8x6nt3/ 

If not, how about using the execute icon we discussed, e.g. fa-bolt?

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Вложения

Re: [pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool

От
Akshay Joshi
Дата:
Hi Dave

On Fri, Apr 8, 2016 at 2:32 PM, Dave Page <dpage@pgadmin.org> wrote:


On Fri, Apr 8, 2016 at 7:43 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

- The Query Tool menu icon should be a glyphicon, to match the others.
 
   There is no glyphicon available which match the Query Tool icon. I have found one like below which is "database-search" or can you please suggest some other icon. 
       Inline image 1 

That one looks perfect.

           We can't use this icon because it's not come with Bootstrap , I have picked this from "http://glyphicons.com/" and I am not sure we can use it as per the Licence.

At the risk of annoying everyone immensely, on reflection I'm thinking we should use Font Awesome as our default generic icons, and fall back to Bootstrap's Glyphicons. I really hadn't realised how much larger the FA set is.

For this particular issue, could we use FA's stacking? e.g. something like: https://jsfiddle.net/pa8x6nt3/ 

If not, how about using the execute icon we discussed, e.g. fa-bolt?

   I have used fa-bolt. Apart from that I have completed below review comments 

  • The View Data menu option should be on the Object menu, which should mirror the Context menu, except options should be disabled when not applicable instead of hidden. Note: - With current implementation "Object" menu is re-created dynamically depending on the node clicked, so we will have to re-create the "View Data" menu as well and it require change in the generic code. For the time being "View Data" menu is visible in "Object" menu when appropriate node is selected.       
  • Please merge the functionality of the Refresh and Execute buttons into one button. We shouldn't have two buttons that do essentially the same thing.
  • In Edit Grid mode, that textbox should be read-only, but should display the SQL used (including any LIMIT/FILTER clauses). Please refer the attached screenshot (Modified-Data-Grid). 
  • Please adjust the height of the Delete icon in the Edit Grid, such that it doesn't force the row height to be higher than it should be.
  • I think the names of the tabs are far too long. Can we change them to "Query 1", "Query 2" etc, then rename them to the filename if the user saves/loads a file? Note: - As discussed I have added one more container to show the title. Please refer the attached screenshots (Modified-Query-Tool) 
  • Query results should have spaces converted to "&nbsp;", so that proper indenting is maintained (for example, on EXPLAIN queries)
    • To fix this we have added css style "white-space: pre-wrap;", but it changes the backgrid cell size. Please refer the screenshot (Modified-Data-Grid). 
   Please review the screenshots and please let me know will it looks good.

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

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



--
Akshay Joshi
Principal Software Engineer 


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246
Вложения

Re: [pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool

От
Dave Page
Дата:


On Fri, Apr 8, 2016 at 3:39 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Dave

On Fri, Apr 8, 2016 at 2:32 PM, Dave Page <dpage@pgadmin.org> wrote:


On Fri, Apr 8, 2016 at 7:43 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

- The Query Tool menu icon should be a glyphicon, to match the others.
 
   There is no glyphicon available which match the Query Tool icon. I have found one like below which is "database-search" or can you please suggest some other icon. 
       Inline image 1 

That one looks perfect.

           We can't use this icon because it's not come with Bootstrap , I have picked this from "http://glyphicons.com/" and I am not sure we can use it as per the Licence.

At the risk of annoying everyone immensely, on reflection I'm thinking we should use Font Awesome as our default generic icons, and fall back to Bootstrap's Glyphicons. I really hadn't realised how much larger the FA set is.

For this particular issue, could we use FA's stacking? e.g. something like: https://jsfiddle.net/pa8x6nt3/ 

If not, how about using the execute icon we discussed, e.g. fa-bolt?

   I have used fa-bolt. Apart from that I have completed below review comments 

OK.
 

  • The View Data menu option should be on the Object menu, which should mirror the Context menu, except options should be disabled when not applicable instead of hidden. Note: - With current implementation "Object" menu is re-created dynamically depending on the node clicked, so we will have to re-create the "View Data" menu as well and it require change in the generic code. For the time being "View Data" menu is visible in "Object" menu when appropriate node is selected.       
  • Please merge the functionality of the Refresh and Execute buttons into one button. We shouldn't have two buttons that do essentially the same thing.
  • In Edit Grid mode, that textbox should be read-only, but should display the SQL used (including any LIMIT/FILTER clauses). Please refer the attached screenshot (Modified-Data-Grid). 
  • Please adjust the height of the Delete icon in the Edit Grid, such that it doesn't force the row height to be higher than it should be.
  • I think the names of the tabs are far too long. Can we change them to "Query 1", "Query 2" etc, then rename them to the filename if the user saves/loads a file? Note: - As discussed I have added one more container to show the title. Please refer the attached screenshots (Modified-Query-Tool) 
  • Query results should have spaces converted to "&nbsp;", so that proper indenting is maintained (for example, on EXPLAIN queries)
    • To fix this we have added css style "white-space: pre-wrap;", but it changes the backgrid cell size. Please refer the screenshot (Modified-Data-Grid). 
Hmmm. I'd rather that the heights didn't change (or at least the rows were resizeable like in pga3. Can we set the column to hide vertical overflow, unless it gets focus or something?
 
   Please review the screenshots and please let me know will it looks good.

Bar the cell size, certainly it's good :-) 

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Вложения

Re: [pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool

От
Dave Page
Дата:
Very nice!

On Tue, Apr 12, 2016 at 1:11 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Dave

As per suggestion, I have changed History tab to be a grid. Attached is the two screenshot one for Query Tool and other is for the Data Grid where some of the transaction gets rollback while saving the data due to one of the error condition. Please review the screenshots and let me know the comments if any.   

On Fri, Apr 8, 2016 at 9:49 PM, Dave Page <dpage@pgadmin.org> wrote:


On Fri, Apr 8, 2016 at 3:39 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Dave

On Fri, Apr 8, 2016 at 2:32 PM, Dave Page <dpage@pgadmin.org> wrote:


On Fri, Apr 8, 2016 at 7:43 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

- The Query Tool menu icon should be a glyphicon, to match the others.
 
   There is no glyphicon available which match the Query Tool icon. I have found one like below which is "database-search" or can you please suggest some other icon. 
       Inline image 1 

That one looks perfect.

           We can't use this icon because it's not come with Bootstrap , I have picked this from "http://glyphicons.com/" and I am not sure we can use it as per the Licence.

At the risk of annoying everyone immensely, on reflection I'm thinking we should use Font Awesome as our default generic icons, and fall back to Bootstrap's Glyphicons. I really hadn't realised how much larger the FA set is.

For this particular issue, could we use FA's stacking? e.g. something like: https://jsfiddle.net/pa8x6nt3/ 

If not, how about using the execute icon we discussed, e.g. fa-bolt?

   I have used fa-bolt. Apart from that I have completed below review comments 

OK.
 

  • The View Data menu option should be on the Object menu, which should mirror the Context menu, except options should be disabled when not applicable instead of hidden. Note: - With current implementation "Object" menu is re-created dynamically depending on the node clicked, so we will have to re-create the "View Data" menu as well and it require change in the generic code. For the time being "View Data" menu is visible in "Object" menu when appropriate node is selected.       
  • Please merge the functionality of the Refresh and Execute buttons into one button. We shouldn't have two buttons that do essentially the same thing.
  • In Edit Grid mode, that textbox should be read-only, but should display the SQL used (including any LIMIT/FILTER clauses). Please refer the attached screenshot (Modified-Data-Grid). 
  • Please adjust the height of the Delete icon in the Edit Grid, such that it doesn't force the row height to be higher than it should be.
  • I think the names of the tabs are far too long. Can we change them to "Query 1", "Query 2" etc, then rename them to the filename if the user saves/loads a file? Note: - As discussed I have added one more container to show the title. Please refer the attached screenshots (Modified-Query-Tool) 
  • Query results should have spaces converted to "&nbsp;", so that proper indenting is maintained (for example, on EXPLAIN queries)
    • To fix this we have added css style "white-space: pre-wrap;", but it changes the backgrid cell size. Please refer the screenshot (Modified-Data-Grid). 
Hmmm. I'd rather that the heights didn't change (or at least the rows were resizeable like in pga3. Can we set the column to hide vertical overflow, unless it gets focus or something?
 
   Please review the screenshots and please let me know will it looks good.

Bar the cell size, certainly it's good :-) 

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

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



--
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
Вложения

Re: [pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool

От
Akshay Joshi
Дата:
Hi Dave

As per suggestion, I have changed History tab to be a grid. Attached is the two screenshot one for Query Tool and other is for the Data Grid where some of the transaction gets rollback while saving the data due to one of the error condition. Please review the screenshots and let me know the comments if any.   

On Fri, Apr 8, 2016 at 9:49 PM, Dave Page <dpage@pgadmin.org> wrote:


On Fri, Apr 8, 2016 at 3:39 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Dave

On Fri, Apr 8, 2016 at 2:32 PM, Dave Page <dpage@pgadmin.org> wrote:


On Fri, Apr 8, 2016 at 7:43 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

- The Query Tool menu icon should be a glyphicon, to match the others.
 
   There is no glyphicon available which match the Query Tool icon. I have found one like below which is "database-search" or can you please suggest some other icon. 
       Inline image 1 

That one looks perfect.

           We can't use this icon because it's not come with Bootstrap , I have picked this from "http://glyphicons.com/" and I am not sure we can use it as per the Licence.

At the risk of annoying everyone immensely, on reflection I'm thinking we should use Font Awesome as our default generic icons, and fall back to Bootstrap's Glyphicons. I really hadn't realised how much larger the FA set is.

For this particular issue, could we use FA's stacking? e.g. something like: https://jsfiddle.net/pa8x6nt3/ 

If not, how about using the execute icon we discussed, e.g. fa-bolt?

   I have used fa-bolt. Apart from that I have completed below review comments 

OK.
 

  • The View Data menu option should be on the Object menu, which should mirror the Context menu, except options should be disabled when not applicable instead of hidden. Note: - With current implementation "Object" menu is re-created dynamically depending on the node clicked, so we will have to re-create the "View Data" menu as well and it require change in the generic code. For the time being "View Data" menu is visible in "Object" menu when appropriate node is selected.       
  • Please merge the functionality of the Refresh and Execute buttons into one button. We shouldn't have two buttons that do essentially the same thing.
  • In Edit Grid mode, that textbox should be read-only, but should display the SQL used (including any LIMIT/FILTER clauses). Please refer the attached screenshot (Modified-Data-Grid). 
  • Please adjust the height of the Delete icon in the Edit Grid, such that it doesn't force the row height to be higher than it should be.
  • I think the names of the tabs are far too long. Can we change them to "Query 1", "Query 2" etc, then rename them to the filename if the user saves/loads a file? Note: - As discussed I have added one more container to show the title. Please refer the attached screenshots (Modified-Query-Tool) 
  • Query results should have spaces converted to "&nbsp;", so that proper indenting is maintained (for example, on EXPLAIN queries)
    • To fix this we have added css style "white-space: pre-wrap;", but it changes the backgrid cell size. Please refer the screenshot (Modified-Data-Grid). 
Hmmm. I'd rather that the heights didn't change (or at least the rows were resizeable like in pga3. Can we set the column to hide vertical overflow, unless it gets focus or something?
 
   Please review the screenshots and please let me know will it looks good.

Bar the cell size, certainly it's good :-) 

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

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



--
Akshay Joshi
Principal Software Engineer 


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246
Вложения

Re: [pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool

От
Akshay Joshi
Дата:
Hi All 

I have fixed review comments given by Dave and couple of them are remaining

Fixed Review Comments:
  • The View Data menu option should be on the Object menu, which should mirror the Context menu, except options should be disabled when not applicable instead of hidden.
  • The Query Tool menu icon should be a glyphicon, to match the others.
  • Please merge the functionality of the Refresh and Execute buttons into one button. We shouldn't have two buttons that do essentially the same thing.
  • In Edit Grid mode, the History panel should log all queries (SELECTs, UPDATEs, DELETEs etc) as it would in the Query Tool.
  • In Edit Grid mode, the Messages panel should display any messages from the most recent action  as it would in the Query Tool.
  • In Edit Grid mode, that textbox should be read-only, but should display the SQL used (including any LIMIT/FILTER clauses)
  • Please remove the border from the SQL box, such that it fills all available space.
  • The Filter box should be in a modal overlay over the top of the SQL box/Results tabs as required. Those elements should be grayed whilst it is open.
  • Please adjust the height of the Delete icon in the Edit Grid, such that it doesn't force the row height to be higher than it should be.
  • I think the names of the tabs are far too long. Can we change them to "Query 1", "Query 2" etc, then rename them to the filename if the user saves/loads a file?
  • We should add an "Edit" button, which opens a drop-down menu. This would eventually include options as found on the Edit menu in the pgAdmin3 query tool, such as the "Clear SQL" option.
  • Ashesh and I discussed changing the History tab to be a grid, showing: Date/Time, Query (first line only), Rows affected, Runtime and Status, in a row per query executed. Ashesh suggested using a sub-form that can be expanded for each row, which could show the full query and error details (SQL State etc). New rows should be added to the top of the list.
  • Errors should be highlighted in the SQL box - a marker in the margin to note the line, and spellcheck-style underlining for the error word.
  • Query results should have spaces converted to "&nbsp;", so that proper indenting is maintained (for example, on EXPLAIN queries).
  • on shutdown pgAdmin server , appropriate message should be displayed.
Remaining review comments:
  • Please add an SQL button. This should show/hide the SQL panel in *both* Query Tool and Edit Grid modes. 
  • If a field has been edited, but not saved, can we highlight it somehow? Maybe make the text dark blue?
  • The "Add Row" button only works if you're on the last page of the resultset.
  • Can the "Copy Row" button also populate the clipboard with CSV data for the row?
  • In Edit mode, we need to be able to represent/set values to NULL.
  • The layout of the result tabs should be maintained if new Query Tool or Edit Grid tabs are opened.
TODO's (apart from above)
  • Open/Save SQL file.
  • Cut, Copy, Paste functionality.  

 Attached is the patch file which contains complete DataGrid and Query Tool code with fixed review comments. Can you please review the patch and let me know the review comments if any else can you please commit the code if it looks good to you. 

On Tue, Apr 12, 2016 at 6:10 PM, Dave Page <dpage@pgadmin.org> wrote:
Very nice!


On Tue, Apr 12, 2016 at 1:11 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Dave

As per suggestion, I have changed History tab to be a grid. Attached is the two screenshot one for Query Tool and other is for the Data Grid where some of the transaction gets rollback while saving the data due to one of the error condition. Please review the screenshots and let me know the comments if any.   

On Fri, Apr 8, 2016 at 9:49 PM, Dave Page <dpage@pgadmin.org> wrote:


On Fri, Apr 8, 2016 at 3:39 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Dave

On Fri, Apr 8, 2016 at 2:32 PM, Dave Page <dpage@pgadmin.org> wrote:


On Fri, Apr 8, 2016 at 7:43 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

- The Query Tool menu icon should be a glyphicon, to match the others.
 
   There is no glyphicon available which match the Query Tool icon. I have found one like below which is "database-search" or can you please suggest some other icon. 
       Inline image 1 

That one looks perfect.

           We can't use this icon because it's not come with Bootstrap , I have picked this from "http://glyphicons.com/" and I am not sure we can use it as per the Licence.

At the risk of annoying everyone immensely, on reflection I'm thinking we should use Font Awesome as our default generic icons, and fall back to Bootstrap's Glyphicons. I really hadn't realised how much larger the FA set is.

For this particular issue, could we use FA's stacking? e.g. something like: https://jsfiddle.net/pa8x6nt3/ 

If not, how about using the execute icon we discussed, e.g. fa-bolt?

   I have used fa-bolt. Apart from that I have completed below review comments 

OK.
 

  • The View Data menu option should be on the Object menu, which should mirror the Context menu, except options should be disabled when not applicable instead of hidden. Note: - With current implementation "Object" menu is re-created dynamically depending on the node clicked, so we will have to re-create the "View Data" menu as well and it require change in the generic code. For the time being "View Data" menu is visible in "Object" menu when appropriate node is selected.       
  • Please merge the functionality of the Refresh and Execute buttons into one button. We shouldn't have two buttons that do essentially the same thing.
  • In Edit Grid mode, that textbox should be read-only, but should display the SQL used (including any LIMIT/FILTER clauses). Please refer the attached screenshot (Modified-Data-Grid). 
  • Please adjust the height of the Delete icon in the Edit Grid, such that it doesn't force the row height to be higher than it should be.
  • I think the names of the tabs are far too long. Can we change them to "Query 1", "Query 2" etc, then rename them to the filename if the user saves/loads a file? Note: - As discussed I have added one more container to show the title. Please refer the attached screenshots (Modified-Query-Tool) 
  • Query results should have spaces converted to "&nbsp;", so that proper indenting is maintained (for example, on EXPLAIN queries)
    • To fix this we have added css style "white-space: pre-wrap;", but it changes the backgrid cell size. Please refer the screenshot (Modified-Data-Grid). 
Hmmm. I'd rather that the heights didn't change (or at least the rows were resizeable like in pga3. Can we set the column to hide vertical overflow, unless it gets focus or something?
 
   Please review the screenshots and please let me know will it looks good.

Bar the cell size, certainly it's good :-) 

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

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



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



--
Akshay Joshi
Principal Software Engineer 


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246
Вложения

Re: [pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool

От
Dave Page
Дата:
Hi

On Thu, Apr 14, 2016 at 1:58 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi All 

I have fixed review comments given by Dave and couple of them are remaining

Fixed Review Comments:
  • The View Data menu option should be on the Object menu, which should mirror the Context menu, except options should be disabled when not applicable instead of hidden.
  • The Query Tool menu icon should be a glyphicon, to match the others.
  • Please merge the functionality of the Refresh and Execute buttons into one button. We shouldn't have two buttons that do essentially the same thing.
  • In Edit Grid mode, the History panel should log all queries (SELECTs, UPDATEs, DELETEs etc) as it would in the Query Tool.
  • In Edit Grid mode, the Messages panel should display any messages from the most recent action  as it would in the Query Tool.
  • In Edit Grid mode, that textbox should be read-only, but should display the SQL used (including any LIMIT/FILTER clauses)
  • Please remove the border from the SQL box, such that it fills all available space.
  • The Filter box should be in a modal overlay over the top of the SQL box/Results tabs as required. Those elements should be grayed whilst it is open.
  • Please adjust the height of the Delete icon in the Edit Grid, such that it doesn't force the row height to be higher than it should be.
  • I think the names of the tabs are far too long. Can we change them to "Query 1", "Query 2" etc, then rename them to the filename if the user saves/loads a file?
  • We should add an "Edit" button, which opens a drop-down menu. This would eventually include options as found on the Edit menu in the pgAdmin3 query tool, such as the "Clear SQL" option.
  • Ashesh and I discussed changing the History tab to be a grid, showing: Date/Time, Query (first line only), Rows affected, Runtime and Status, in a row per query executed. Ashesh suggested using a sub-form that can be expanded for each row, which could show the full query and error details (SQL State etc). New rows should be added to the top of the list.
  • Errors should be highlighted in the SQL box - a marker in the margin to note the line, and spellcheck-style underlining for the error word.
  • Query results should have spaces converted to "&nbsp;", so that proper indenting is maintained (for example, on EXPLAIN queries).
  • on shutdown pgAdmin server , appropriate message should be displayed.

Awesome!

I've made a few minor style changes - mostly colouring, but I also widened the Execute button and it was easier to push it's dropdown than it - and pushed the code.
 
Remaining review comments:
  • Please add an SQL button. This should show/hide the SQL panel in *both* Query Tool and Edit Grid modes. 
  • If a field has been edited, but not saved, can we highlight it somehow? Maybe make the text dark blue?
  • The "Add Row" button only works if you're on the last page of the resultset.
  • Can the "Copy Row" button also populate the clipboard with CSV data for the row?
  • In Edit mode, we need to be able to represent/set values to NULL.
  • The layout of the result tabs should be maintained if new Query Tool or Edit Grid tabs are opened.
TODO's (apart from above)
  • Open/Save SQL file.
  • Cut, Copy, Paste functionality. 
Agreed on the above.

My only real suggestion on the code itself (which looks good and clean on a quick review), is that:

- The button bar be moved out into an HTML template
- create.sql should perhaps be renamed to insert.sql
- It looks like we only allow updates if we have a pkey. Should we allow use of OIDs when present, if a pkey isn't there?

I'll continue to log additional issues if/when I find them.
 
--
Dave Page
VP, Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

Re: [pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool

От
Akshay Joshi
Дата:


On Thu, Apr 14, 2016 at 7:40 PM, Dave Page <dave.page@enterprisedb.com> wrote:
Hi

On Thu, Apr 14, 2016 at 1:58 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi All 

I have fixed review comments given by Dave and couple of them are remaining

Fixed Review Comments:
  • The View Data menu option should be on the Object menu, which should mirror the Context menu, except options should be disabled when not applicable instead of hidden.
  • The Query Tool menu icon should be a glyphicon, to match the others.
  • Please merge the functionality of the Refresh and Execute buttons into one button. We shouldn't have two buttons that do essentially the same thing.
  • In Edit Grid mode, the History panel should log all queries (SELECTs, UPDATEs, DELETEs etc) as it would in the Query Tool.
  • In Edit Grid mode, the Messages panel should display any messages from the most recent action  as it would in the Query Tool.
  • In Edit Grid mode, that textbox should be read-only, but should display the SQL used (including any LIMIT/FILTER clauses)
  • Please remove the border from the SQL box, such that it fills all available space.
  • The Filter box should be in a modal overlay over the top of the SQL box/Results tabs as required. Those elements should be grayed whilst it is open.
  • Please adjust the height of the Delete icon in the Edit Grid, such that it doesn't force the row height to be higher than it should be.
  • I think the names of the tabs are far too long. Can we change them to "Query 1", "Query 2" etc, then rename them to the filename if the user saves/loads a file?
  • We should add an "Edit" button, which opens a drop-down menu. This would eventually include options as found on the Edit menu in the pgAdmin3 query tool, such as the "Clear SQL" option.
  • Ashesh and I discussed changing the History tab to be a grid, showing: Date/Time, Query (first line only), Rows affected, Runtime and Status, in a row per query executed. Ashesh suggested using a sub-form that can be expanded for each row, which could show the full query and error details (SQL State etc). New rows should be added to the top of the list.
  • Errors should be highlighted in the SQL box - a marker in the margin to note the line, and spellcheck-style underlining for the error word.
  • Query results should have spaces converted to "&nbsp;", so that proper indenting is maintained (for example, on EXPLAIN queries).
  • on shutdown pgAdmin server , appropriate message should be displayed.

Awesome!

I've made a few minor style changes - mostly colouring, but I also widened the Execute button and it was easier to push it's dropdown than it - and pushed the code.
 
Remaining review comments:
  • Please add an SQL button. This should show/hide the SQL panel in *both* Query Tool and Edit Grid modes. 
  • If a field has been edited, but not saved, can we highlight it somehow? Maybe make the text dark blue?
  • The "Add Row" button only works if you're on the last page of the resultset.
  • Can the "Copy Row" button also populate the clipboard with CSV data for the row?
  • In Edit mode, we need to be able to represent/set values to NULL.
  • The layout of the result tabs should be maintained if new Query Tool or Edit Grid tabs are opened.
TODO's (apart from above)
  • Open/Save SQL file.
  • Cut, Copy, Paste functionality. 
Agreed on the above.

My only real suggestion on the code itself (which looks good and clean on a quick review), is that:

- The button bar be moved out into an HTML template
- create.sql should perhaps be renamed to insert.sql
- It looks like we only allow updates if we have a pkey. Should we allow use of OIDs when present, if a pkey isn't there?

I'll continue to log additional issues if/when I find them.

   Thanks for committing the patch. I'll work on the above comments. Meanwhile I have found one issue where "View Filtered Row" is not working, so attached is the patch file to fix that. Can you please review/commit it.
 
--
Dave Page
VP, Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake



--
Akshay Joshi
Principal Software Engineer 


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246
Вложения

Re: [pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool

От
Dave Page
Дата:
Thanks - committed.

On Fri, Apr 15, 2016 at 8:03 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:


On Thu, Apr 14, 2016 at 7:40 PM, Dave Page <dave.page@enterprisedb.com> wrote:
Hi

On Thu, Apr 14, 2016 at 1:58 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi All 

I have fixed review comments given by Dave and couple of them are remaining

Fixed Review Comments:
  • The View Data menu option should be on the Object menu, which should mirror the Context menu, except options should be disabled when not applicable instead of hidden.
  • The Query Tool menu icon should be a glyphicon, to match the others.
  • Please merge the functionality of the Refresh and Execute buttons into one button. We shouldn't have two buttons that do essentially the same thing.
  • In Edit Grid mode, the History panel should log all queries (SELECTs, UPDATEs, DELETEs etc) as it would in the Query Tool.
  • In Edit Grid mode, the Messages panel should display any messages from the most recent action  as it would in the Query Tool.
  • In Edit Grid mode, that textbox should be read-only, but should display the SQL used (including any LIMIT/FILTER clauses)
  • Please remove the border from the SQL box, such that it fills all available space.
  • The Filter box should be in a modal overlay over the top of the SQL box/Results tabs as required. Those elements should be grayed whilst it is open.
  • Please adjust the height of the Delete icon in the Edit Grid, such that it doesn't force the row height to be higher than it should be.
  • I think the names of the tabs are far too long. Can we change them to "Query 1", "Query 2" etc, then rename them to the filename if the user saves/loads a file?
  • We should add an "Edit" button, which opens a drop-down menu. This would eventually include options as found on the Edit menu in the pgAdmin3 query tool, such as the "Clear SQL" option.
  • Ashesh and I discussed changing the History tab to be a grid, showing: Date/Time, Query (first line only), Rows affected, Runtime and Status, in a row per query executed. Ashesh suggested using a sub-form that can be expanded for each row, which could show the full query and error details (SQL State etc). New rows should be added to the top of the list.
  • Errors should be highlighted in the SQL box - a marker in the margin to note the line, and spellcheck-style underlining for the error word.
  • Query results should have spaces converted to "&nbsp;", so that proper indenting is maintained (for example, on EXPLAIN queries).
  • on shutdown pgAdmin server , appropriate message should be displayed.

Awesome!

I've made a few minor style changes - mostly colouring, but I also widened the Execute button and it was easier to push it's dropdown than it - and pushed the code.
 
Remaining review comments:
  • Please add an SQL button. This should show/hide the SQL panel in *both* Query Tool and Edit Grid modes. 
  • If a field has been edited, but not saved, can we highlight it somehow? Maybe make the text dark blue?
  • The "Add Row" button only works if you're on the last page of the resultset.
  • Can the "Copy Row" button also populate the clipboard with CSV data for the row?
  • In Edit mode, we need to be able to represent/set values to NULL.
  • The layout of the result tabs should be maintained if new Query Tool or Edit Grid tabs are opened.
TODO's (apart from above)
  • Open/Save SQL file.
  • Cut, Copy, Paste functionality. 
Agreed on the above.

My only real suggestion on the code itself (which looks good and clean on a quick review), is that:

- The button bar be moved out into an HTML template
- create.sql should perhaps be renamed to insert.sql
- It looks like we only allow updates if we have a pkey. Should we allow use of OIDs when present, if a pkey isn't there?

I'll continue to log additional issues if/when I find them.

   Thanks for committing the patch. I'll work on the above comments. Meanwhile I have found one issue where "View Filtered Row" is not working, so attached is the patch file to fix that. Can you please review/commit it.
 
--
Dave Page
VP, Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake



--
Akshay Joshi
Principal Software Engineer 


Phone: +91 20-3058-9517
Mobile: +91 976-788-8246



--
Dave Page
VP, Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

Re: [pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool

От
Akshay Joshi
Дата:
Hi 

I have fixed following review comments given by Dave and attached is the patch file:
  • The button bar be moved out into an HTML template
  • create.sql should perhaps be renamed to insert.sql
  • The "Add Row" button only works if you're on the last page of the resultset.
I have tried to fix the following review comments, but unable to figure out the solution
    • Please add an SQL button. This should show/hide the SQL panel in *both* Query Tool and Edit Grid modes. 
               I have changed the design and have only one wcDocker and add SQL panel to the Top and remaining panel to the Bottom, then I have tried to hide the top panel, but not able to fix it.   
    • If a field has been edited, but not saved, can we highlight it somehow? Maybe make the text dark blue?
               For this we have "backgrid:edited" event in which we get model and column, but not sure how we can find the cell to add the class to change the colour and if we will have solution then, we will have to remove that class again once user will enter the original value again.   
    • Can the "Copy Row" button also populate the clipboard with CSV data for the row?
    • Cut, Copy and Paste to Clipboard. 
               I have googled for solution and found some of them which have only "Copy" feature, some of them works for limited browsers. One solution is to use "document.execCommand("Copy")" but it works for TextArea and we have CodeMirror. 
    • The layout of the result tabs should be maintained if new Query Tool or Edit Grid tabs are opened.
              To solve the above I need some help/suggestions here. 


              On Fri, Apr 15, 2016 at 1:22 PM, Dave Page <dave.page@enterprisedb.com> wrote:
              Thanks - committed.

              On Fri, Apr 15, 2016 at 8:03 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:


              On Thu, Apr 14, 2016 at 7:40 PM, Dave Page <dave.page@enterprisedb.com> wrote:
              Hi

              On Thu, Apr 14, 2016 at 1:58 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
              Hi All 

              I have fixed review comments given by Dave and couple of them are remaining

              Fixed Review Comments:
              • The View Data menu option should be on the Object menu, which should mirror the Context menu, except options should be disabled when not applicable instead of hidden.
              • The Query Tool menu icon should be a glyphicon, to match the others.
              • Please merge the functionality of the Refresh and Execute buttons into one button. We shouldn't have two buttons that do essentially the same thing.
              • In Edit Grid mode, the History panel should log all queries (SELECTs, UPDATEs, DELETEs etc) as it would in the Query Tool.
              • In Edit Grid mode, the Messages panel should display any messages from the most recent action  as it would in the Query Tool.
              • In Edit Grid mode, that textbox should be read-only, but should display the SQL used (including any LIMIT/FILTER clauses)
              • Please remove the border from the SQL box, such that it fills all available space.
              • The Filter box should be in a modal overlay over the top of the SQL box/Results tabs as required. Those elements should be grayed whilst it is open.
              • Please adjust the height of the Delete icon in the Edit Grid, such that it doesn't force the row height to be higher than it should be.
              • I think the names of the tabs are far too long. Can we change them to "Query 1", "Query 2" etc, then rename them to the filename if the user saves/loads a file?
              • We should add an "Edit" button, which opens a drop-down menu. This would eventually include options as found on the Edit menu in the pgAdmin3 query tool, such as the "Clear SQL" option.
              • Ashesh and I discussed changing the History tab to be a grid, showing: Date/Time, Query (first line only), Rows affected, Runtime and Status, in a row per query executed. Ashesh suggested using a sub-form that can be expanded for each row, which could show the full query and error details (SQL State etc). New rows should be added to the top of the list.
              • Errors should be highlighted in the SQL box - a marker in the margin to note the line, and spellcheck-style underlining for the error word.
              • Query results should have spaces converted to "&nbsp;", so that proper indenting is maintained (for example, on EXPLAIN queries).
              • on shutdown pgAdmin server , appropriate message should be displayed.

              Awesome!

              I've made a few minor style changes - mostly colouring, but I also widened the Execute button and it was easier to push it's dropdown than it - and pushed the code.
               
              Remaining review comments:
              • Please add an SQL button. This should show/hide the SQL panel in *both* Query Tool and Edit Grid modes. 
              • If a field has been edited, but not saved, can we highlight it somehow? Maybe make the text dark blue?
              • The "Add Row" button only works if you're on the last page of the resultset.
              • Can the "Copy Row" button also populate the clipboard with CSV data for the row?
              • In Edit mode, we need to be able to represent/set values to NULL.
              • The layout of the result tabs should be maintained if new Query Tool or Edit Grid tabs are opened.
              TODO's (apart from above)
              • Open/Save SQL file.
              • Cut, Copy, Paste functionality. 
              Agreed on the above.

              My only real suggestion on the code itself (which looks good and clean on a quick review), is that:

              - The button bar be moved out into an HTML template
              - create.sql should perhaps be renamed to insert.sql
              - It looks like we only allow updates if we have a pkey. Should we allow use of OIDs when present, if a pkey isn't there?

              I'll continue to log additional issues if/when I find them.

                 Thanks for committing the patch. I'll work on the above comments. Meanwhile I have found one issue where "View Filtered Row" is not working, so attached is the patch file to fix that. Can you please review/commit it.
               
              --
              Dave Page
              VP, Chief Architect, Tools & Installers
              EnterpriseDB: http://www.enterprisedb.com
              The Enterprise PostgreSQL Company

              Blog: http://pgsnake.blogspot.com
              Twitter: @pgsnake



              --
              Akshay Joshi
              Principal Software Engineer 


              Phone: +91 20-3058-9517
              Mobile: +91 976-788-8246



              --
              Dave Page
              VP, Chief Architect, Tools & Installers
              EnterpriseDB: http://www.enterprisedb.com
              The Enterprise PostgreSQL Company

              Blog: http://pgsnake.blogspot.com
              Twitter: @pgsnake



              --
              Akshay Joshi
              Principal Software Engineer 


              Phone: +91 20-3058-9517
              Mobile: +91 976-788-8246
              Вложения

              Re: [pgAdmin4] [Patch] Implementation of the Data Grid and Query Tool

              От
              Akshay Joshi
              Дата:
              Hi All

              Added support of code-folding for QueryTool. Please ignore previous patch, attached is the new combined patch.

              On Thu, Apr 21, 2016 at 2:00 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
              Hi 

              I have fixed following review comments given by Dave and attached is the patch file:
              • The button bar be moved out into an HTML template
              • create.sql should perhaps be renamed to insert.sql
              • The "Add Row" button only works if you're on the last page of the resultset.
              I have tried to fix the following review comments, but unable to figure out the solution
                • Please add an SQL button. This should show/hide the SQL panel in *both* Query Tool and Edit Grid modes. 
                           I have changed the design and have only one wcDocker and add SQL panel to the Top and remaining panel to the Bottom, then I have tried to hide the top panel, but not able to fix it.   
                • If a field has been edited, but not saved, can we highlight it somehow? Maybe make the text dark blue?
                           For this we have "backgrid:edited" event in which we get model and column, but not sure how we can find the cell to add the class to change the colour and if we will have solution then, we will have to remove that class again once user will enter the original value again.   
                • Can the "Copy Row" button also populate the clipboard with CSV data for the row?
                • Cut, Copy and Paste to Clipboard. 
                           I have googled for solution and found some of them which have only "Copy" feature, some of them works for limited browsers. One solution is to use "document.execCommand("Copy")" but it works for TextArea and we have CodeMirror. 
                • The layout of the result tabs should be maintained if new Query Tool or Edit Grid tabs are opened.
                          To solve the above I need some help/suggestions here. 


                          On Fri, Apr 15, 2016 at 1:22 PM, Dave Page <dave.page@enterprisedb.com> wrote:
                          Thanks - committed.

                          On Fri, Apr 15, 2016 at 8:03 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:


                          On Thu, Apr 14, 2016 at 7:40 PM, Dave Page <dave.page@enterprisedb.com> wrote:
                          Hi

                          On Thu, Apr 14, 2016 at 1:58 PM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
                          Hi All 

                          I have fixed review comments given by Dave and couple of them are remaining

                          Fixed Review Comments:
                          • The View Data menu option should be on the Object menu, which should mirror the Context menu, except options should be disabled when not applicable instead of hidden.
                          • The Query Tool menu icon should be a glyphicon, to match the others.
                          • Please merge the functionality of the Refresh and Execute buttons into one button. We shouldn't have two buttons that do essentially the same thing.
                          • In Edit Grid mode, the History panel should log all queries (SELECTs, UPDATEs, DELETEs etc) as it would in the Query Tool.
                          • In Edit Grid mode, the Messages panel should display any messages from the most recent action  as it would in the Query Tool.
                          • In Edit Grid mode, that textbox should be read-only, but should display the SQL used (including any LIMIT/FILTER clauses)
                          • Please remove the border from the SQL box, such that it fills all available space.
                          • The Filter box should be in a modal overlay over the top of the SQL box/Results tabs as required. Those elements should be grayed whilst it is open.
                          • Please adjust the height of the Delete icon in the Edit Grid, such that it doesn't force the row height to be higher than it should be.
                          • I think the names of the tabs are far too long. Can we change them to "Query 1", "Query 2" etc, then rename them to the filename if the user saves/loads a file?
                          • We should add an "Edit" button, which opens a drop-down menu. This would eventually include options as found on the Edit menu in the pgAdmin3 query tool, such as the "Clear SQL" option.
                          • Ashesh and I discussed changing the History tab to be a grid, showing: Date/Time, Query (first line only), Rows affected, Runtime and Status, in a row per query executed. Ashesh suggested using a sub-form that can be expanded for each row, which could show the full query and error details (SQL State etc). New rows should be added to the top of the list.
                          • Errors should be highlighted in the SQL box - a marker in the margin to note the line, and spellcheck-style underlining for the error word.
                          • Query results should have spaces converted to "&nbsp;", so that proper indenting is maintained (for example, on EXPLAIN queries).
                          • on shutdown pgAdmin server , appropriate message should be displayed.

                          Awesome!

                          I've made a few minor style changes - mostly colouring, but I also widened the Execute button and it was easier to push it's dropdown than it - and pushed the code.
                           
                          Remaining review comments:
                          • Please add an SQL button. This should show/hide the SQL panel in *both* Query Tool and Edit Grid modes. 
                          • If a field has been edited, but not saved, can we highlight it somehow? Maybe make the text dark blue?
                          • The "Add Row" button only works if you're on the last page of the resultset.
                          • Can the "Copy Row" button also populate the clipboard with CSV data for the row?
                          • In Edit mode, we need to be able to represent/set values to NULL.
                          • The layout of the result tabs should be maintained if new Query Tool or Edit Grid tabs are opened.
                          TODO's (apart from above)
                          • Open/Save SQL file.
                          • Cut, Copy, Paste functionality. 
                          Agreed on the above.

                          My only real suggestion on the code itself (which looks good and clean on a quick review), is that:

                          - The button bar be moved out into an HTML template
                          - create.sql should perhaps be renamed to insert.sql
                          - It looks like we only allow updates if we have a pkey. Should we allow use of OIDs when present, if a pkey isn't there?

                          I'll continue to log additional issues if/when I find them.

                             Thanks for committing the patch. I'll work on the above comments. Meanwhile I have found one issue where "View Filtered Row" is not working, so attached is the patch file to fix that. Can you please review/commit it.
                           
                          --
                          Dave Page
                          VP, Chief Architect, Tools & Installers
                          EnterpriseDB: http://www.enterprisedb.com
                          The Enterprise PostgreSQL Company

                          Blog: http://pgsnake.blogspot.com
                          Twitter: @pgsnake



                          --
                          Akshay Joshi
                          Principal Software Engineer 


                          Phone: +91 20-3058-9517
                          Mobile: +91 976-788-8246



                          --
                          Dave Page
                          VP, Chief Architect, Tools & Installers
                          EnterpriseDB: http://www.enterprisedb.com
                          The Enterprise PostgreSQL Company

                          Blog: http://pgsnake.blogspot.com
                          Twitter: @pgsnake



                          --
                          Akshay Joshi
                          Principal Software Engineer 


                          Phone: +91 20-3058-9517
                          Mobile: +91 976-788-8246



                          --
                          Akshay Joshi
                          Principal Software Engineer 


                          Phone: +91 20-3058-9517
                          Mobile: +91 976-788-8246
                          Вложения