Re: Module-wise Keyboard preferences

From: Harshal Dhumal <harshal(dot)dhumal(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Harshal Dhumal <harshaldhumal15(at)gmail(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: Module-wise Keyboard preferences
Date: 2018-01-25 07:07:07
Message-ID: CAFiP3vyiqq2-LpP8yyGJjohSKAe4o5dewLhyWAaj7LsNqv5mtQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Dave,

Please find attached updated patch for keyboard preferences (infrastructure
part only).
In this patch I have changed keyboard shortcut UI in preferences dialog
and added jasmine test cases for newly added backform control.

--
*Harshal Dhumal*
*Sr. Software Engineer*

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

On Tue, Jan 23, 2018 at 5:34 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

>
>
> On Tue, Jan 23, 2018 at 12:03 PM, Harshal Dhumal <
> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>
>> On Tue, Jan 23, 2018 at 4:52 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>>
>>>
>>> On Tue, Jan 23, 2018 at 6:31 AM, Harshal Dhumal <
>>> harshaldhumal15(at)gmail(dot)com> wrote:
>>>
>>>> Hi Dave,
>>>>
>>>> On Mon, Jan 22, 2018 at 5:26 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>
>>>>> Hi
>>>>>
>>>>> On Fri, Jan 19, 2018 at 9:29 AM, Harshal Dhumal <
>>>>> harshal(dot)dhumal(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Dave,
>>>>>>
>>>>>> On Wed, Jan 17, 2018 at 4:58 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>>
>>>>>>> Hi
>>>>>>>
>>>>>>> To summarise an offline discussion we just had, the intention is to
>>>>>>> replace the switches with checkboxes for Shift, Control, and Alt/Option,
>>>>>>> then to try to come up with a set of default shortcuts that work on the
>>>>>>> major platforms/browsers. Investigation into Javascript's handling of
>>>>>>> keycodes vs. strings is ongoing to figure out the best way to capture and
>>>>>>> display the selected keys for non-ASCII locales (String.fromCharCode()).
>>>>>>>
>>>>>>
>>>>>> I have replaced switches with checkboxes and also showing all
>>>>>> controls for one preference in single line.
>>>>>>
>>>>>> [image: Inline image 2]
>>>>>>
>>>>>
>>>>> I'm still not happy with the way that looks. Wrapping "Alt/Option"
>>>>> looks horrible - plus it also looks a little odd that labels are closer to
>>>>> the preceding controls than to the following controls to which they
>>>>> actually apply.
>>>>>
>>>>>
>>>>>> Regarding String.fromCharCode() to reverse map keycode to char
>>>>>> (actual char on keyboard) does not work for all keyboard keys.
>>>>>>
>>>>>> For eg. on mac with standard keyboard and with my locale settings I
>>>>>> got below behaviour.
>>>>>> For key tilde (~) I got keycode 192 (Unicode value of the pressed
>>>>>> keyboard key as per docs) and
>>>>>> when I tried to map 192 (String.fromCharCode(192)) to get actual
>>>>>> char, I got À <http://www.codetable.net/decimal/192>
>>>>>>
>>>>>> So instead of mapping keycode to char for display purposse I'm using
>>>>>> char which is returned by event object
>>>>>>
>>>>>
>>>>> OK, sounds fine.
>>>>>
>>>>> Regarding the patch:
>>>>>
>>>>> - It's missing doc updates
>>>>>
>>>> My patch is related to add infrastructure for keyboard shortcut
>>>> preferences and not to add any particular preference.
>>>> We can add docs for actual keyboard preference when we start adding
>>>> keyboard preferences.
>>>>
>>>> I share two patches out of one is example (keyboard shortcut
>>>> preferences for debugger) and not a final patch.
>>>> It's purpose is provide example (for developers only) about how to add
>>>> keyboard preferences.
>>>>
>>>> Please let me know which doc updates are you expecting?
>>>>
>>>
>>> Good point - I missed that it's only the infrastructure part that's
>>> ready to go.
>>>
>>> My other comments stand; there's got to be a nicer way to present the
>>> UI, and I'd like to see the JS broken up into smaller chunks, with tests,
>>> if feasible.
>>>
>>
>> Yes, I have changed UI (attached screen-shot).
>>
>
> That looks much nicer.
>
>
>> Regarding JS code, almost 99% code from this patch constitutes
>> backfrom controls and control formtter (KeyCodeControl, Keyboardshort
>> cutControl, KeyCodeControlFormatter) and we never had a case
>> where we have written jasmine test cases for backform controls. I'm
>> currently exploring if we can write jasmine test cases for
>> backform control.
>>
>
> Understood.
>
>
>>
>>
>> [image: Inline image 1]
>>
>>
>>
>>> 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
>

Attachment Content-Type Size
keyboard_pref_RM2984_V2.patch text/x-patch 32.6 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2018-01-25 12:27:17 pgAdmin 4 commit: Add keyboard navigation in Query tool module via Tab/
Previous Message Joao De Almeida Pereira 2018-01-24 22:28:29 [pgadmin4][patch] Correct Bug #3050