Re: Regarding RM #3316 Pgadmin4 No Tray Crash

On Thu, Jul 12, 2018 at 4:40 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

>
>
> 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.
>

I played some more. It looks like it's an easy fix - something like the
following seems to work for me on Fedora 28 (no tray) and macOS, with no
noticeable delay on either:

if (!QSystemTrayIcon::isSystemTrayAvailable() || !trayicon->Init())

{

// Delete Tray Icon object

delete trayicon;

trayicon = NULL;

splash->showMessage(QString(QWidget::tr("System tray not
found, creating floating window...")), Qt::AlignBottom |
Qt::AlignCenter);

Of course, if we call QSystemTrayIcon::isSystemTrayAvailable() earlier, we
can skip even trying to create the tray icon, rather than creating it and
then deleting it. Can you look at refactoring that way, and see if it works
for you please?

Thanks.

>
>
>>
>>
>>> 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
>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company