Re: Regarding RM #3316 Pgadmin4 No Tray Crash

On Tue, Jul 17, 2018 at 6:37 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

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

OK, should I commit one change adding "&" before the "pgAdmin4" string
in menu, that will add shortcut and user can open menu by pressing "ALT +
p" ?

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

--
*Akshay Joshi*

*Sr. Software Architect *

*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*