Re: [pgAdmin][RM6131] Port query tool to React - Mailing list pgadmin-hackers

Hi,

Please find the rebased patch.

On Fri, Apr 29, 2022 at 3:40 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Hi,

Please ignore the previous patch, attached is the updated one.

On Fri, Apr 29, 2022 at 1:12 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Hi,

Attached patch fixes:

1. Filter dialog doesn't work in edit mode (getting back end errors), reverting it back to create mode of SchemaView.

2. Enable the copy button when a cell is selected.

3. Exclude, Include related fixes.

Please review.

On Thu, Apr 28, 2022 at 6:02 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Thanks, the patch applied.

On Thu, Apr 28, 2022 at 5:53 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Hi,

Attached patch disables the "Save results" button when there are no rows.

On Tue, Apr 26, 2022 at 4:30 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Thanks, the patch applied.

On Tue, Apr 26, 2022 at 4:23 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Please ignore the previous patch. Attached is the new one.

Fixes:

1. In Dark mode > Replace/Find dialogue forward & reverse buttons are stuck to each other.
   
2. In Dark/High contrast mode > Checkbox is not visible for false value.
   
3. Wrap toolbar when size goes very small.
4. Replace functionality does not work when tried in sequence 2 times. Codemirror search is not cyclic. So, changes are made to always search from the start.
   
5. Replace all does not work when tried in sequence 2 times. Same reason as above.
6. Fix broken macros $SELECTION$ feature.
7. Make query history SQL readonly.
8. The Filter dialog save button should be disabled when opened.

On Tue, Apr 26, 2022 at 3:05 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Hi,

Attached path fixes:

1. In Dark mode > Replace/Find dialogue forward & reverse buttons are stuck to each other.
   
2. In Dark/High contrast mode > Checkbox is not visible for false value.
   
3. Wrap toolbar when size goes very small.
4. Replace functionality does not work when tried in sequence 2 times. Codemirror search is not cyclic. So, changes are made to always search from the start.
   
5. Replace all does not work when tried in sequence 2 times. Same reason as above.
6. Fix broken macros $SELECTION$ feature.
7. Make query history SQL readonly.

On Mon, Apr 25, 2022 at 6:13 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Thanks, the patch applied.

On Mon, Apr 25, 2022 at 6:07 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Attached path fixes:

1. Failed to fetch query history error sometimes.
   
2. In copy paste row, if a copied row has [null], then those are pasted as empty string.
   
3. When Dataoutput is empty, show an empty grid.
   
4. Schema diff generate script button results in empty window. Fixes #7306.
   
5. Detach the DataOutput panel > Try editing text cell > Text editor is hidden behind data output panel

On Fri, Apr 22, 2022 at 6:18 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Thanks, the patch applied

On Fri, Apr 22, 2022 at 6:02 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Hi,

Attached patch fixes:

1. Add min width to panels.

2. Fix issues related to New connection in query tool. Also fixed some existing bugs related to this.

Please review.

On Wed, Apr 20, 2022 at 7:04 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Thanks, the patch applied.

On Wed, Apr 20, 2022 at 6:31 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Attached patch fixes following issues:

1. Find/Replace both opens the same dialogue box.(by clicking menu option)
2. Add New Server Connection > Server options keep loading(For multiple Server groups & should have some server)
3. Fixed CSS issues of slickgrid at various places.
4. Try to edit cell with char varying data type(which opens text editor, leave editor open) > Scroll result grid verticall so that ediot disappaears > Click on another cell > query edior shows white screen & refresh is the only option left.(TypeError: Cannot read properties of null (reading 'querySelector’) - Issue is not reproducible if you scroll horizontally
   
5. C should be captial in ’<New connection…>'
   
6. In pop title for New Connection, all words should be capital.(Add new connection)
   
7. Explain > Analaysis tab > Column heading missing ROWS PLAN with cost & In explain only.
   
8. Explain > Analaysis tab > with cost enabled > Upward arrow size does not match with font of number. Arrow is little bigger than number.
   
9. Boolean default is not considered while ading new row.(try table from feature test defaults)
   
10. In query history , when not query history present, warning icon size big. Match it to warning message - No history found
    
11. Select table/db object > Open query tool from Tools menu > NOT FOUND error is shown. Existing issue, fixed.
    
12. Any cell just open by clicking it > Do NOT change any thing > Click Ok > Cell is shown as edited.

Please review.

On Mon, Apr 18, 2022 at 12:54 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Thanks, the patch applied.

On Mon, Apr 18, 2022 at 11:27 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Hi,

Attached is the patch to fix the issues raised. Few of them are pending and will send it later.

Fixed:

1. Add New Server Connection > Server options keep loading(For empty Server group). 
2. After clicking indent/Unindent(for all operations) for large query option left as it is till operation completes
   
3. Also check sign beside options in Execute Option/Copy Header is little bit big
   
4. In explain > Analysis tab does not show ROWS column
   
5. In explain > Explain > analysis previous explain output is NOT cleared. New rows are appended. Same applies to the statistics tab.
   
6. Update new query tool connection tool tip.(7289)
   
7. Explain-Analyze > Loops column is empty.
   
8. Explain-Analyze with Verbose & Costs > in ROW X columns upward arrows are missing.
   
9. Explain-Analyze with all option checked > background colors are missing for timing.
   
10. Explain-Analyze > Additional bullet is added before Hash Cond.
    
11. Browser Tree > Filtered rows icon is not working.
    
12. Create table with timestamp and default value as function now() > Add new row > Enter mandatory columns except column where default value is function(now()) > Click Save > New row added but column with default value has value [default]. not updated to actual value. / Default values are not considered for any column while adding a new entry.
    
13. Disable execute options in View/Edit data.
    
14. The Boolean column always shows null.
    
15. In Query history Remove & Remove all buttons are stuck to each other.
    
16. On Remove all, the right panel is empty.
    
17. Create a column with boolean[]/ text[], Try to add a new entry from data grid, enter “” quotes > Click Ok > Now try edit cell > You can not change value.
    
18. In query history - Select queries are suffixed by ’Save Data’ icon
    
19. Edit any table with PK > Try to insert duplicate PK > Error thrown > Correct pK value > Still old error shown > Not able to add new entry (This works when focus is moved from edited cell)
    
20. Clicking arrows after opening dropdown options, does not collapse dropdown.
    

I was not able to reproduce some of the bugs on webpack dev mode, but reproducible on webpack prod mode bundles. After a lot of debugging it turned out that webpack/babel transpile was changing the meaning of a piece of code in prod mode. I tweaked the code then most issues were not reproducible anymore. 

That said, following issues were not reproducible and this fix could be the reason:

1. Not able to load more than 1000 rows.
   
2. Find/Replace both opens the same dialogue box.
   
3. Try to edit cell with char varying data type(which opens text editor) > Scroll result grid > Click on another cell > query edior shows white screen & refresh is the only option left.(TypeError: Cannot read properties of null (reading 'querySelector')
   at getCellElement (sqleditor.js?ver=60800:1:995456))
4. Generate script is not working for schema diff for tables with target only/ not working for any.(TypeError: Cannot read properties of undefined (reading 'database'))
   
5. Query results are appended in the Notification tab.
   
6. Panel name is NOT updated on opening file. Panel-name should be filename
   
7. Open a file in query tool > Open another file > Check panel name > It is the first file name.
   
8. Incorrect CSV downloaded (film table) when CSV quotes select single quote from preferences. *CSV generated at backend. No changes done.*
9. In Data grid > Add New data to cell > without clicking on other cell click on Add New row > previous data is gone.
   

Please find the comment inline for other issues:

1. Small white line is added below Total rows status bar.
   This is an existing issue with wcDocker. It is somehow not getting the correct size for the query tool. You can verify this by opening query tool in new tab.
2. In explain > Data output > Query Plan is editable.
   To be precise, the JSON editor is editable but does not allow saving it. This is inline with other editors like text editor which allows editing but no save.
3. Color is NOT fainted in View/Edit data when query tool is NOT editable.
   This is inline with other places where the SQL is read only like the properties dialog SQL tab or the RE-SQL tab. Plus, greying out of SQL affects query readability.
4. If data in result grid is edited & changes are reverted, then also Save button remain enabled/ Cell is shown in bold indicating data is edited.
   This is based on existing behaviour. A separate RM can be raised to have any improvement in this.
5. When the Save button is disabled then 'Save as' should be disabled as well.
   Save and Save As are different in behaviour. You can change a file and save it. The save will be disabled but the user should be allowed. I also checked the behaviour of VS-Code and PyCharm. They never disable the "save as" button. After all, there is no harm in allowing a user to save as even if it is empty.
6. Manage Macros - Help button is disabled. Remove SQL help button(Not sure).
   As I already mentioned in the review by Askhay, the existing help button opens the query tool help. Query tool help is already added on the toolbar and so this one is disabled.
7. Macros defined in one database are shown for other databases also/even across servers.
   As per the existing design.
8. Query tool notifier setting is missing in preferences.
   Previously, the total time and number of rows were shown in the notifier. And so, the notifier setting was added so that users can tweak it to keep it open for a longer time. Now, we do not show those details on the notifier since we have a fixed status bar for that. This setting is not relevant anymore.

Issues that need to be checked and pending:

1. Explain-Analyze with all option checked > Statistics tab > % of query is always 0 for node type. Need to check all the calculations.
   
2. In the Result grid multiple rows can not be selected with shift + down arrow.
   
3. In Geometry Viewer , map disappears if taken to bottom.
   
4. Keyboard shortcut - Focus in query tool and try Previous/Next tab is Not working add quotes in query tool
   
5. Keyboard shortcut Switch Panel is not working

On Thu, Apr 7, 2022 at 3:37 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Hi,

Please find an updated patch with PEP8 issues fixed.

On Thu, Apr 7, 2022 at 3:12 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Hi Hackers,

Attached is updated patch which now also includes:

Can't copy and paste row correctly if first column contains no data #7294

On Tue, Apr 5, 2022 at 5:45 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Hi Akshay,

Thank you for doing such a detailed review. Please find my comments inline below and attached patch.

On Thu, Mar 17, 2022 at 4:05 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Hi Aditya

Following are the review comments:

GUI:

• The Maximize/Minimize button on the panel should be consistent with other panels.

Fixed. 

• Scratch Pad is missing or is there any new setting to add a scratch pad?

Added. The layout lib currently does not have a context menu on the header to add a panel. For now, you can use the reset layout button to add the scratch pad again if closed. Context menu can be added separately later. Reset layout will not refresh the page.

• Press "Cmd + G" on Query Tool it opens the old search bar is it still valid
• 

Fixed.  

• In case of any error, we should move the cursor to the error location. We are highlighting the error row but it should be scroll to that location in the editor.

Existing behaviour. Improvement done. 

• Error highlighting color should be aligned with the theme, check the existing color.

Fixed. 

• Error highlighting color is not cleared when running the successful query. Run "SELECT * from pg_class123" and then run "SELECT * from pg_class".

Fixed.  

• ToolBar buttons:
  • When the Save button is disabled then 'Save as' should be disabled as well.

This is not correct. A user should be allowed to "Save as" a file even if it is saved and save button is disabled. 

• Open any SQL file, change some text and click on the 'Save' button, No notifier message has been flashed that 'File saved successfully' and the button does not get disabled as well.

Fixed. 

• Format SQL not working.

Fixed. 

• Clear Query keyboard shortcut not working.
  

Fixed.  

• Clicking on the 'Clear Query' menu should pop up a confirmation dialog 'Are you sure you wish to discard the current changes?'

Fixed.  

• Open any SQL file, change some text, and try to open another file, it should pop up a confirmation dialog 'Are you sure you wish to discard the current changes?'

Fixed.   

• When clicking on the New Query Tool button it is not opening the new query tool window.

Fixed.   

• Shortcut Key for Replace is not correct on Windows (Tooltip showing Alt + Ctrl + F) but actual is (Shift + Ctrl + F)

Fixed.   

• Most of the Keyboard shortcuts are not working on Windows at all.

Fixed.   

• New Connection Dialog:
  • The close button should be right-aligned.

Fixed.    

• Unable to test further because when selecting any disconnected server it should pop up the password dialog to connect and then fetch the details like databases, users, roles.

Fixed.    

• Help buttons are disabled.

The existing help button opens the query tool help. Query tool help is already added on the toolbar.

• Query History:
  • For some queries like 'ROLLBACK' and 'COMMIT,' Rows affected shows in the negative (-1).

Based on existing. I have made it blank. 

• Remove All should warn the user before removing everything "Are you sure you wish to remove all the history? This will remove all of your query histories from this and other sessions for this database."

Fixed.   

• Duration is missing. It should be there with the Date and Rows affected.

Fixed.   

• Status Bar:
  • We should display Milliseconds as well in Query Complete.

It will now display in <hr>:<min>:<sec>.<msec> format. Fixed.

• Total Rows: N of N, I always observe the same, can you please change it to N only, Or am I missing some scenario where it is changed?

We're fetching rows on demand. "X of N" says total X rows fetched till now but total N are available.

• Data output:
  • Default message "No data output......" should be shown when there are no rows/data, check the old behavior.

Fixed.  

• Extra space in the JSON Editor, even if I resize it, its height is not adjusted. Check the second screenshot
•              
  

Fixed.   

• Explain:
  • Old content should be cleared from the panel if we run the new query by clicking the play button. It should show an informative message, check the old behavior.

Fixed.   

• Macros:
  • The close button should be right-aligned.

Fixed.   

• Help buttons are disabled.

The existing help button opens the query tool help. Query tool help is already added on the toolbar. 

• View/Edit Data:
  
  • Clipboard issues.

Fixed.    

• Unable to edit table data even if the primary key is defined.

Fixed. 

• Select All rows is missing at the left corner of the 'Data Output' Panel.

Fixed.  

• Change some data and try to save if an error comes then Spinner is not cleared.

Fixed.  

•  Filtered Rows not working. Click on the 'Filtered Rows ...' context menu.

Fixed.  

• Sort/Filter Dialog is missing.

The sort/filter dialog is present and opens when you click the filter button. I have removed the "Sort/Filter" menu item which does the same thing.

• Select any cell of the table content, click on any filter menu 'Filter by selection' or 'Exclude by selection' multiple times. It updates the query every time and adds the condition which is not there previously.

Fixed.   

• Limit (100 rows, 500 rows ...) not working. View data of any table having 1000+ rows and then apply the limit.

I am not able to reproduce this. Works fine. 

• Themes:
  • Check and Fix all the issues related to the theme.
  • .                            .
  •       
  •                              
    

Fixed.   

Code:

• Fix pep8 issues.

Fixed.   

• Can we fix the Deprecation Warnings?

I think those are coming from bootstrap. Need to check that separately. Not related to the query tool. 

Note: Code review still remains, meanwhile you can start fixing the above issues.

Forgot to mention in initial mail - I have removed dependency on Snap.svg and have written Explain SVG codes from scratch.

On Wed, Mar 16, 2022 at 5:54 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Hi,

Attached is an updated one with few more improvements and fixes.

On Wed, Mar 16, 2022 at 1:40 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Please find the attached patch :)

On Wed, Mar 16, 2022 at 12:16 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Hi Aditya

I think you forgot to attach the patch.

On Tue, Mar 15, 2022 at 4:00 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Hi Hackers,

Attached is the initial patch that migrates the SQL Editor tool to React based. Change highlights:
1. Complete rewrite to React code.
2. UI improvements based on suggestions and requests.
3. Work towards stability and performance improvement.
4. Keep row numbers in view when scrolling horizontally. Fixes #3989
5. Fixed status bar at the bottom with useful details. Fixes #3253
6. Relocate GIS Viewer Button to the Left Side of Results Table. Fixed #6830
7. Allow to remove single history records. Refs #4113
8. Macros usability improvements. Ref #6969
9. Connection bar visibility issue. Fixes #7188
10. Query tool layout issues. Fixes #6725

Please note, there are still few minor niggles at some places but the patch qualified to be reviewed. We will need a good amount of time to test this properly. So, I am sending the feature patch. JS test cases and documentation patches will follow soon. 

Please review.

--

Thanks,

Aditya Toshniwal

pgAdmin Hacker | Software Architect | edbpostgres.com

"Don't Complain about Heat, Plant a TREE"

--

Thanks & Regards

Akshay Joshi

pgAdmin Hacker | Principal Software Architect

EDB Postgres

Mobile: +91 976-788-8246

--

Thanks,

Aditya Toshniwal

pgAdmin Hacker | Software Architect | edbpostgres.com

"Don't Complain about Heat, Plant a TREE"

--

Thanks,

Aditya Toshniwal

pgAdmin Hacker | Software Architect | edbpostgres.com

"Don't Complain about Heat, Plant a TREE"

--

Thanks & Regards

Akshay Joshi

pgAdmin Hacker | Principal Software Architect

EDB Postgres

Mobile: +91 976-788-8246

--

Thanks,

Aditya Toshniwal

pgAdmin Hacker | Software Architect | edbpostgres.com

"Don't Complain about Heat, Plant a TREE"

--

Thanks,

Aditya Toshniwal

pgAdmin Hacker | Software Architect | edbpostgres.com

"Don't Complain about Heat, Plant a TREE"

--

Thanks,

Aditya Toshniwal

pgAdmin Hacker | Software Architect | edbpostgres.com

"Don't Complain about Heat, Plant a TREE"

--

Thanks,

Aditya Toshniwal

pgAdmin Hacker | Software Architect | edbpostgres.com

"Don't Complain about Heat, Plant a TREE"

--

Thanks & Regards

Akshay Joshi

pgAdmin Hacker | Principal Software Architect

EDB Postgres

Mobile: +91 976-788-8246

--

Thanks,

Aditya Toshniwal

pgAdmin Hacker | Software Architect | edbpostgres.com

"Don't Complain about Heat, Plant a TREE"

--

Thanks & Regards

Akshay Joshi

pgAdmin Hacker | Principal Software Architect

EDB Postgres

Mobile: +91 976-788-8246

--

Thanks,

Aditya Toshniwal

pgAdmin Hacker | Software Architect | edbpostgres.com

"Don't Complain about Heat, Plant a TREE"

--

Thanks & Regards

Akshay Joshi

pgAdmin Hacker | Principal Software Architect

EDB Postgres

Mobile: +91 976-788-8246

--

Thanks,

Aditya Toshniwal

pgAdmin Hacker | Software Architect | edbpostgres.com

"Don't Complain about Heat, Plant a TREE"

--

Thanks & Regards

Akshay Joshi

pgAdmin Hacker | Principal Software Architect

EDB Postgres

Mobile: +91 976-788-8246

--

Thanks,

Aditya Toshniwal

pgAdmin Hacker | Software Architect | edbpostgres.com

"Don't Complain about Heat, Plant a TREE"

--

Thanks,

Aditya Toshniwal

pgAdmin Hacker | Software Architect | edbpostgres.com

"Don't Complain about Heat, Plant a TREE"

--

Thanks & Regards

Akshay Joshi

pgAdmin Hacker | Principal Software Architect

EDB Postgres

Mobile: +91 976-788-8246

--

Thanks,

Aditya Toshniwal

pgAdmin Hacker | Software Architect | edbpostgres.com

"Don't Complain about Heat, Plant a TREE"

--

Thanks & Regards

Akshay Joshi

pgAdmin Hacker | Principal Software Architect

EDB Postgres

Mobile: +91 976-788-8246

--

Thanks,

Aditya Toshniwal

pgAdmin Hacker | Software Architect | edbpostgres.com

"Don't Complain about Heat, Plant a TREE"

--

Thanks,

Aditya Toshniwal

pgAdmin Hacker | Software Architect | edbpostgres.com

"Don't Complain about Heat, Plant a TREE"

--

Thanks,

Aditya Toshniwal

pgAdmin Hacker | Software Architect | edbpostgres.com

"Don't Complain about Heat, Plant a TREE"