public inbox for [email protected]
help / color / mirror / Atom feedFrom: Surinder Kumar <[email protected]>
To: Dave Page <[email protected]>
To: pgadmin-hackers <[email protected]>
Cc: Ashesh Vashi <[email protected]>
Cc: Harshal Dhumal <[email protected]>
Subject: Re: [pgAdmin4][Patch]: Using F5 for Query Execution in Query tool
Date: Wed, 27 Jul 2016 22:39:16 +0530
Message-ID: <CAM5-9D_2h3uTOPagHNTXAFmgK_UmNjUJjc_j4zhS1NayccvEQg@mail.gmail.com> (raw)
In-Reply-To: <CAM5-9D8=SR_EbapHfDGbHh=FNkszWswmh51FD8-Z0CuAn+zcyg@mail.gmail.com>
References: <CAM5-9D_k4UbuHWku7rKX-5TJ4sS=AbRy6-z5O2gfDz4zLoQ=2g@mail.gmail.com>
<CA+OCxoyeh-fidUJBA2g8AoeOmh6vqZO4hYBwL-jpzAjEapOK9w@mail.gmail.com>
<CAM5-9D8JPAErQPFe7i4bVT1FWZVHheUmCkUGCaVJxkHUbj1Fjg@mail.gmail.com>
<CAM5-9D8=SR_EbapHfDGbHh=FNkszWswmh51FD8-Z0CuAn+zcyg@mail.gmail.com>
List-Unsubscribe: <mailto:[email protected]?body=unsub%20pgadmin-hackers>
Hi
Please find attached patch with following fixes:
1) This patch adds support for Function Keys(F5, F7, F8) as keyboard
shortcuts.
Following are the keyboard shortcuts for query tool operations such as:
- Execute query --> *F5*
- Explain query --> *F7*
- Explain analyze query --> *Shift + F7*
- Download query result as CSV --> *F8*
2) Add proper condition to check string in JS array in explain analyze. The
code breaks while running explain analyse.
This patch partially resolves *RM1478*.
Please review.
On Wed, Jul 27, 2016 at 5:48 PM, Surinder Kumar <
[email protected]> wrote:
> On Wed, Jul 27, 2016 at 2:31 PM, Dave Page <[email protected]> wrote:
>
> >
>
> > Hi
> >
> > On Tue, Jul 26, 2016 at 7:13 PM, Surinder Kumar
> > <[email protected]> wrote:
> > > Hi Dave
> > >
> > > I spend some time looking at RM#1412 - Function Keys in Query Tool
> specific
> > > to execute query using F5 key.
> >
> > Please note that RM1412 was rejected. 1478 is where we're tracking all
> > the shortcut issues now.
> >
> > > As we know F5 key is bound to reload/refresh for browsers on Linux &
> > > Windows.
> > > But If we use F5 key to execute query, then F5 key event is first
> captured
> > > inside the code and execution works.
> >
> > OK.
> >
> > > It will not affect the browser reload behaviour. F5 key for execution
> will
> > > only work if focus is on Query tool.
> > >
> > > Ashesh's concern is:
> > > If focus is not on query tool and user mistakenly pressed Fn+F5 key,
> the
> > > page will reload which shouldn't happened.
> > >
> > > In such case:
> > > The page doesn't reload directly as we always ask user "Whether to
> Stay on
> > > page or Leave page".
> >
> > Right.
> >
> > > The other way is to disable reload page on F5 Key only when focus is
> inside
> > > the app.
> > > If focus is on browser then F5 for reload will work. (not a good idea)
> > >
> > > @Dave What do you think?
> >
> > On Mac it works nicely, both in the runtime and in Chrome, Safari and
> > Firefox - and given the number of people complaining about the
> > shortcuts here, I think the limitation of focus on the text box is
> > acceptable.
> >
> > > I have tested this attached patch on following platforms without any
> issue:
> > > Windows 7 - Runtime & IE 9 & 10.
> > > Mac OSX Yosemite - Safari(8.0.3), Chrome(51) & Firefox(47)
> > > Ubuntu-14.04.2 - Chrome & Firefox.
> > >
> > > Please review the patch.
> >
> > The patch doesn't update the tooltips/buttons/menus to display the
> > correct shortcuts - plus, if we're doing this for F5, then I think we
> > should also keep F7 for Explain, Shift+F7 for Explain Analyse and F8
> > for Execute to File (CSV Downlaod), like pgAdmin 3.
>
Tooltips/buttons/menus are updated with correct shortcuts keys in attached
patch.
> I will send a patch later today.
>
>
> >
> >
> > Please note though that that would only partially resolve RM1478. On
> > some browsers/platforms, there is also some CodeMirror related
> > shortcut weirdness that needs to be resolved. It may be worth seeing
> > if a CodeMirror update would help.
> Okay
> >
> > Thanks!
> >
> > --
> > 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
Attachments:
[application/octet-stream] support_for_function_keys.patch (4.9K, 3-support_for_function_keys.patch)
download | inline diff:
diff --git a/web/pgadmin/tools/datagrid/templates/datagrid/index.html b/web/pgadmin/tools/datagrid/templates/datagrid/index.html
index d1010bd..537685c 100644
--- a/web/pgadmin/tools/datagrid/templates/datagrid/index.html
+++ b/web/pgadmin/tools/datagrid/templates/datagrid/index.html
@@ -86,7 +86,7 @@
</div>
<div class="btn-group" role="group" aria-label="">
<button id="btn-flash" type="button" class="btn btn-default" style="width: 40px;"
- title="{{ _('Execute/Refresh (Ctrl+Shift+E)') }}">
+ title="{{ _('Execute/Refresh (F5)') }}">
<i class="fa fa-bolt" aria-hidden="true"></i>
</button>
<button id="btn-query-dropdown" type="button" class="btn btn-default dropdown-toggle"
@@ -96,12 +96,12 @@
<ul class="dropdown-menu dropdown-menu">
<li>
<a id="btn-explain" href="#">
- <span>{{ _('Explain (Ctrl+Shift+X)') }}</span>
+ <span>{{ _('Explain (F7)') }}</span>
</a>
</li>
<li>
<a id="btn-explain-analyze" href="#">
- <span>{{ _('Explain analyze (Ctrl+Shift+N)') }}</span>
+ <span>{{ _('Explain analyze (F8)') }}</span>
</a>
</li>
<li class="divider"></li>
@@ -175,7 +175,7 @@
</ul>
</div>
<div class="btn-group" role="group" aria-label="">
- <button id="btn-download" type="button" class="btn btn-default" title="{{ _('Download as CSV') }}">
+ <button id="btn-download" type="button" class="btn btn-default" title="{{ _('Download as CSV (F8)') }}">
<i class="fa fa-download" aria-hidden="true"></i>
</button>
</div>
diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
index 8961c55..2e6426a 100644
--- a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
+++ b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js
@@ -27,6 +27,11 @@ define(
*/
function epicRandomString(b){for(var a=(Math.random()*eval("1e"+~~(50*Math.random()+50))).toString(36).split(""),c=3;c<a.length;c++)c==~~(Math.random()*c)+1&&a[c].match(/[a-z]/)&&(a[c]=a[c].toUpperCase());a=a.join("");a=a.substr(~~(Math.random()*~~(a.length/3)),~~(Math.random()*(a.length-~~(a.length/3*2)+1))+~~(a.length/3*2));if(24>b)return b?a.substr(a,b):a;a=a.substr(a,b);if(a.length==b)return a;for(;a.length<b;)a+=epicRandomString();return a.substr(0,b)};
+ // Define key codes for shortcut keys
+ var F5_KEY = 116,
+ F7_KEY = 118,
+ F8_KEY = 119;
+
// Defining the backbone model for the sql grid
var sqlEditorViewModel = Backbone.Model.extend({
@@ -1030,22 +1035,35 @@ define(
);
},
- // Callback for keyboard event
+ /*
+ * Callbacks for keyboard events bind to the
+ * query tool buttons.
+ * Following are the keyboard shortcuts:
+ * F5 - Execute query
+ * F7 - Explain query
+ * F8 - Download result as CSV
+ * Shift+F7 - Explain analyze query
+ */
keyAction: function(ev) {
- if(ev.ctrlKey && ev.shiftKey) {
- if(ev.keyCode == 69) {
- // char e/E
- // Execute query.
- this.on_flash(ev);
- } else if(ev.keyCode == 88) {
- // char x/X
- // Explain query.
- this.on_explain(ev);
- } else if(ev.keyCode == 78) {
- // char n/N
- // Explain analyze query.
- this.on_explain_analyze(ev);
- }
+ var keyCode = ev.which || ev.keyCode;
+ if (ev.shiftKey && keyCode == F7_KEY) {
+ // Explain analyze query.
+ this.on_explain_analyze(ev);
+ ev.preventDefault();
+ }
+
+ if (keyCode == F5_KEY) {
+ // Execute query.
+ this.on_flash(ev);
+ ev.preventDefault();
+ } else if (keyCode == F7_KEY) {
+ // Explain query.
+ this.on_explain(ev);
+ ev.preventDefault();
+ } else if (keyCode == F8_KEY) {
+ // Download query result as CSV.
+ this.on_download(ev);
+ ev.preventDefault();
}
}
});
@@ -1432,7 +1450,7 @@ define(
*/
var explain_data_array = [];
if(data.result &&
- 'QUERY PLAN' in data.result[0] &&
+ data.result[0] && data.result[0].hasOwnProperty('QUERY PLAN') &&
_.isObject(data.result[0]['QUERY PLAN'])) {
var explain_data = {'QUERY PLAN' : JSON.stringify(data.result[0]['QUERY PLAN'], null, 2)};
explain_data_array.push(explain_data);
view thread (2+ 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], [email protected], [email protected]
Subject: Re: [pgAdmin4][Patch]: Using F5 for Query Execution in Query tool
In-Reply-To: <CAM5-9D_2h3uTOPagHNTXAFmgK_UmNjUJjc_j4zhS1NayccvEQg@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