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

Hi Aditya

Following are the review comments:

*GUI:*

- The Maximize/Minimize button on the panel should be consistent with
other panels.
- Scratch Pad is missing or is there any new setting to add a scratch
pad?
- 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]
- 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.
- Error highlighting color should be aligned with the theme, check the
existing color.
- Error highlighting color is not cleared when running the successful
query. Run "SELECT * from pg_class123" and then run "SELECT * from
pg_class".

- *ToolBar buttons*:
- When the Save button is disabled then 'Save as' should be disabled
as well.
- 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.
- Format SQL not working.
- Clear Query keyboard shortcut not working.
- Clicking on the 'Clear Query' menu should pop up a confirmation
dialog 'Are you sure you wish to discard the current changes?'
- 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?'
- When clicking on the New Query Tool button it is not opening the
new query tool window.
- Shortcut Key for Replace is not correct on *Windows* (Tooltip
showing Alt + Ctrl + F) but actual is (Shift + Ctrl + F)
- Most of the Keyboard shortcuts are not working on Windows at all.
- *New Connection Dialog:*
- The close button should be right-aligned.
- 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.
- Help buttons are disabled.
- *Query History*:
- For some queries like 'ROLLBACK' and 'COMMIT,' Rows affected shows
in the negative (-1).
- *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."
- Duration is missing. It should be there with the Date and Rows
affected.
- *Status Bar*:
- We should display Milliseconds as well in Query Complete.
- 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?
- *Data output*:
- Default message "No data output......" should be shown when there
are no rows/data, check the old behavior.
- 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]
- *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.
- *Macros*:
- The close button should be right-aligned.
- Help buttons are disabled.
- *View/Edit Data*:
- Clipboard issues.
- Unable to edit table data even if the primary key is defined.
- Select All rows is missing at the left corner of the 'Data Output'
Panel.
- Change some data and try to save if an error comes then Spinner is
not cleared.
- Filtered Rows not working. Click on the 'Filtered Rows ...'
context menu.
- Sort/Filter Dialog is missing.
- 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.
- Limit (100 rows, 500 rows ...) not working. View data of any table
having 1000+ rows and then apply the limit.
- *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]

*Code:*

- Fix pep8 issues.
- Can we fix the *Deprecation Warnings?*

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

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*