Обсуждение: Module-wise Keyboard preferences

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()).

On Wed, Jan 17, 2018 at 6:34 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:

Hi Team,

I'm attaching initial patch for feature module-wise keyboard preference.

Also apart from main patch I have also attached one sample debugger 

keyboard shortcut patch to illustrate how to add module-wise preferences

-- 

Harshal Dhumal

Sr. Software Engineer

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

--

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

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

Hi Dave,

On Wed, Jan 17, 2018 at 4:58 PM, Dave Page <dpage@pgadmin.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.

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 À

So instead of mapping keycode to char for display purposse I'm using char which is returned by event object 

Hi

On Fri, Jan 19, 2018 at 9:29 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:

Hi Dave,

On Wed, Jan 17, 2018 at 4:58 PM, Dave Page <dpage@pgadmin.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.

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 À

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

- There's a huge chunk of JS in there. Can that be split up into smaller chunks and unit tests created for them?

Thanks.

--

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

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

It would be easier for users if we determined the best shortcut for these are and then list them rather than give people options.

For example:

On Tue, Jan 23, 2018 at 2:31 PM Harshal Dhumal <harshaldhumal15@gmail.com> wrote:

Hi Dave,

On Mon, Jan 22, 2018 at 5:26 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Fri, Jan 19, 2018 at 9:29 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:

Hi Dave,

On Wed, Jan 17, 2018 at 4:58 PM, Dave Page <dpage@pgadmin.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.

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 À

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?

 

- There's a huge chunk of JS in there. Can that be split up into smaller chunks and unit tests created for them?

Thanks.

--

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

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

Oh interesting. Was there a particular reason why they wanted it configurable? 

On Tue, Jan 23, 2018 at 6:24 PM Dave Page <dpage@pgadmin.org> wrote:

On Tue, Jan 23, 2018 at 10:20 AM, Shirley Wang <swang@pivotal.io> wrote:

It would be easier for users if we determined the best shortcut for these are and then list them rather than give people options.

For the defaults, we will (where possible). The problem is that we're extremely limited in what we can use as there's too much conflict between browsers.

Not to mention that users have asked us to make this configurable a number of times.

 

For example:

On Tue, Jan 23, 2018 at 2:31 PM Harshal Dhumal <harshaldhumal15@gmail.com> wrote:

Hi Dave,

On Mon, Jan 22, 2018 at 5:26 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Fri, Jan 19, 2018 at 9:29 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:

Hi Dave,

On Wed, Jan 17, 2018 at 4:58 PM, Dave Page <dpage@pgadmin.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.

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 À

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?

 

- There's a huge chunk of JS in there. Can that be split up into smaller chunks and unit tests created for them?

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

Hi Dave,

On Mon, Jan 22, 2018 at 5:26 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Fri, Jan 19, 2018 at 9:29 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:

Hi Dave,

On Wed, Jan 17, 2018 at 4:58 PM, Dave Page <dpage@pgadmin.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.

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 À

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?

On Tue, Jan 23, 2018 at 6:31 AM, Harshal Dhumal <harshaldhumal15@gmail.com> wrote:

Hi Dave,

On Mon, Jan 22, 2018 at 5:26 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Fri, Jan 19, 2018 at 9:29 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:

Hi Dave,

On Wed, Jan 17, 2018 at 4:58 PM, Dave Page <dpage@pgadmin.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.

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 À

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.

Thanks.

--

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

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

On Tue, Jan 23, 2018 at 4:52 PM, Dave Page <dpage@pgadmin.org> wrote:

On Tue, Jan 23, 2018 at 6:31 AM, Harshal Dhumal <harshaldhumal15@gmail.com> wrote:

Hi Dave,

On Mon, Jan 22, 2018 at 5:26 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Fri, Jan 19, 2018 at 9:29 AM, Harshal Dhumal <harshal.dhumal@enterprisedb.com> wrote:

Hi Dave,

On Wed, Jan 17, 2018 at 4:58 PM, Dave Page <dpage@pgadmin.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.

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 À

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).

Regarding JS code, almost 99% code from this patch constitutes

backfrom controls and control formtter (KeyCodeControl, KeyboardshortcutControl, 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.

 

Thanks.

--

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

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