public inbox for [email protected]
help / color / mirror / Atom feedRe: [pgAdmin4][Patch]: Using F5 for Query Execution in Query tool
2+ messages / 2 participants
[nested] [flat]
* Re: [pgAdmin4][Patch]: Using F5 for Query Execution in Query tool
@ 2016-07-27 17:09 Surinder Kumar <[email protected]>
0 siblings, 1 reply; 2+ messages in thread
From: Surinder Kumar @ 2016-07-27 17:09 UTC (permalink / raw)
To: Dave Page <[email protected]>; pgadmin-hackers; +Cc: Ashesh Vashi <[email protected]>; Harshal Dhumal <[email protected]>
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);
^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: [pgAdmin4][Patch]: Using F5 for Query Execution in Query tool
@ 2016-08-01 14:12 Dave Page <[email protected]>
parent: Surinder Kumar <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Dave Page @ 2016-08-01 14:12 UTC (permalink / raw)
To: Surinder Kumar <[email protected]>; +Cc: pgadmin-hackers; Ashesh Vashi <[email protected]>; Harshal Dhumal <[email protected]>
Thanks - applied with a couple of fixes:
- The menu option for Explain Analyze showed the shortcut as F8, not Shift+F7
- Explain Analyze was broken - the patch checked for Shift+F7 and
called on_explain_analyze, but then also checked for F7 and called
on_explain.
On Wed, Jul 27, 2016 at 6:09 PM, Surinder Kumar
<[email protected]> wrote:
> 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
>
>
--
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
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2016-08-01 14:12 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2016-07-27 17:09 Re: [pgAdmin4][Patch]: Using F5 for Query Execution in Query tool Surinder Kumar <[email protected]>
2016-08-01 14:12 ` Dave Page <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox