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.
I have saved the current state of the tree. so If I have disconnected the server, so that closed state will be considered for that server.
If another server(s) is still connected and opened at some node, and then you reload the browser, on connecting/expanding the server it will populate the server nodes.
@Dave,
Do I need to change this behaviour?
Per our Skype call, let's store and restore both the current tree state and the previous state of disconnected servers (as pgAdmin 3 did). So:
- On browser open, the treeview is restored to the previous state, with databases reconnected as required.
- On re-connect of a database, the sub-treeview for that database is restored to the state it was in prior to disconnection.
Thanks!
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).
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.