public inbox for [email protected]help / color / mirror / Atom feed
Pains and thoughts about refactoring the Tree Menu using React 4+ messages / 2 participants [nested] [flat]
* Pains and thoughts about refactoring the Tree Menu using React @ 2017-04-21 18:47 Joao Pedro De Almeida Pereira <[email protected]> 0 siblings, 2 replies; 4+ messages in thread From: Joao Pedro De Almeida Pereira @ 2017-04-21 18:47 UTC (permalink / raw) To: pgadmin-hackers Hi Hackers, After a conversation with Dave we believe that we need to provide more context on our pains and what we propose as a first step for implementing the Tree Menu using React. The pain: Here is a quick description of what we think we understand and where we got stuck while doing our refactor. If you look at the above image, both the NodeJS and CollectionJS classes are templates. Our goal was to extract those from being templates so that we could test them. We started with the generate_url function. The issue we ran into was that the generate_url function is inherited by CollectionJS from NodeJS but then over writes some of the functionality that it inherits. In order to have one function that our react component would delegate to, we have to consolidate these two class methods. This turned out to be a non-trivial refactor due to the lack of tests/documentation. Our proposal: This diagram represents what we believe can be our first approach to updating the tree. We can create a very basic tree that only does tree stuff (Open Node, Close Nodes) and then delegates to other adapters to execute all the business logic functionality (Generate URLs to get the node, Right Click menu, …) Asks: - Are there any more operation that currently are being triggered by the tree? If so we need to add them to the Adapter List. - Because we lack the context and knowledge of the current tree implementation, we need your help to extract these methods from the places they are currently in. We believe that a good place to start would be writing tests for and implementing the generation of URLs used to fetch the child nodes. Attached you can find an example of the JavaScript tests that we have been writing that can be applied to this extracted method. Thanks Rob, Oliver & Joao ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Pains and thoughts about refactoring the Tree Menu using React @ 2017-04-21 18:52 Joao Pedro De Almeida Pereira <[email protected]> parent: Joao Pedro De Almeida Pereira <[email protected]> 1 sibling, 0 replies; 4+ messages in thread From: Joao Pedro De Almeida Pereira @ 2017-04-21 18:52 UTC (permalink / raw) To: pgadmin-hackers Hi Hackers, The attachment was missing in the previous email, so there you go. Note: We had to remove the extension of the file due to security reasons, just add a .js Thanks Joao & Oliver On Fri, Apr 21, 2017 at 2:47 PM, Joao Pedro De Almeida Pereira < [email protected]> wrote: > Hi Hackers, > > After a conversation with Dave we believe that we need to provide more > context on our pains and what we propose as a first step for implementing > the Tree Menu using React. > > The pain: > > Here is a quick description of what we think we understand and where we > got stuck while doing our refactor. > > If you look at the above image, both the NodeJS and CollectionJS classes > are templates. Our goal was to extract those from being templates so that > we could test them. We started with the generate_url function. The issue we > ran into was that the generate_url function is inherited by CollectionJS > from NodeJS but then over writes some of the functionality that it > inherits. In order to have one function that our react component would > delegate to, we have to consolidate these two class methods. This turned > out to be a non-trivial refactor due to the lack of tests/documentation. > > Our proposal: > > > This diagram represents what we believe can be our first approach to > updating the tree. We can create a very basic tree that only does tree > stuff (Open Node, Close Nodes) and then delegates to other adapters to > execute all the business logic functionality (Generate URLs to get the > node, Right Click menu, …) > > Asks: > > - > > Are there any more operation that currently are being triggered by the > tree? If so we need to add them to the Adapter List. > - > > Because we lack the context and knowledge of the current tree > implementation, we need your help to extract these methods from the places > they are currently in. We believe that a good place to start would be > writing tests for and implementing the generation of URLs used to fetch the > child nodes. > > Attached you can find an example of the JavaScript tests that we have been > writing that can be applied to this extracted method. > > > Thanks > > Rob, Oliver & Joao > > -- Sent via pgadmin-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers Attachments: [application/octet-stream] node_url_generator_spec (2.0K, 3-node_url_generator_spec) download ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Pains and thoughts about refactoring the Tree Menu using React @ 2017-04-28 09:51 Ashesh Vashi <[email protected]> parent: Joao Pedro De Almeida Pereira <[email protected]> 1 sibling, 1 reply; 4+ messages in thread From: Ashesh Vashi @ 2017-04-28 09:51 UTC (permalink / raw) To: Joao Pedro De Almeida Pereira <[email protected]>; +Cc: pgadmin-hackers On Sat, Apr 22, 2017 at 12:17 AM, Joao Pedro De Almeida Pereira < [email protected]> wrote: > Hi Hackers, > > After a conversation with Dave we believe that we need to provide more > context on our pains and what we propose as a first step for implementing > the Tree Menu using React. > > The pain: > > Here is a quick description of what we think we understand and where we > got stuck while doing our refactor. > > If you look at the above image, both the NodeJS and CollectionJS classes > are templates. Our goal was to extract those from being templates so that > we could test them. We started with the generate_url function. The issue we > ran into was that the generate_url function is inherited by CollectionJS > from NodeJS but then over writes some of the functionality that it > inherits. In order to have one function that our react component would > delegate to, we have to consolidate these two class methods. This turned > out to be a non-trivial refactor due to the lack of tests/documentation. > > Our proposal: > > > This diagram represents what we believe can be our first approach to > updating the tree. We can create a very basic tree that only does tree > stuff (Open Node, Close Nodes) and then delegates to other adapters to > execute all the business logic functionality (Generate URLs to get the > node, Right Click menu, …) > We will loose the current extendible architecture of by creating fix number of adapters, instead - we should generates events like right now, let the modules/extensions decide - what to do. There are many operations are dependents on the tree events. e.g. - Before open When server node is being opened, 'beforeopen' event checks if server is connected, or not. If not try to connect it, otherwise it does not let it open it - Added Dependent modules libraries are loaded, when a particular type of node is added. i.e. When the first database node is added in the tree, it loads all the schema level, and other javascript modules. - Selected When any of the node is added, it shows properties of the selected, change the dashboard context (if necessary). Current tree generates many events. beforeload, beforeappend, added, appended, loaded, init, beforeselect, selected, beforefocus, focused, focus, blurred, beforetoggle, beforeopen, openned, toggled, etc. We generate 'pgadmin-browser:tree' events on a pgBrowser.Events objects to be used by different modules. At the moment, 'pgadmin-browser:tree:selected' is widely used by different modules. i.e. SQL, dependents, dependencies, properties, Dashboard, etc. If you want to see the complete list of events generated, you can use the following patch. *diff --git a/web/pgadmin/browser/static/js/datamodel.js b/web/pgadmin/browser/static/js/datamodel.js* *index 5b1c3a7..575c78d 100644* *--- a/web/pgadmin/browser/static/js/datamodel.js* *+++ b/web/pgadmin/browser/static/js/datamodel.js* *@@ -1149,5 +1149,10 @@ function(_, pgAdmin, $, Backbone) {* * pgBrowser.Events = _.extend({}, Backbone.Events);* *+ pgBrowser.Events.on(* *+ 'pgadmin-browser:tree', function() {* *+ console.log(arguments[0]);* *+ });* *+* * return pgBrowser;* * });* Apart from them, we also need to understand, how the tree traversal will work. How would you traverse through a particular node, and expand all its parent? > Asks: > > - > > Are there any more operation that currently are being triggered by the > tree? If so we need to add them to the Adapter List. > > Please see my above comments. > > - > > Because we lack the context and knowledge of the current tree > implementation, we need your help to extract these methods from the places > they are currently in. We believe that a good place to start would be > writing tests for and implementing the generation of URLs used to fetch the > child nodes. > > Attached you can find an example of the JavaScript tests that we have been > writing that can be applied to this extracted method. > You may want to start from here: https://www.pgadmin.org/docs4/dev/code_snippets.html#nodeview Every tree node is inherited from this class, you can start looking at the DatabaseView, and DatabaseModule class. -- Thanks, Ashesh > > Thanks > > Rob, Oliver & Joao > > ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Pains and thoughts about refactoring the Tree Menu using React @ 2017-05-01 15:50 Joao Pedro De Almeida Pereira <[email protected]> parent: Ashesh Vashi <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Joao Pedro De Almeida Pereira @ 2017-05-01 15:50 UTC (permalink / raw) To: Ashesh Vashi <[email protected]>; +Cc: pgadmin-hackers; Matthew Kleiman <[email protected]> > > > Apart from them, we also need to understand, how the tree traversal will > work. > How would you traverse through a particular node, and expand all its > parent? In the PoC that we sent with React Tree email <https://www.postgresql.org/message-id/[email protected]...; we added a service that as a function called buildURL, that would map into the GenerateURLAdaptor, and this function will create the URL to retrieve the child nodes of a specific node. Once we have retrieved those child nodes from the backend api, we would simply add the new nodes as children in the tree. Joao & Matt On Fri, Apr 28, 2017 at 5:51 AM, Ashesh Vashi <[email protected] > wrote: > On Sat, Apr 22, 2017 at 12:17 AM, Joao Pedro De Almeida Pereira < > [email protected]> wrote: > >> Hi Hackers, >> >> After a conversation with Dave we believe that we need to provide more >> context on our pains and what we propose as a first step for implementing >> the Tree Menu using React. >> >> The pain: >> >> Here is a quick description of what we think we understand and where we >> got stuck while doing our refactor. >> >> If you look at the above image, both the NodeJS and CollectionJS classes >> are templates. Our goal was to extract those from being templates so that >> we could test them. We started with the generate_url function. The issue we >> ran into was that the generate_url function is inherited by CollectionJS >> from NodeJS but then over writes some of the functionality that it >> inherits. In order to have one function that our react component would >> delegate to, we have to consolidate these two class methods. This turned >> out to be a non-trivial refactor due to the lack of tests/documentation. >> >> Our proposal: >> >> >> This diagram represents what we believe can be our first approach to >> updating the tree. We can create a very basic tree that only does tree >> stuff (Open Node, Close Nodes) and then delegates to other adapters to >> execute all the business logic functionality (Generate URLs to get the >> node, Right Click menu, …) >> > We will loose the current extendible architecture of by creating fix > number of adapters, instead - we should generates events like right now, > let the modules/extensions decide - what to do. > > There are many operations are dependents on the tree events. > e.g. > - Before open > When server node is being opened, 'beforeopen' event checks if server is > connected, or not. > If not try to connect it, otherwise it does not let it open it > > - Added > Dependent modules libraries are loaded, when a particular type of node is > added. > i.e. When the first database node is added in the tree, it loads all the > schema level, and other javascript modules. > > - Selected > When any of the node is added, it shows properties of the selected, change > the dashboard context (if necessary). > > Current tree generates many events. > beforeload, beforeappend, added, appended, loaded, init, beforeselect, > selected, beforefocus, focused, focus, blurred, beforetoggle, beforeopen, > openned, toggled, etc. > > We generate 'pgadmin-browser:tree' events on a pgBrowser.Events objects to > be used by different modules. > At the moment, 'pgadmin-browser:tree:selected' is widely used by > different modules. > i.e. SQL, dependents, dependencies, properties, Dashboard, etc. > > If you want to see the complete list of events generated, you can use the > following patch. > > *diff --git a/web/pgadmin/browser/static/js/datamodel.js > b/web/pgadmin/browser/static/js/datamodel.js* > *index 5b1c3a7..575c78d 100644* > *--- a/web/pgadmin/browser/static/js/datamodel.js* > *+++ b/web/pgadmin/browser/static/js/datamodel.js* > *@@ -1149,5 +1149,10 @@ function(_, pgAdmin, $, Backbone) {* > > * pgBrowser.Events = _.extend({}, Backbone.Events);* > > *+ pgBrowser.Events.on(* > *+ 'pgadmin-browser:tree', function() {* > *+ console.log(arguments[0]);* > *+ });* > *+* > * return pgBrowser;* > * });* > > Apart from them, we also need to understand, how the tree traversal will > work. > How would you traverse through a particular node, and expand all its > parent? > > >> Asks: >> >> - >> >> Are there any more operation that currently are being triggered by >> the tree? If so we need to add them to the Adapter List. >> >> Please see my above comments. > >> >> - >> >> Because we lack the context and knowledge of the current tree >> implementation, we need your help to extract these methods from the places >> they are currently in. We believe that a good place to start would be >> writing tests for and implementing the generation of URLs used to fetch the >> child nodes. >> >> Attached you can find an example of the JavaScript tests that we have >> been writing that can be applied to this extracted method. >> > You may want to start from here: > https://www.pgadmin.org/docs4/dev/code_snippets.html#nodeview > > Every tree node is inherited from this class, you can start looking at the > DatabaseView, and DatabaseModule class. > > -- Thanks, Ashesh > >> >> Thanks >> >> Rob, Oliver & Joao >> >> > ^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2017-05-01 15:50 UTC | newest] Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2017-04-21 18:47 Pains and thoughts about refactoring the Tree Menu using React Joao Pedro De Almeida Pereira <[email protected]> 2017-04-21 18:52 ` Joao Pedro De Almeida Pereira <[email protected]> 2017-04-28 09:51 ` Ashesh Vashi <[email protected]> 2017-05-01 15:50 ` Joao Pedro De Almeida Pereira <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox