Here is a new patchset that instead hides the spinner when the acitree has been initialized. On average, the spinner seems to disappear about 2 seconds sooner, and I haven't seen flakiness with these changes yet.
Tira & Joao On Mon, Mar 20, 2017 at 4:17 PM, Atira Odhner <aodh...@pivotal.io> wrote: > Note that this patch makes the problem of the tree not having loaded > worse, because it only waits for js modules to load rather than arbitrarily > waiting 900ms. > > On Mon, Mar 20, 2017 at 3:17 PM, Atira Odhner <aodh...@pivotal.io> wrote: > >> Hi Ashesh, >> >> *Regarding your second patch:* >> >> It looks like your second patch addresses module loading. This is an >> improvement over the previous hard timeout, but won’t do anything for the >> tree issues. The module loading code can also be simplified; we’ve attached >> a patchset that is tidier, tests the behavior, and speeds up the polling. >> >> Ashesh, can you explain why you are setting the text on the spinner after >> hiding it, or why you are hiding it rather than removing it? >> >> *Regarding your first patch:* >> >> Descriptive variable names are clearer than single-letter variable names. >> Could you clean up the first patch to use clearer variables, perhaps add >> some tests and break it up into methods so that I can more easily >> understand what your change does? >> >> Thank you, >> >> Tira & George >> >> On Mon, Mar 20, 2017 at 8:03 AM, Dave Page <dp...@pgadmin.org> wrote: >> >>> On Mon, Mar 20, 2017 at 10:24 AM, Ashesh Vashi >>> <ashesh.va...@enterprisedb.com> wrote: >>> > On Fri, Mar 17, 2017 at 8:35 PM, Sarah McAlear <smcal...@pivotal.io> >>> wrote: >>> >> >>> >> Hello, >>> >> >>> >> We agree that we should keep an eye on this and the failing feature >>> tests. >>> >> Our current story touches part of this code, but we won't go into >>> changing >>> >> the library for now. >>> >> >>> >> The patch Tira sent fixes a global variable problem that we found >>> while >>> >> looking into the code that generates the Tree, that >>> >> had the potential to load the tree in an infinite loop. >>> >> Can you apply the patch like this, or would you rather us send it in a >>> >> separate patch email? >>> > >>> > Name of the variable should have been itemData, d (as earlier), as it >>> > represents the data for the expanding node item. >>> > And, that is not good enough for resolving the issue. >>> > >>> > I've two approaches to resolve the idea. >>> > 1. Load the nodes (even when the module representing that node is not >>> yet >>> > loaded) >>> > Pros: >>> > - Nodes will be loaded even when the module for the node type is not >>> yet >>> > loaded. >>> > Cons: >>> > - The nodes with modified url (not the default mechanism) may get >>> failed, if >>> > the module is not yet loaded. >>> > (NOTE: We've not no nodes with that changes at the moment. so - we >>> can >>> > safely ignore it.) >>> > - Operations specific to the nodes will not be honoured until modules >>> is not >>> > loaded. >>> > >>> > 2. Wait for the modules to get loaded before allowing any operations on >>> > them. >>> > Pros: >>> > - All operations will be done on a node only after the respective >>> module is >>> > loaded. >>> > Cons: >>> > - It will block any operations on pgAdmin 4, when the dependent >>> modules are >>> > being loaded. It can annoy the user. >>> > >>> > Please find the patches for both the approaches. >>> > >>> > Dave - please take a look at it. >>> >>> I'll leave you and Tira to figure this one out if you don't mind. My >>> plate is kinda full at the moment. >>> >>> I will note though that neither blocking or potential failures are >>> desirable. >>> >>> -- >>> Dave Page >>> Blog: http://pgsnake.blogspot.com >>> Twitter: @pgsnake >>> >>> EnterpriseDB UK: http://www.enterprisedb.com >>> The Enterprise PostgreSQL Company >>> >> >> >
0001-Extract-acitree-event-handling-so-it-can-be-tested.patch
Description: Binary data
0002-Hide-the-spinner-once-the-acitree-is-initialized.patch
Description: Binary data
0003-Remove-code-that-hides-spinner-after-900ms.patch
Description: Binary data
-- Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers
