Re: [pgAdmin4][Patch] - RM #6129 - Port browser tree to React

From: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
To: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch] - RM #6129 - Port browser tree to React
Date: 2021-09-24 05:51:51
Message-ID: CAFOhELfdHLZwZkAAOfs7VsE4aK7Y2xM1ufBvNwjOtHbdYQgOfg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi,

Please find the attached updated patch.
All the review comments are fixed except one as below.

Thanks,
Khushboo

On Fri, Sep 17, 2021 at 8:35 PM Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
wrote:

> Hi Khushboo
>
> Following are the GUI review comments:
>
> - Arrow ( > ) is not center-aligned with the node labels.
> - Sorting of tree items is not correct when you have servers like
> (PostgreSQL 9.5, PostgreSQL 9.6, PostgreSQL 10...)
>
> The sorting of the tree is alphabetically, so, PG 10 will come first than
PG 9.5.

>
> - Servers collection node not showing count while expanding it.
> - The last child should not have a collapse/expand arrow ( >).
> - Properties panel should not be rendered again and again when the
> same tree object is collapsed/expanded. I have tried on Schema node.
> - The labels should be properly quoted. Create a Cast
> *"money->bigint", *in properties dialog it is showing correctly, but
> in browser tree, it is showing "*money-&gt;bigint*".
> - Mouse hover any node, we will see the URL as a tooltip.
> - Open query tool "*tree_data.slice is not a function" *error showing
> in the developer tools. File: sqleditor.js:2392
> - *node.getTreeNodeHierarchy is not a function* error visible. File:
> dependents.js and dependencies.js:167. Please check the same function in
> the whole source code.
> - Select "*Postgres*" database or server node and refresh the node
> using the context menu, after the refresh, it will show the number of
> children which should not. The same behavior observes for each node.
> - Lable "Group Roles" should be "Login/Group Roles".
> - When scrolling at the bottom, the scroll bar automatically moves up.
> Steps to reproduce Connect to one server and expand till table node, then
> scroll down and connect to another server, when we try to scroll down
> completely, the scroll bar automatically moves up.
> - The following menu options are not working on the respective node:
> - *Server: *Reload Configuration, Clear Saved Password, and Add
> Named Restore Point
> - *Database*: Maintenence (*item.getMetadata is not a function
> tree.js line 710*), Grant Wizard, PSQL Tool (Please select a server
> or database object)
> - *Table*: Count Rows, Reset Statistics, Import/Export,
> Maintenence, Truncate
> - *Partitions*: Detach Partition,
> - *Check Constraint*: Validate check constraint
> - *Constraints*: Context menu not opening
> - *MView*: Context menu not opening:- Cannot read properties of
> undefined (reading 'apply') mview.js line no 395
>
> *Please check all the other Menu items if I missed them. *
>
> Code Review:
>
> - Remove commented code. Remove "console.warn" if used for testing
> purposes.
> - Remove if (m.name == 'create_table') console.warn(m); } browser.js
> line no 797
>
> Code review still remains.
>
> On Wed, Sep 15, 2021 at 12:10 PM Khushboo Vashi <
> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>> Hi,
>>
>> Please find the attached patch for the RM #6129 - Port browser tree to
>> React.
>>
>> Thanks,
>> Khushboo
>>
>>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
> *pgAdmin Hacker | Principal Software Architect*
> *EDB Postgres <http://edbpostgres.com>*
>
> *Mobile: +91 976-788-8246*
>

Attachment Content-Type Size
RM_6129_v1.patch application/octet-stream 296.5 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2021-09-24 08:02:18 pgAdmin 4 commit: Fixed html help for foreign server dialog.
Previous Message Akshay Joshi 2021-09-23 14:00:22 Re: [pgAdmin][RM6741]: Cast node issues