Re: Regarding RM #3316 Pgadmin4 No Tray Crash

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

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

Sure, if it works. I've found Qt doesn't always seem to honour that.

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

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

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