Re: [pgAdmin4][Patch]: RM 1253 - Store and reload current location in treeview

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
Cc: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch]: RM 1253 - Store and reload current location in treeview
Date: 2018-08-27 07:49:20
Message-ID: CANxoLDf9vyLRjLQ+ehxXgMbkSLK0QYO18dNT=jWS9C2vq+cyLg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Khushboo

Patch looks good to me, following are my review comments:

- Logic doesn't work when we disconnect and re-connect to the database
server. We have duplicated RM #1335.
- Instead of -1 we should use 0 to stop the tree saving functionality.
Send request at 0 second doesn't make any sense.
- What if pgAdmin4 opens in two different browsers or two different tabs
with different tree hierarchy in that case it will override the state of
each other. (This could be an enhancement request).

On Mon, Aug 27, 2018 at 11:09 AM, Aditya Toshniwal <
aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:

> Hi Khushboo,
>
> Cheers, the patch looks good to me. I have few suggestions:
> 1) Expand the tree -> Go to server properties and rename the server -> the
> tree closes. It should expand to last state. Same applies to other nodes
> which can be renamed.
> 2) It would be nice if we can also save the last selected object. The tree
> expands but the last selected object is not selected.
> 3) In /web/pgadmin/static/js/tree/pgadmin_tree_save_state.js
> import pgAdmin from '../../../static/js/pgadmin'; -- please use alias
> here rather than relative path.
> 4) I would suggest to add 1 example/format JSON of tree state in comments
> in the file so that it will be easier to relate the code with.
>
>
> On Fri, Aug 24, 2018 at 10:08 AM Khushboo Vashi <
> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>> Hi,
>>
>> Please find the attached patch for RM #1253 - Store and reload current
>> location in treeview
>>
>> Feature Details:
>> - The current tree state will be stored in the sqlite database.
>> - The time interval to store the tree state is configurable via
>> preferences and the default is 30 secs.
>> -1 can be used to stop the tree saving functionality,
>> - On window unload the current tree state will be saved.
>> - On Reload, while opening the server, the stored path will be populated.
>> - On closing the node the saved will be updated accordingly.
>> - Jasmine test cases are included.
>>
>>
>> Thanks,
>> Khushboo
>>
>
>
> --
> Thanks and Regards,
> Aditya Toshniwal
> Software Engineer | EnterpriseDB Software Solutions | Pune
> "Don't Complain about Heat, Plant a tree"
>

--
*Akshay Joshi*

*Sr. Software Architect *

*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Aditya Toshniwal 2018-08-27 08:58:44 Re: [pgAdmin4][Patch]: RM #3420 Merge latest code of pgcli with pgAdmin4
Previous Message Aditya Toshniwal 2018-08-27 05:39:04 Re: [pgAdmin4][Patch]: RM 1253 - Store and reload current location in treeview