On Tue, Jul 17, 2018 at 6:37 PM, Dave Page <dp...@pgadmin.org> wrote:
> > > On Tue, Jul 17, 2018 at 2:06 PM, Akshay Joshi < > akshay.jo...@enterprisedb.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 <dp...@pgadmin.org> wrote: >> >>> Thanks, patch applied. >>> >>> On Fri, Jul 13, 2018 at 7:10 AM, Akshay Joshi < >>> akshay.jo...@enterprisedb.com> wrote: >>> >>>> Attached is the refactored patch. >>>> >>>> On Fri, Jul 13, 2018 at 11:19 AM, Akshay Joshi < >>>> akshay.jo...@enterprisedb.com> wrote: >>>> >>>>> >>>>> >>>>> 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* >>>>> >>>> >>>> >>>> >>>> -- >>>> *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*