Re: [pgAdmin4][Patch]: RM#1242 - Cleanup busy indication for tree view and tab panes

From: Surinder Kumar <surinder(dot)kumar(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
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 09:45:50
Message-ID: CAM5-9D9Shz6x-ZQpewc+XHJc9_taq2WG3H2Qz9HatxVV2qMCaw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2016-06-29 09:46:51 pgAdmin 4 commit: Show a loading indicator until things are ready to ro
Previous Message Dave Page 2016-06-29 09:43:23 Re: [pgAdmin4][Patch]: RM#1242 - Cleanup busy indication for tree view and tab panes