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