Re: Regarding RM #3316 Pgadmin4 No Tray Crash

On Tue, Jul 17, 2018 at 2:06 PM, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com
> wrote:

> Hi Dave,
>
> I have changed the background colour on mouse hover for "pgAdmin4" menu.
> Attached is the screenshot, please review it and let me know which colour
> looks good or any other suggestion. I'll send the patch accordingly.
>

I think we should leave it at the OS defaults.

>
> On Tue, Jul 17, 2018 at 4:42 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Thanks, patch applied.
>>
>> On Fri, Jul 13, 2018 at 7:10 AM, Akshay Joshi <
>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>
>>> Attached is the refactored patch.
>>>
>>> On Fri, Jul 13, 2018 at 11:19 AM, Akshay Joshi <
>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>
>>>>
>>>>
>>>> On Thu, Jul 12, 2018 at 9:38 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>
>>>>>
>>>>>
>>>>> 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?
>>>>>
>>>>
>>>> Sure, If you can see the Init() method of trayicon it calls
>>>> isSystemTrayAvailable() which eventually call QSystemTrayIcon::isSystemTrayAvailable()
>>>> for 10 times in loop with wait of 3 seconds. Why we have added that logic?
>>>> Is that method required to be called multiple times for the correct return
>>>> value ? I am asking that because it would be easy for me to refactor the
>>>> logic. If we can rely on single call to the function QSystemTrayIcon::
>>>> isSystemTrayAvailable() then no need to create a tray icon if system
>>>> tray is not available.
>>>>
>>>> Meanwhile I'll start refactoring the logic. Thanks
>>>>
>>>>
>>>>
>>>>>
>>>>> 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
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> *Akshay Joshi*
>>>>
>>>> *Sr. Software Architect *
>>>>
>>>>
>>>>
>>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>>
>>>
>>>
>>>
>>> --
>>> *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
>>
>
>
>
> --
> *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