Re: Regarding RM #3316 Pgadmin4 No Tray Crash

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: Regarding RM #3316 Pgadmin4 No Tray Crash
Date: 2018-07-12 10:17:17
Message-ID: CANxoLDdc-1MgotfO03TaDWizPbSujmqUSzptgcm234mYd9jN+A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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.
- Window is resizable, to fix that i have set the fixed size.

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

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

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.

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*

Attachment Content-Type Size
image/png 29.0 KB
RM_3316_v2.patch application/octet-stream 25.5 KB
Splash_With_Message.png image/png 137.5 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2018-07-12 12:29:00 pgAgent commit: Tag REL-4_0_0 has been created.
Previous Message Dave Page 2018-07-12 09:25:33 Re: [pgAdmin4][Patch]: Fix RM #3191 : Debug option is not working