Re: [pgAdmin][RM6131] Port query tool to React

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin][RM6131] Port query tool to React
Date: 2022-04-18 07:24:21
Message-ID: CANxoLDeKDRYY3Woq13MxOrL9Wcfkz5533iFtdLJqwZaDoCA4cA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks, the patch applied.

On Mon, Apr 18, 2022 at 11:27 AM Aditya Toshniwal <
aditya(dot)toshniwal(at)enterprisedb(dot)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(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>
>> Hi,
>>
>> Please find an updated patch with PEP8 issues fixed.
>>
>> On Thu, Apr 7, 2022 at 3:12 PM Aditya Toshniwal <
>> aditya(dot)toshniwal(at)enterprisedb(dot)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(dot)toshniwal(at)enterprisedb(dot)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(dot)joshi(at)enterprisedb(dot)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
>>>>> - [image: Screenshot 2022-03-16 at 7.02.34 PM.png]
>>>>>
>>>>> 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
>>>>> - [image: Screenshot 2022-03-17 at 11.29.21 AM.png] [image:
>>>>> Screenshot 2022-03-17 at 11.29.59 AM.png]
>>>>>
>>>>> 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.
>>>>> - [image: Screenshot 2022-03-17 at 1.43.01 PM.png].
>>>>> [image: Screenshot 2022-03-17 at 1.44.10 PM.png]
>>>>> .
>>>>> -
>>>>> - [image: Screenshot 2022-03-17 at 1.44.53 PM.png]
>>>>> [image: Screenshot 2022-03-17 at 1.45.54 PM.png]
>>>>>
>>>>> 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(dot)toshniwal(at)enterprisedb(dot)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(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Please find the attached patch :)
>>>>>>>
>>>>>>> On Wed, Mar 16, 2022 at 12:16 PM Akshay Joshi <
>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Hi Aditya
>>>>>>>>
>>>>>>>> I think you forgot to attach the patch.
>>>>>>>>
>>>>>>>> On Tue, Mar 15, 2022 at 4:00 PM Aditya Toshniwal <
>>>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)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.
>>>>>>>>>
>>>>>>>>> [image: image.png]
>>>>>>>>> --
>>>>>>>>> Thanks,
>>>>>>>>> Aditya Toshniwal
>>>>>>>>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>>>>>>>>> <http://edbpostgres.com>
>>>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> *Thanks & Regards*
>>>>>>>> *Akshay Joshi*
>>>>>>>> *pgAdmin Hacker | Principal Software Architect*
>>>>>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>>>>>
>>>>>>>> *Mobile: +91 976-788-8246*
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Thanks,
>>>>>>> Aditya Toshniwal
>>>>>>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>>>>>>> <http://edbpostgres.com>
>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Thanks,
>>>>>> Aditya Toshniwal
>>>>>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>>>>>> <http://edbpostgres.com>
>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> *Thanks & Regards*
>>>>> *Akshay Joshi*
>>>>> *pgAdmin Hacker | Principal Software Architect*
>>>>> *EDB Postgres <http://edbpostgres.com>*
>>>>>
>>>>> *Mobile: +91 976-788-8246*
>>>>>
>>>>
>>>>
>>>> --
>>>> Thanks,
>>>> Aditya Toshniwal
>>>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>>>> <http://edbpostgres.com>
>>>> "Don't Complain about Heat, Plant a TREE"
>>>>
>>>
>>>
>>> --
>>> Thanks,
>>> Aditya Toshniwal
>>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>>> <http://edbpostgres.com>
>>> "Don't Complain about Heat, Plant a TREE"
>>>
>>
>>
>> --
>> Thanks,
>> Aditya Toshniwal
>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>> <http://edbpostgres.com>
>> "Don't Complain about Heat, Plant a TREE"
>>
>
>
> --
> Thanks,
> Aditya Toshniwal
> pgAdmin Hacker | Software Architect | *edbpostgres.com*
> <http://edbpostgres.com>
> "Don't Complain about Heat, Plant a TREE"
>

--
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Principal Software Architect*
*EDB Postgres <http://edbpostgres.com>*

*Mobile: +91 976-788-8246*

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Khushboo Vashi 2022-04-18 08:26:10 Re: [pgAdmin4][Patch] - RM #7179 - PostgreSQL deployment on EDB BigAnimal
Previous Message Akshay Joshi 2022-04-18 07:24:03 pgAdmin 4 commit: Fixed following issues for query tool after react por