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

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"