Re: Regarding RM #3316 Pgadmin4 No Tray Crash

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: Regarding RM #3316 Pgadmin4 No Tray Crash
Date: 2018-07-12 15:40:14
Message-ID: CA+OCxoxDmYpU-0XpnTAnEcxxUS1TC=FiJd1kM4X6448hBD_bjw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

On Thu, Jul 12, 2018 at 11:17 AM, Akshay Joshi <
akshay(dot)joshi(at)enterprisedb(dot)com> wrote:

>
>
> On Wed, Jul 11, 2018 at 8:18 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>>
>>
>> On Wed, Jul 11, 2018 at 11:05 AM, Akshay Joshi <
>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Dave
>>>
>>> On Wed, Jul 11, 2018 at 1:31 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>>
>>>>
>>>> On Wed, Jul 11, 2018 at 8:03 AM, Akshay Joshi <
>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi
>>>>>
>>>>> On Tue, Jul 10, 2018 at 9:16 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Tue, Jul 10, 2018 at 4:05 PM, Akshay Joshi <
>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Tue, 10 Jul 2018, 18:32 Dave Page, <dpage(at)pgadmin(dot)org> wrote:
>>>>>>>
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> On Tue, Jul 10, 2018 at 11:20 AM, Akshay Joshi <
>>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> Hi Hackers,
>>>>>>>>>
>>>>>>>>> I have implemented the fix for RM #3316 "Pgadmin4 No Tray Crash".
>>>>>>>>> I have implemented it as follows:
>>>>>>>>>
>>>>>>>>> - Check the availability of System Tray for 30 seconds (no
>>>>>>>>> changes made here).
>>>>>>>>> - If System Tray not found create one Floating Window with
>>>>>>>>> menu. (Refer attached screenshot).
>>>>>>>>> - I have remove close(x) button of the floating window.
>>>>>>>>> - Fedora have "Quit" menu for the applications, so I have
>>>>>>>>> handle the close event and shutdown the python server.
>>>>>>>>>
>>>>>>>>> I have tested this on Fedora-28 (no system tray) and Ubuntu 18.04
>>>>>>>>> (with system tray).
>>>>>>>>>
>>>>>>>>> Please review the screenshot and suggest changes (if any). I'll
>>>>>>>>> send the patch later.
>>>>>>>>>
>>>>>>>>
>>>>>>>> What does the window look like without the menu? I'd suggest
>>>>>>>> putting a Slonik image there, and maybe a note (or a button to display a
>>>>>>>> note) saying that installing a system tray plugin can be used to hide the
>>>>>>>> window.
>>>>>>>>
>>>>>>>
>>>>>>> You mean instead of "pgAdmin4" menu we should add toolbar with
>>>>>>> button having slonik image and when click on there submenus will be open.
>>>>>>>
>>>>>>
>>>>>> No - I assume that's a drop down menu over a blank window? I'm
>>>>>> suggesting something to fill the blank space.
>>>>>>
>>>>>
>>>>> Attached are the screenshot after adding 'Slonik' icon and 'Note'.
>>>>>
>>>>
>>>> Looks good - though please fix the aspect ratio so it doesn't squish
>>>> the logo. I may tweak the message in review.
>>>>
>>>
>>> I have tried using "QPixmap", but not able to fix the aspect ratio.
>>>
>>>>
>>>>
>>>>> Should we allow user to resize the floating window? Definitely not
>>>>> maximize, because icon gets blurred.
>>>>>
>>>>
>>>> No - but it should be minimisable.
>>>>
>>>
>>> Fixed. Attached is the working patch. Please review it.
>>>
>>
>> Thanks. Here's an update to the patch which makes the floating window a
>> Qt Form for ease of maintenance, as well as making a few other tweaks.
>>
>
> I have applied the patch and found below issues:
>
> - Slonik icon is not visible, I have remove the QPixmap and apply the
> style sheet in ui file, it works for me.
>
> I'll bet it was because I told it to use a local file, which it included
without a path. I've seen Qt mess that up before.

>
> - Window is resizable, to fix that i have set the fixed size.
>
> OK.

> Please refer "*pgAdmin4_Floating_Window.png*"
>
>>
>> I really dislike the 30 second delay that's caused by the attempt to
>> initialise the tray icon. We need to do something about that;
>>
>
> Do we need to reduce the delay? Meanwhile I have added some action
> messages like "Checking for system tray..." onto the splash screen, so the
> user will be aware of the actions. Please refer "*Splash_With_Message.png*"
>
>

Yes - we cannot have the check for a system tray taking 30 seconds. People
get very annoyed when pgAdmin takes too long to start!

As a worst case scenario, we can add a command line switch that tells it to
not bother using the tray, which can then be used in the shortcut on the
menu. However, I really would like to make this fully automatic, and near
instantaneous if possible, so let's try to figure it out before falling
back to that hack.

>
>
>> can we start the splash screen and server before trying to create the
>> icon? That way the user can be up and running immediately; they just won't
>> see the tray icon or floating window for a short while.
>>
>
> Logic to show splash screen is already there and it is before
> initializing the tray icon, but for some reason it is not visible. It is
> visible only when app.exec() will be called. To fix that I'll have to add
> sleep of 1 second before calling processEvents() (googled it).
>

We shouldn't need a sleep - simply processing events should be enough.

>
> Starting the server before trying to create tray icon shouldn't be a
> good idea, because in case of some error in starting the server, user
> should be able to view the log by clicking the "View log" menu from tray
> icon.
>

Yeah - I'm not sure it's a huge issue as they'll be able to click something
within 30 seconds, but it's certainly far from ideal.

Thanks.

>
> Attached is the modified patch. Please review it.
>
>>
>> Thanks.
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>
>
> --
> *Akshay Joshi*
>
> *Sr. Software Architect *
>
>
>
> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>

--
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 2018-07-12 16:08:03 Re: Regarding RM #3316 Pgadmin4 No Tray Crash
Previous Message Dave Page 2018-07-12 14:17:31 Re: pgAgent v4.0.0 Released