From: | Dave Page <dpage(at)pgadmin(dot)org> |
---|---|
To: | Surinder Kumar <surinder(dot)kumar(at)enterprisedb(dot)com> |
Cc: | pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org> |
Subject: | Re: [pgAdmin4][Patch]: RM#1242 - Cleanup busy indication for tree view and tab panes |
Date: | 2016-06-29 11:16:11 |
Message-ID: | CA+OCxowK4hmsC85UdXUU0EKn99UNTVN7Ca2O+5k7aFYs9v3iiA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
Thanks, applied.
On Wed, Jun 29, 2016 at 11:49 AM, Surinder Kumar
<surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
> Hi,
>
> Please find the v2 patch for RM#1242
> Change:
> 1) Removed the busy indicators from the dashboard, stats, properties, deps
> etc.
> 2) Also merged the changes of patch "busy icon not centered".
>
> Apart from this, In Runtime environment busy indicators are not visible
> while rendering of Debugger and Query tool of some reason.
> I will take a look this issue.
>
>
> On Wed, Jun 29, 2016 at 3:15 PM, Surinder Kumar
> <surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>
>> On Wed, Jun 29, 2016 at 3:13 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>> Hi
>>>
>>> On Mon, Jun 27, 2016 at 7:09 PM, Surinder Kumar
>>> <surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>> > Hi Dave,
>>> >
>>> > I have seen following changes are missed in the code checked in:
>>> > 1) "Code to avoid unnecessary reloads & displaying spinner" for
>>> > Properties,
>>> > SQL & Statistics tab is missing.
>>> > whereas the code for depends and dependability panel is checked in.
>>> > 2) "Code to show spinner before panel is loaded" for Query Tool and
>>> > Data
>>> > grid is missing. File is datagrid.js
>>> >
>>> > I guess they are missed?
>>>
>>> Yeah, looks like PyCharms messed up applying the patch.
>>>
>>> I've reverted it all for now though - after playing with it for a
>>> little longer, it became clear that the behaviour is actually quite
>>> annoying because we now have a 1 second delay to avoid redrawing the
>>> display too frequently.
>>>
>>> Can you please remove the busy indicators from the individual panels
>>> (dashboard, stats, properties, deps etc), and just leave them for the
>>> long-running stuff like the debugger and query tool?
>>
>> Ok, I will do
>>>
>>>
>>> > On Mon, Jun 27, 2016 at 9:42 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>> >>
>>> >> Thanks - I've applied this as it's a vast improvement. However, the
>>> >> busy text shown over panels is still not centered - for example, in
>>> >> the attached screenshot, I would expect the busy cursor and center of
>>> >> the text to be directly below the "void (4)".
>>> >>
>>> >> Can you tweak that as needed please?
>>> >>
>>> >> Thanks.
>>> >>
>>> >> On Mon, Jun 27, 2016 at 2:38 PM, Surinder Kumar
>>> >> <surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>> >> > Hi Dave,
>>> >> >
>>> >> > Following are the changes/fixes in the patch:
>>> >> >
>>> >> > The spinner icons for the Query Tool and Debugger are now
>>> >> > consistent.
>>> >> > Greyed background is now consistent throughout the panels.
>>> >> > Text is centered.
>>> >> > Now CSS styles are generic for spinner in all panels.
>>> >> > Panes content is now refreshed only when new data differs from
>>> >> > previous
>>> >> > data, reused the code written in dashboard js
>>> >> > Add spinner to the data grid and query tool when they opened from
>>> >> > the
>>> >> > context menu.
>>> >> > Hide spinner after 1000ms so that it doesn't seems to cause
>>> >> > flickering.
>>> >> > Bigger spinner icon is removed from the treeview.
>>> >> >
>>> >> > Please review.
>>> >> >
>>> >> > On Fri, Jun 24, 2016 at 3:04 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>> >> > wrote:
>>> >> >>
>>> >> >> On Thu, Jun 23, 2016 at 1:52 PM, Surinder Kumar
>>> >> >> <surinder(dot)kumar(at)enterprisedb(dot)com> wrote:
>>> >> >> > Hi
>>> >> >> >
>>> >> >> > Following are the changes in patch:
>>> >> >> >
>>> >> >> > 1) Assign a common class 'pg_spinner' for loading spinner in
>>> >> >> > wcDocker
>>> >> >> > for
>>> >> >> > all panels.
>>> >> >> > 2) Removed the default wcDocker spinner icon which was specific
>>> >> >> > to
>>> >> >> > page
>>> >> >> > load.
>>> >> >> > 3) In case of collection node, we were re-creating the grid every
>>> >> >> > time
>>> >> >> > new
>>> >> >> > data comes. instead it should only reset the collection with new
>>> >> >> > data.
>>> >> >> > 4) Move the code to hide the msgContainer and display grid
>>> >> >> > outside
>>> >> >> > the
>>> >> >> > ajax
>>> >> >> > which was causing flicker into the dependent panel.
>>> >> >> > 5) Set the timeout for hiding the spinner to 500 inside the
>>> >> >> > success
>>> >> >> > callback
>>> >> >> > to stop flicker because response is received immediately and
>>> >> >> > cause
>>> >> >> > flicker.
>>> >> >> >
>>> >> >> > Not Fixed:
>>> >> >> > Panes should have their content refreshed until and unless the
>>> >> >> > new
>>> >> >> > data
>>> >> >> > differs from the previous, to reduce flicker.
>>> >> >> > I am working on this issue.
>>> >> >> >
>>> >> >> > Please review the patch.
>>> >> >>
>>> >> >> I think this is an improvement, but not yet ready. For example:
>>> >> >>
>>> >> >> - The treeview shows spinners on the junctions of the branches, and
>>> >> >> a
>>> >> >> bigger one in the middle of the pane, simultaneously. I think the
>>> >> >> bigger one should be removed.
>>> >> >>
>>> >> >> - The spinners for the Query Tool and Debugger are inconsistent and
>>> >> >> poorly laid out:
>>> >> >> - Different spinner icons (FYI, I prefer the one used in the
>>> >> >> debugger - can we use that everywhere?)
>>> >> >> - Text not centered
>>> >> >> - Background greyed in the Query Tool, but not the Debugger
>>> >> >>
>>> >> >> Additionally, I've logged another issue
>>> >> >> (https://redmine.postgresql.org/issues/1400) which you may want to
>>> >> >> take care of at the same time as this.
>>> >> >>
>>> >> >> Thanks!
>>> >> >>
>>> >> >> --
>>> >> >> Dave Page
>>> >> >> Blog: http://pgsnake.blogspot.com
>>> >> >> Twitter: @pgsnake
>>> >> >>
>>> >> >> EnterpriseDB UK: http://www.enterprisedb.com
>>> >> >> The Enterprise PostgreSQL Company
>>> >> >
>>> >> >
>>> >>
>>> >>
>>> >>
>>> >> --
>>> >> Dave Page
>>> >> Blog: http://pgsnake.blogspot.com
>>> >> Twitter: @pgsnake
>>> >>
>>> >> EnterpriseDB UK: http://www.enterprisedb.com
>>> >> The Enterprise PostgreSQL Company
>>> >
>>> >
>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>
>>
>
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Dave Page | 2016-06-29 11:19:06 | Re: PATCH: 'serial' like types were missing while creating table/column (pgAdmin4) |
Previous Message | Dave Page | 2016-06-29 11:16:05 | pgAdmin 4 commit: Consistent busy indication. Fixes #1242 |