Re: Fix for issue RM1336 [pgadmin4]

public inbox for [email protected]  
help / color / mirror / Atom feed

From: Dave Page <[email protected]>
To: Harshal Dhumal <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: Fix for issue RM1336 [pgadmin4]
Date: Fri, 1 Jul 2016 13:16:04 +0100
Message-ID: <CA+OCxoxRnxnJjKocRccY4_vQCuHX9U-gjGsEeUv+KxT-Q5T1sA@mail.gmail.com> (raw)
In-Reply-To: <CAFiP3vzaSHvdgC3y8DLeR8XHNHOA6_FXwJ4BA+WT0SJuFGFrvQ@mail.gmail.com>
References: <CAFiP3vxoqJ8Rd2fiXev2di1Se_WuyAZdMtUHcHG3HuHwk-vj0g@mail.gmail.com>
	<CA+OCxozZz4q4Cac8sS2wZ0OprfjOKQRqmSXQjHdd=-sBoVfsbA@mail.gmail.com>
	<CAFiP3vxfQ4=+zjLW9HR_dtYkbgfZLvKLnLZi-A3m4rbFH_A5xA@mail.gmail.com>
	<CAFiP3vznYPXdvmGj2J7i4S7e-BPUVWWUC5KFAK=Q7n6Nee5ThA@mail.gmail.com>
	<CA+OCxoza3djrtauF1VqSvqU1pXBr8gAzHD9xF1T37HbkbgEztA@mail.gmail.com>
	<CAFiP3vyTB+ceHGxwXn_w8RBmDQn9SVC3i+1kXhv=CD8UWr3=Xg@mail.gmail.com>
	<CA+OCxowGax9fd5YmTG3u82Ums0rwkETiXFQPuBokKBx_Br8BOA@mail.gmail.com>
	<CAG7mmozksaXEZv=Sz-f4vkHgDjn-_5LTBzP5=wP-+RNeCMVH3Q@mail.gmail.com>
	<CA+OCxoy=cO2feBinWxO8hMuUN3x9+DFeKkDZ-5hv=2NWfht-dw@mail.gmail.com>
	<CAFiP3vxq4-jHf0GALVH=eaQM5d+rrdu8Z49Q1ksL4ftKUzzVyQ@mail.gmail.com>
	<CA+OCxowSS8R1zh1jajhhN6ibtH4x1+w=xwY5i11_Ub7zCJ9JFQ@mail.gmail.com>
	<CAMjNa7e3vkEsiRGqt6kw3Z+z6v-z6Ox4-F0VmCDJEGW2pynWOg@mail.gmail.com>
	<CA+OCxoxbbVX6HZJVeXxRFhbkiG692RZx9WoWo=jmkv6gc79eRA@mail.gmail.com>
	<[email protected]>
	<CA+OCxowW1+ezv93fdbvKtmper+jWOfs+XbgdCSMUJiH23xHQAA@mail.gmail.com>
	<CAFiP3vywwsR96uwDd-KzwF1Vc3DEaBnKWT5v4Sc6y3f-FstCGQ@mail.gmail.com>
	<CA+OCxowF=XkYmrv4uggjW=rWDyS71+ZFhf-sdB-+5beU0_o=jQ@mail.gmail.com>
	<CAFiP3vxp-Y0OYuDMYR7Fbe9g3j5EKSxvSk=iz3MmXeSjsSs+hw@mail.gmail.com>
	<CA+OCxozxp4ssW6=0YcY2dOY_htUsP2dPwi5ZXyONg3+FomScww@mail.gmail.com>
	<CAFiP3vzLfX+8LYSgCvno3R=3kLkdnU7NR_hgwVC+OsT-i6FSCA@mail.gmail.com>
	<CA+OCxoy20eBO-z9jK9eNJQvoEYOCOQrB4hs-9rgLUc1V=hwN9w@mail.gmail.com>
	<CAFiP3vzaSHvdgC3y8DLeR8XHNHOA6_FXwJ4BA+WT0SJuFGFrvQ@mail.gmail.com>
List-Unsubscribe:  <mailto:[email protected]?body=unsub%20pgadmin-hackers>

Hi

On Fri, Jul 1, 2016 at 11:10 AM, Harshal Dhumal
<[email protected]> wrote:
> Hi,
>
> PFA patch
>
> This contains:
> 1] All shortcuts' list which we are using in pgAdmin4.
> 2] Fixed shortcut display tooltips.
>
>
> --
> Harshal Dhumal
> Software Engineer
>
> EnterpriseDB India: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
> On Thu, Jun 30, 2016 at 2:34 PM, Dave Page <[email protected]> wrote:
>>
>> Hi Harshal,
>>
>> On Thu, Jun 30, 2016 at 7:59 AM, Harshal Dhumal
>> <[email protected]> wrote:
>> > Hi Dave,
>> >
>> > Somehow control key is mapped to Command key in Mac. So on Mac shortcuts
>> > are
>> > Shift+Command+E, Shift+Command+X, Shift+Command+A
>>
>> OK, those work, but I think we need to take a step back here (partly
>> because Cmd+Shift+A seems to be Select All in CodeMirror on Mac). I've
>> committed the patch now, but changed Ctrl+Shift+A to Ctrl+Shift+N.
>>
>> Can you please work on the following:
>>
>> 1) Create a table of shortcuts from our runtime, our web app, and
>> CodeMirror. This should list:
>>
>> Component (e.g. runtime, CodeMirror, Query Tool)
>> Standard shortcut (e.g. Ctrl+Shift+A)
>> Mac shortcut (e.g. Cmd+Shift+A)
>> Function (e.g. Explain Analyze or Select All)
>>
>> Let's make this as complete and accurate as possible, so it can be
>> included in the documentation, and used by us to select or
>> de-duplicate shortcuts.
>
> Added shortcuts' list.

Let's keep it as a separate file for now, and not part of the source
tree or any patches.

I notice that it's missing much of what I wanted to collect though, e.g.

Ctrl+Space - Autocomplete(?)
Cut
Copy
Paste
Select All
...

The other CodeMirror keys can be found at
https://codemirror.net/doc/manual.html#keymaps. Please add at least
the basic editting commands.

>> 2) Confirm that the shortcuts we're using in our runtime and web
>> application don't conflict with any in CodeMirror (or web browsers, in
>> the case of the web app).
>
> Tested. No other conflicting shortcuts found.

Good.

>>
>> 3) Update the web application so the shortcuts are correctly displayed
>> on Mac automatically - e.g. the tooltips and menus should show
>> Cmd+Shift not Ctrl+Shift
>
> Fixed
>
>>
>>
>> 4) Investigate #1360, and ensure that the CodeMirror shortcuts work
>> consistently between the runtime and browsers on all platforms.
>
> Investigating now.

You've already found one inconsistency - Cmd vs. Ctrl on Mac browsers
vs. the runtime. It should be Cmd in either UI, to avoid user
confusion.

>>
>> 5) Investigate any remaining shortcuts that don't work as expected.
>
>
> Investigating now.: Codemirror shortcut (Cmd/Ctrl+Shift+A)  "Select all"
> only works on Mac (web/runtime) not on linux platform (haven't tested on
> windows)

Yeah - see also some of the comments I made on
https://redmine.postgresql.org/issues/1360

Let's figure out what's wrong first, then come up with a set of
fixes/changes that will get us to a consistent set of keys that work
as per the list you're preparing.

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

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


-- 
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

view thread (26+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: Fix for issue RM1336 [pgadmin4]
  In-Reply-To: <CA+OCxoxRnxnJjKocRccY4_vQCuHX9U-gjGsEeUv+KxT-Q5T1sA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox