On Tue, Jul 17, 2018 at 2:11 PM, Akshay Joshi <akshay.jo...@enterprisedb.com > wrote:
> > > 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" ? > 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 <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* > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company