On Thu, Jul 12, 2018 at 9:38 PM, Dave Page <dp...@pgadmin.org> wrote:
> > > On Thu, Jul 12, 2018 at 4:40 PM, Dave Page <dp...@pgadmin.org> wrote: > >> >> >> On Thu, Jul 12, 2018 at 11:17 AM, Akshay Joshi < >> akshay.jo...@enterprisedb.com> wrote: >> >>> >>> >>> On Wed, Jul 11, 2018 at 8:18 PM, Dave Page <dp...@pgadmin.org> wrote: >>> >>>> >>>> >>>> On Wed, Jul 11, 2018 at 11:05 AM, Akshay Joshi < >>>> akshay.jo...@enterprisedb.com> wrote: >>>> >>>>> Hi Dave >>>>> >>>>> On Wed, Jul 11, 2018 at 1:31 PM, Dave Page <dp...@pgadmin.org> wrote: >>>>> >>>>>> >>>>>> >>>>>> On Wed, Jul 11, 2018 at 8:03 AM, Akshay Joshi < >>>>>> akshay.jo...@enterprisedb.com> wrote: >>>>>> >>>>>>> Hi >>>>>>> >>>>>>> On Tue, Jul 10, 2018 at 9:16 PM, Dave Page <dp...@pgadmin.org> >>>>>>> wrote: >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Tue, Jul 10, 2018 at 4:05 PM, Akshay Joshi < >>>>>>>> akshay.jo...@enterprisedb.com> wrote: >>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On Tue, 10 Jul 2018, 18:32 Dave Page, <dp...@pgadmin.org> wrote: >>>>>>>>> >>>>>>>>>> Hi >>>>>>>>>> >>>>>>>>>> On Tue, Jul 10, 2018 at 11:20 AM, Akshay Joshi < >>>>>>>>>> akshay.jo...@enterprisedb.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*