Thread: [pgadmin-hackers] [pgAdmin4][Patch][Feature_1535]: Pressing ESC from within a dialogbox should act like "Cancel" button

Hi

On Thu, Jun 8, 2017 at 5:38 AM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> Hi
>
> The attribute key for Ok, Save buttons should be mapped to 13(Enter),
> for Help - 112(F1) and
> for Cancel - 27(Esc)
> The above changes are made into all dialog related template files.
>
> PFA patch and review.

Looks good. Just a couple of thoughts:

- You've missed the object property dialogues. This really needs to be
consistent across all dialogues :-)

- The behaviour with Enter is a bit odd, because priority will be
given the the focussed control to handle it. That's what we want if
it's a text area for example, but on the other hand, if you open the
Preference dialogue, then the Help button gets focus first, so Enter
there will end up opening the docs. I suspect in most cases the
behaviour you've got is fine, so I'm not inclined to change it. <plays
a little> Hmm, I see similar behaviour in some other apps. Let's leave
it as you've done it :-)

Please add support for the properties dialogues, then I think we're good to go.

Thanks!

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi

Ahh, now we have a bug. In the properties dialogues, if you edit a
comment, as soon as the Save button gets enabled, hitting Return will
Save the changes instead of adding a newline to the comment. That's
clearly not right :-(

On Fri, Jun 9, 2017 at 7:48 AM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> Hi
>
> Please find updated patch.
>
> Thanks,
> Surinder
>
> On Thu, Jun 8, 2017 at 7:16 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> On Thu, Jun 8, 2017 at 5:38 AM, Surinder Kumar
>> <surinder.kumar@enterprisedb.com> wrote:
>> > Hi
>> >
>> > The attribute key for Ok, Save buttons should be mapped to 13(Enter),
>> > for Help - 112(F1) and
>> > for Cancel - 27(Esc)
>> > The above changes are made into all dialog related template files.
>> >
>> > PFA patch and review.
>>
>> Looks good. Just a couple of thoughts:
>>
>> - You've missed the object property dialogues. This really needs to be
>> consistent across all dialogues :-)
>>
>> - The behaviour with Enter is a bit odd, because priority will be
>> given the the focussed control to handle it. That's what we want if
>> it's a text area for example, but on the other hand, if you open the
>> Preference dialogue, then the Help button gets focus first, so Enter
>> there will end up opening the docs. I suspect in most cases the
>> behaviour you've got is fine, so I'm not inclined to change it. <plays
>> a little> Hmm, I see similar behaviour in some other apps. Let's leave
>> it as you've done it :-)
>>
>> Please add support for the properties dialogues, then I think we're good
>> to go.
>
> Done.
>>
>>
>> Thanks!
>>
>> --
>> 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

Could you rebase this please?

Thanks.

On Fri, Jun 9, 2017 at 10:40 AM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> Ignore previous patch as it doesn't contains new changes.
> Please find attached patch.
>
>
> On Fri, Jun 9, 2017 at 3:06 PM, Surinder Kumar
> <surinder.kumar@enterprisedb.com> wrote:
>>
>> Hi
>>
>> PFA patch and review.
>>
>>
>> On Fri, Jun 9, 2017 at 1:47 PM, Dave Page <dpage@pgadmin.org> wrote:
>>>
>>> Hi
>>>
>>> Ahh, now we have a bug. In the properties dialogues, if you edit a
>>> comment, as soon as the Save button gets enabled, hitting Return will
>>> Save the changes instead of adding a newline to the comment. That's
>>> clearly not right :-(
>>
>> Now if event.target is its child elements(for example input, textarea
>> etc), the function "handleKeyDown(event)" won't call.
>>>
>>>
>>> On Fri, Jun 9, 2017 at 7:48 AM, Surinder Kumar
>>> <surinder.kumar@enterprisedb.com> wrote:
>>> > Hi
>>> >
>>> > Please find updated patch.
>>> >
>>> > Thanks,
>>> > Surinder
>>> >
>>> > On Thu, Jun 8, 2017 at 7:16 PM, Dave Page <dpage@pgadmin.org> wrote:
>>> >>
>>> >> Hi
>>> >>
>>> >> On Thu, Jun 8, 2017 at 5:38 AM, Surinder Kumar
>>> >> <surinder.kumar@enterprisedb.com> wrote:
>>> >> > Hi
>>> >> >
>>> >> > The attribute key for Ok, Save buttons should be mapped to
>>> >> > 13(Enter),
>>> >> > for Help - 112(F1) and
>>> >> > for Cancel - 27(Esc)
>>> >> > The above changes are made into all dialog related template files.
>>> >> >
>>> >> > PFA patch and review.
>>> >>
>>> >> Looks good. Just a couple of thoughts:
>>> >>
>>> >> - You've missed the object property dialogues. This really needs to be
>>> >> consistent across all dialogues :-)
>>> >>
>>> >> - The behaviour with Enter is a bit odd, because priority will be
>>> >> given the the focussed control to handle it. That's what we want if
>>> >> it's a text area for example, but on the other hand, if you open the
>>> >> Preference dialogue, then the Help button gets focus first, so Enter
>>> >> there will end up opening the docs. I suspect in most cases the
>>> >> behaviour you've got is fine, so I'm not inclined to change it. <plays
>>> >> a little> Hmm, I see similar behaviour in some other apps. Let's leave
>>> >> it as you've done it :-)
>>> >>
>>> >> Please add support for the properties dialogues, then I think we're
>>> >> good
>>> >> to go.
>>> >
>>> > Done.
>>> >>
>>> >>
>>> >> Thanks!
>>> >>
>>> >> --
>>> >> 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
>>
>>
>



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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi

Is that the correct patch? Hitting <return> in a comment text area is
still causing the dialogue to close.

Thanks.

On Mon, Jun 12, 2017 at 12:55 PM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> Hi
>
> Please find rebased patch
>
> Thanks
>
> On Mon, Jun 12, 2017 at 5:20 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Could you rebase this please?
>>
>> Thanks.
>>
>> On Fri, Jun 9, 2017 at 10:40 AM, Surinder Kumar
>> <surinder.kumar@enterprisedb.com> wrote:
>> > Ignore previous patch as it doesn't contains new changes.
>> > Please find attached patch.
>> >
>> >
>> > On Fri, Jun 9, 2017 at 3:06 PM, Surinder Kumar
>> > <surinder.kumar@enterprisedb.com> wrote:
>> >>
>> >> Hi
>> >>
>> >> PFA patch and review.
>> >>
>> >>
>> >> On Fri, Jun 9, 2017 at 1:47 PM, Dave Page <dpage@pgadmin.org> wrote:
>> >>>
>> >>> Hi
>> >>>
>> >>> Ahh, now we have a bug. In the properties dialogues, if you edit a
>> >>> comment, as soon as the Save button gets enabled, hitting Return will
>> >>> Save the changes instead of adding a newline to the comment. That's
>> >>> clearly not right :-(
>> >>
>> >> Now if event.target is its child elements(for example input, textarea
>> >> etc), the function "handleKeyDown(event)" won't call.
>> >>>
>> >>>
>> >>> On Fri, Jun 9, 2017 at 7:48 AM, Surinder Kumar
>> >>> <surinder.kumar@enterprisedb.com> wrote:
>> >>> > Hi
>> >>> >
>> >>> > Please find updated patch.
>> >>> >
>> >>> > Thanks,
>> >>> > Surinder
>> >>> >
>> >>> > On Thu, Jun 8, 2017 at 7:16 PM, Dave Page <dpage@pgadmin.org> wrote:
>> >>> >>
>> >>> >> Hi
>> >>> >>
>> >>> >> On Thu, Jun 8, 2017 at 5:38 AM, Surinder Kumar
>> >>> >> <surinder.kumar@enterprisedb.com> wrote:
>> >>> >> > Hi
>> >>> >> >
>> >>> >> > The attribute key for Ok, Save buttons should be mapped to
>> >>> >> > 13(Enter),
>> >>> >> > for Help - 112(F1) and
>> >>> >> > for Cancel - 27(Esc)
>> >>> >> > The above changes are made into all dialog related template
>> >>> >> > files.
>> >>> >> >
>> >>> >> > PFA patch and review.
>> >>> >>
>> >>> >> Looks good. Just a couple of thoughts:
>> >>> >>
>> >>> >> - You've missed the object property dialogues. This really needs to
>> >>> >> be
>> >>> >> consistent across all dialogues :-)
>> >>> >>
>> >>> >> - The behaviour with Enter is a bit odd, because priority will be
>> >>> >> given the the focussed control to handle it. That's what we want if
>> >>> >> it's a text area for example, but on the other hand, if you open
>> >>> >> the
>> >>> >> Preference dialogue, then the Help button gets focus first, so
>> >>> >> Enter
>> >>> >> there will end up opening the docs. I suspect in most cases the
>> >>> >> behaviour you've got is fine, so I'm not inclined to change it.
>> >>> >> <plays
>> >>> >> a little> Hmm, I see similar behaviour in some other apps. Let's
>> >>> >> leave
>> >>> >> it as you've done it :-)
>> >>> >>
>> >>> >> Please add support for the properties dialogues, then I think we're
>> >>> >> good
>> >>> >> to go.
>> >>> >
>> >>> > Done.
>> >>> >>
>> >>> >>
>> >>> >> Thanks!
>> >>> >>
>> >>> >> --
>> >>> >> 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
>> >>
>> >>
>> >
>>
>>
>>
>> --
>> 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

Hi

On Mon, Jun 12, 2017 at 2:03 PM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> Hi
>
> I confirm this is updated patch.

Thanks. Something else seems to have changed that I can't spot. Only
<return> should be ignored if a child control has focus - Escape and
F1 should continue to work. I tried moving the:

if (panel && panel['_type'] !== 'node_props') return;

into the branch for keyCode.ENTER, but that doesn't seem to help.

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Thanks, applied.

On Mon, Jun 12, 2017 at 3:52 PM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> Hi Dave,
>
> Please find updated patch.
>
> On Mon, Jun 12, 2017 at 7:58 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> On Mon, Jun 12, 2017 at 2:03 PM, Surinder Kumar
>> <surinder.kumar@enterprisedb.com> wrote:
>> > Hi
>> >
>> > I confirm this is updated patch.
>>
>> Thanks. Something else seems to have changed that I can't spot. Only
>> <return> should be ignored if a child control has focus - Escape and
>> F1 should continue to work. I tried moving the:
>>
>> if (panel && panel['_type'] !== 'node_props') return;
>>
>> into the branch for keyCode.ENTER, but that doesn't seem to help.
>
> Fixed.
>>
>>
>> --
>> 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