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.jo...@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.jo...@enterprisedb.com> wrote: > >> Hi Dave, >> >> On Mon, Jul 2, 2018 at 4:10 PM, Dave Page <dp...@pgadmin.org> wrote: >> >>> Hi >>> >>> On Sat, Jun 30, 2018 at 9:15 AM, Akshay Joshi < >>> akshay.jo...@enterprisedb.com> wrote: >>> >>>> Hi Dave >>>> >>>> On Fri, Jun 29, 2018 at 7:45 PM, Dave Page <dp...@pgadmin.org> wrote: >>>> >>>>> >>>>> >>>>> On Fri, Jun 29, 2018 at 3:12 PM, Akshay Joshi < >>>>> akshay.jo...@enterprisedb.com> wrote: >>>>> >>>>>> Hi Dave >>>>>> >>>>>> On Fri, Jun 29, 2018 at 6:56 PM, Dave Page <dp...@pgadmin.org> wrote: >>>>>> >>>>>>> Hi >>>>>>> >>>>>>> On Fri, Jun 29, 2018 at 9:55 AM, Akshay Joshi < >>>>>>> akshay.jo...@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*
RM_3397_v5.patch
Description: Binary data
