Re: RM2898 Keyboard navigation in dialog tabs (nav tabs)

Yep I installed the V2 file

On Wed, Feb 21, 2018 at 11:31 AM Dave Page <dpage(at)pgadmin(dot)org> wrote:

> On Wed, Feb 21, 2018 at 4:22 PM, Joao De Almeida Pereira <
> jdealmeidapereira(at)pivotal(dot)io> wrote:
>
>> Hello Harshal,
>>
>> I passed the patch through our CI and all the tests passed. The changes
>> do not break previous behavior but because there are no tests on the new
>> feature we could not be sure it was really working. So we did some manual
>> testing and sometimes it doesn't work, like it gets stuck in a place and
>> you need to press the shortcut again in order for it to move.
>>
>
> Was that with the updated patch? It sounds like the issue I saw with the
> original one.
>
>
>>
>> Codewise I have some suggestions:
>> - dialogTabNavigator looks a nice candidate for a class with its own
>> file. This way we can test the behavior
>> - There is no difference between a variable called e and a variable
>> error so for sake of clarity I would love to see variable names that we
>> can easily read
>> - We are also using 2 different types of variable naming camelCase and
>> snake_case, if we could use only camelCase on Javascript it would make the
>> code more uniform
>> - I noticed that there are some linting issues in the Javascript code
>>
>> Summing it up I believe that despite the feature not working 100% of the
>> time, looks like the code is almost there but needs some refactoring to
>> make it more readable, instead of comments we could have function calls
>> that more clearly state what we are looking for something like
>> DialogTabNavigator.isLastTabOfChild
>> ​
>> ​
>>
>>
>> Thanks
>> Joao
>>
>> On Wed, Feb 21, 2018 at 4:32 AM Harshal Dhumal <
>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>
>>> Hi,
>>>
>>> --
>>> *Harshal Dhumal*
>>> *Sr. Software Engineer*
>>>
>>> EnterpriseDB India: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> On Tue, Feb 20, 2018 at 10:34 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>> Hi
>>>>
>>>> On Tue, Feb 20, 2018 at 7:22 AM, Harshal Dhumal <
>>>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Please find attached patch to enable keyboard navigation in dialog.
>>>>>
>>>>> To allow navigation from one tab pane (bootstrap tab pane) to another
>>>>> one
>>>>> I have added two new shortcut preferences *Dialog tab previous *(
>>>>> shift+control+[ ) and *Dialog tab next* ( shift+control+] ) for backward
>>>>> and forward tab navigation.
>>>>>
>>>>> Also all dialog controls (within same tab pane) can be navigated using
>>>>> TAB key.
>>>>>
>>>>
>>>>
>>>
>>>> This seems unreliable to me - for example, it keeps getting stuck on
>>>> the connection tab on the server properties dialog.
>>>>
>>>>
>>>
>>>> Also, can we use the same wording as for the tabbed panel navigation
>>>> please? E.g. Next/Previous instead of Forward/Back.
>>>>
>>>
>>> I have fixed all of above issues. Please find updated patch.
>>>
>>> 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
>