Re: [pgAdmin4][Patch]: RM #3397 Add support for JIT stats in EXPLAIN output in PG11

Hi

On Thu, Jul 5, 2018 at 7:17 PM, Dave Page <dpage(at)pgadmin(dot)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.
>

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(dot)joshi(at)enterprisedb(dot)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(dot)joshi(at)enterprisedb(dot)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(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Dave,
>>>>
>>>> On Mon, Jul 2, 2018 at 4:10 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>
>>>>> Hi
>>>>>
>>>>> On Sat, Jun 30, 2018 at 9:15 AM, Akshay Joshi <
>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Dave
>>>>>>
>>>>>> On Fri, Jun 29, 2018 at 7:45 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Fri, Jun 29, 2018 at 3:12 PM, Akshay Joshi <
>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Hi Dave
>>>>>>>>
>>>>>>>> On Fri, Jun 29, 2018 at 6:56 PM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi
>>>>>>>>>
>>>>>>>>> On Fri, Jun 29, 2018 at 9:55 AM, Akshay Joshi <
>>>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)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*