Received: from malur.postgresql.org ([2a02:16a8:dc51::56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1fbPdS-0007OA-IX for pgadmin-hackers@arkaria.postgresql.org; Fri, 06 Jul 2018 12:14:51 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1fbPdQ-0003TC-Ui for pgadmin-hackers@arkaria.postgresql.org; Fri, 06 Jul 2018 12:14:48 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1fbPdQ-0003T5-E6 for pgadmin-hackers@lists.postgresql.org; Fri, 06 Jul 2018 12:14:48 +0000 Received: from mail-wm0-x22e.google.com ([2a00:1450:400c:c09::22e]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1fbPdM-0002Ca-Kz for pgadmin-hackers@postgresql.org; Fri, 06 Jul 2018 12:14:47 +0000 Received: by mail-wm0-x22e.google.com with SMTP id s12-v6so14719895wmc.1 for ; Fri, 06 Jul 2018 05:14:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pgadmin-org.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=k25w6DWgSTCJnw/7nQVrS330nhOV9RonaXchhLpWoKA=; b=NzSgmR8vhVcGXLEYJFU7xHuASq3vNHk5GH642k6fL7PAYpwY5RaRHeTbDNifpVdzri rPwaDKKO1JPGrFryDInN8a+3IXe1fEx1uZQnKGqyUYWaVtH7BHwsJao2/R3JWsxUyjCe HXvgXxcHYCC14iu1sbJRToz7Ns69UvzuX0VNUpRPILubaM3o/GMOY7vOn6dCNsjtwla0 eUBTWo7IqJuYQIX1xG8dYtph/H7DiPgfSzrHPqI7Ook77tGffihNYvq5ehrluy8E9lYJ NjEfl2bgfJ1PMe4lWHWab6zzwIdh3XVtSHK4YMEMeyxYYoVpBgSjfb/oPDxnd4SGAMX0 ZVog== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=k25w6DWgSTCJnw/7nQVrS330nhOV9RonaXchhLpWoKA=; b=BIuYpo6HGAGae7YaAS+NcwBwc2zuE+Us6lvMf428VI77Xnyy9NCGBGO/vPoU2vQyTG JN3bvfiPIF3XG/I1tNOUmC7Hea5KVogr3EpgX8dTT+BePdbarihzy1pjQ04oSiyHKZ2z vpen8fMgyBHfYo06gRmRpcBaN6XM6quXAjw54mUh1IpWAuaVRjlDhVYfKdS1C6d3VUub ficjMAG65MP7cnEPhjnvqivoROQpCfawkSaYZh35xiW4dGKbX8InEWe+2rnyc1h/jbBM TTzLRXHiEAB3SDdTzY2vOqB57v9xzEV3xn+5j6d+bQNqro/8jYZgY/+Va46XdhdZpKkG 6gvg== X-Gm-Message-State: APt69E3IuERwUTbX+7Ye8tiEXzD84efj2Yc5o+tu1TSaeb9My+9HkVEq 7Ax6r2YO7+5uNTW2tRrRk0y3i399qpqM6zaBD7B0mg== X-Google-Smtp-Source: AAOMgpewMTxRVJoLWkbNg+H0R6Ll5xPH0ib0UqI7tqVa5a1Gnyb+VSnk2c3u7ULEzLahglbTy05xQs2b+w10U4tY0qU= X-Received: by 2002:a1c:98a:: with SMTP id 132-v6mr6586683wmj.86.1530879283269; Fri, 06 Jul 2018 05:14:43 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a1c:8955:0:0:0:0:0 with HTTP; Fri, 6 Jul 2018 05:14:42 -0700 (PDT) In-Reply-To: References: From: Dave Page Date: Fri, 6 Jul 2018 13:14:42 +0100 Message-ID: Subject: Re: [pgAdmin4][Patch]: RM #3397 Add support for JIT stats in EXPLAIN output in PG11 To: Akshay Joshi Cc: pgadmin-hackers Content-Type: multipart/alternative; boundary="000000000000dfa5400570539b8a" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --000000000000dfa5400570539b8a Content-Type: text/plain; charset="UTF-8" Thanks, patch applied! On Fri, Jul 6, 2018 at 6:44 AM, Akshay Joshi wrote: > Hi > > On Thu, Jul 5, 2018 at 7:17 PM, Dave Page wrote: > >> Hi >> >> Looks good to me - the only remaining issue is that the button still >> isn't the same colours as the zoom ones. I haven't found all the >> differences, but there seems to be an 'opacity: 0.5" on the stats area, and >> the disabled attribute is still being set. >> > > I have set the disabled attribute, so that the button shouldn't be > clickable and because of that there is difference in colour. 'opacity: > 0.5' is also there for zoom area. I have fixed the issue by removing > disabled attribute and add ' > > *pointer-events: none*' for the statistics button. Attached is the patch > please review it. > >> >> On Thu, Jul 5, 2018 at 11:21 AM, Akshay Joshi < >> akshay.joshi@enterprisedb.com> wrote: >> >>> Hi Dave >>> >>> As per discussion attached is the modified patch with following review >>> comments has been fixed : >>> >>> - Hide statistics button if not applicable(no statistics to show). >>> - Extract 'StatisticsModel' into a separate file and added jasmine >>> test for this. >>> >>> >>> >>> >>> On Tue, Jul 3, 2018 at 6:40 PM, Akshay Joshi < >>> akshay.joshi@enterprisedb.com> wrote: >>> >>>> Hi Dave, >>>> >>>> Please ignore the previous patch, I have made some more changes related >>>> to background colour on enabled/disabled state. >>>> Attached is the modified patch. Please review it. >>>> >>>> On Tue, Jul 3, 2018 at 12:35 PM, Akshay Joshi < >>>> akshay.joshi@enterprisedb.com> wrote: >>>> >>>>> Hi Dave, >>>>> >>>>> On Mon, Jul 2, 2018 at 4:10 PM, Dave Page wrote: >>>>> >>>>>> Hi >>>>>> >>>>>> On Sat, Jun 30, 2018 at 9:15 AM, Akshay Joshi < >>>>>> akshay.joshi@enterprisedb.com> wrote: >>>>>> >>>>>>> Hi Dave >>>>>>> >>>>>>> On Fri, Jun 29, 2018 at 7:45 PM, Dave Page >>>>>>> wrote: >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Fri, Jun 29, 2018 at 3:12 PM, Akshay Joshi < >>>>>>>> akshay.joshi@enterprisedb.com> wrote: >>>>>>>> >>>>>>>>> Hi Dave >>>>>>>>> >>>>>>>>> On Fri, Jun 29, 2018 at 6:56 PM, Dave Page >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> Hi >>>>>>>>>> >>>>>>>>>> On Fri, Jun 29, 2018 at 9:55 AM, Akshay Joshi < >>>>>>>>>> akshay.joshi@enterprisedb.com> wrote: >>>>>>>>>> >>>>>>>>>>> Hi Hackers, >>>>>>>>>>> >>>>>>>>>>> Attached is the patch to fix the RM #3397 Add support for JIT >>>>>>>>>>> stats in EXPLAIN output in PG11. Please review it. >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> A couple of immediate thoughts: >>>>>>>>>> >>>>>>>>>> - When the canvas is first rendered, there's a vertical scrollbar >>>>>>>>>> now. As soon as I mouseover the new icon, it vanishes and the icon jumps to >>>>>>>>>> the right. >>>>>>>>>> >>>>>>>>> >>>>>>>>> Will look into it. Vertical scrollbar comes even if you >>>>>>>>> remove my patch and try to hover any image. >>>>>>>>> >>>>>>>>>> >>>>>>>>>> - The icon seems lighter than the other controls on the left. >>>>>>>>>> >>>>>>>>> >>>>>>>>> Same css has been applied, only difference is button is >>>>>>>>> disabled. >>>>>>>>> >>>>>>>>>> >>>>>>>>>> - The icon isn't disabled when there is no info to show. >>>>>>>>>> >>>>>>>>> >>>>>>>>> Button is always disabled, I have just change the opacity. >>>>>>>>> >>>>>>>>>> >>>>>>>>>> Thanks. >>>>>>>>>> >>>>>>>>> >>>>>>>> Maybe - but I can still click it and it reacts as if it's active. >>>>>>>> It may be lighter to indicate that it's disabled, but its not behaving as >>>>>>>> such. >>>>>>>> >>>>>>> >>>>>>> Attached is the modified patch. Please review it. >>>>>>> >>>>>> >>>>>> The button still changes foreground colour on mouseover when >>>>>> disabled. I think it needs to be completely non-reactive when disabled. It >>>>>> should also be a noticably lighter shade when disabled; right now it seems >>>>>> to be darker than the other buttons (see attached). >>>>>> >>>>> >>>>> Attached is the modified patch. Please review it. >>>>> >>>>>> >>>>>> -- >>>>>> 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* >>>> >>> >>> >>> >>> -- >>> *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 --000000000000dfa5400570539b8a Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Thanks, patch applied!
On Fri, Jul 6, 2018 at 6:44 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi

On Thu, Jul 5, 2018 at 7:17 PM, D= ave Page <dpage@pgadmin.org> wrote:=
Hi

Looks good to me - the only remaining issue is that = the button still isn't the same colours as the zoom ones. I haven't= found all the differences, but there seems to be an 'opacity: 0.5"= ; on the stats area, and the disabled attribute is still being set.=C2=A0

=C2=A0 =C2=A0 I have set = the disabled attribute, so that the button shouldn't be clickable and because of that there is d= ifference in colour. 'opacity: 0.5' is also there for zoom area. I = have fixed the issue by removing disabled attribute and add '
pointer-events: none
' for the statistics button. Attac= hed is the patch please review it.=C2=A0 =C2=A0 =C2=A0
=
On Thu, Jul 5, 2018 at 11:21 AM, Akshay Josh= i <akshay.joshi@enterprisedb.com> wrote:
Hi Dave

As per discussion attached is the modified patch with following review com= ments has been fixed :
  • Hide statistics button if not appl= icable(no statistics to show).
  • Extract 'StatisticsModel' in= to a separate file and added jasmine test for this.=C2=A0=C2=A0



O= n Tue, Jul 3, 2018 at 6:40 PM, Akshay Joshi <akshay.joshi@en= terprisedb.com> wrote:
Hi Dave,

Please ignore the previous pa= tch, I have made some more changes related to background colour on enabled/= disabled state.=C2=A0
Attached is the modified patch. Pleas= e review it.=C2=A0 =C2=A0

On Tue, Jul 3, 2018 at 12:3= 5 PM, Akshay Joshi <akshay.joshi@enterprisedb.com>= wrote:
Hi Dave,<= div class=3D"gmail_extra">

On Mon, Jul 2, 2018 at 4:10 PM, Dave Page <dpage@pgadmin.= org> wrote:
Hi

On Sat,= Jun 30, 2018 at 9:15 AM, Akshay Joshi <akshay.joshi@enterpris= edb.com> wrote:
Hi Dave

On Fri, Jun 29, 2018 at 7:45 PM, Dave Page <dpage@pgadmin.org&g= t; wrote:


On Fri, Jun 29= , 2018 at 3:12 PM, Akshay Joshi <akshay.joshi@enterprisedb.com= > wrote:
Hi Dave

= On Fri, Jun 29, 2018 at 6:56 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Fri, Jun 29, 2018 = at 9:55 AM, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi = Hackers,

Attached is the patch to fix the RM #3397=C2=A0= Add support for JIT stats in EXPLAIN= output in PG11. Please review it.

<= /span>
A couple of immediate thoughts:

- When = the canvas is first rendered, there's a vertical scrollbar now. As soon= as I mouseover the new icon, it vanishes and the icon jumps to the right.<= /div>

=C2=A0 =C2= =A0 =C2=A0Will look into it. Vertical scrollbar comes even if you remove my= patch and try to hover any image.=C2=A0

- The icon seems lighter than the other controls on= the left.

= =C2=A0 =C2=A0 =C2=A0Same css has been applied, only difference is button is= disabled.=C2=A0

= - The icon isn't disabled when there is no info to show.

=C2=A0 =C2=A0 =C2=A0Button= is always disabled, I have just change the opacity.=C2=A0 =C2=A0=C2=A0

Thanks.=C2=A0
<= /div>
Maybe - but I can still click it and it reacts as if it= 's active. It may be lighter to indicate that it's disabled, but it= s not behaving as such.=C2=A0

=
=C2=A0 =C2=A0 Attached is the modified patch. Please revi= ew it.=C2=A0=C2=A0

The button still changes foreground colour on mouseover when disable= d.=C2=A0 I think it needs to be completely non-reactive when disabled. It s= hould also be a noticably lighter shade when disabled; right now it seems t= o be darker than the other buttons (see attached).
<= /blockquote>

=C2=A0 =C2=A0Attached is the modified patch= . Please review it.=C2=A0

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQ= L Company



-= -
<= font color=3D"#3333FF">Akshay Joshi
Sr= . Software Architect

<= /span>

Phone: +91 20-3058-9517
Mobile: +91 976-788-8246



--
Akshay Joshi
Sr. Software Architect
<= /font>

=

Phone: +91 20-3058= -9517
Mobile: +91 976-788-8246
=



--
=
Akshay Joshi
Sr. Software Architect
=

<= /font>

Phone: +91 20-3058-9517
Mob= ile: +91 976-788-8246



--
=
Dave Page
Blog: http://pgsnake.blogspot.com
Twit= ter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise Post= greSQL Company



--
Akshay Joshi
Sr. Software Architect

<= font color=3D"#3333FF">
=
Phone: +91= 20-3058-9517
Mobile: +91 976-788-8246



--
Dave Page
Blog: = http://pgsnake.bl= ogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com<= br>The Enterprise PostgreSQL Company
--000000000000dfa5400570539b8a--