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

On Thu, Jun 23, 2016 at 1:52 PM, Surinder Kumar
<surinder.kumar@enterprisedb.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

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.kumar@enterprisedb.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@pgadmin.org> wrote:
>>
>> On Thu, Jun 23, 2016 at 1:52 PM, Surinder Kumar
>> <surinder.kumar@enterprisedb.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

Hi

On Mon, Jun 27, 2016 at 7:09 PM, Surinder Kumar
<surinder.kumar@enterprisedb.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?

> On Mon, Jun 27, 2016 at 9:42 PM, Dave Page <dpage@pgadmin.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.kumar@enterprisedb.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@pgadmin.org> wrote:
>> >>
>> >> On Thu, Jun 23, 2016 at 1:52 PM, Surinder Kumar
>> >> <surinder.kumar@enterprisedb.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

Thanks, applied.

On Wed, Jun 29, 2016 at 11:49 AM, Surinder Kumar
<surinder.kumar@enterprisedb.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.kumar@enterprisedb.com> wrote:
>>
>> On Wed, Jun 29, 2016 at 3:13 PM, Dave Page <dpage@pgadmin.org> wrote:
>>>
>>> Hi
>>>
>>> On Mon, Jun 27, 2016 at 7:09 PM, Surinder Kumar
>>> <surinder.kumar@enterprisedb.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@pgadmin.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.kumar@enterprisedb.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@pgadmin.org>
>>> >> > wrote:
>>> >> >>
>>> >> >> On Thu, Jun 23, 2016 at 1:52 PM, Surinder Kumar
>>> >> >> <surinder.kumar@enterprisedb.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