Tryton Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(309)

Issue 32881002: sao: Add keyboard shortcuts

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 months, 2 weeks ago by perilla
Modified:
2 months, 1 week ago
Reviewers:
pokoli, ced, reviewbot
Visibility:
Public.

Description

I think that use alt key instead ctrl key is a less invasive solution in order to not replace native browser shortcuts. According to irc log https://www.tryton.org/~irclog/2016-06-02.log.html#t2016-06-02 11:25 require:https://tryton-rietveld.appspot.com/31931002/ issue6104

Patch Set 1 #

Total comments: 4

Patch Set 2 : Shorcut on tab apply only on last tab. Definition on screen apply to all #

Patch Set 3 : Register shorcuts once on current tab #

Patch Set 4 : Adding global search shortcut #

Total comments: 1

Patch Set 5 : Register shorcuts on document ready #

Patch Set 6 : shortcut definition on separate function in order to reuse when create dialog #

Patch Set 7 : Callbacks only on allowed view types, example (pagedown and pageup only on tree view) #

Patch Set 8 : Evaluating if there is at least one view open, if shortcut is not global #

Patch Set 9 : Support for next and previous tab with alt+pagedown and alt+pageup #

Total comments: 2

Patch Set 10 : Reuse jQuery(document) #

Patch Set 11 : dont use anonymous function #

Patch Set 12 : Added global search on set_shortcuts #

Total comments: 2

Patch Set 13 : Define global shortcuts one by one, and dont use mousetrap inside dropdown menus #

Total comments: 17

Patch Set 14 : update to tip, and apply ced comments #

Total comments: 2

Patch Set 15 : Replace up and down by alt+up and alt+down, remove search for 'a' tag #

Total comments: 8

Patch Set 16 : Add changelog, modal dialog #

Patch Set 17 : Update to tip, remove kbd css #

Total comments: 19

Patch Set 18 : Apply fixes #

Patch Set 19 : Translated global shortcuts #

Total comments: 10

Patch Set 20 : Fix comments #

Patch Set 21 : Close, switch, reload available on all views #

Total comments: 31

Patch Set 22 : Fix comments and optimizations #

Total comments: 26

Patch Set 23 : Fix comments #

Patch Set 24 : Update to tip #

Patch Set 25 : Fix shortcut documentation, update to tip #

Patch Set 26 : Adding return false on all cases #

Patch Set 27 : update to tip #

Patch Set 28 : update to tip #

Patch Set 29 : update to tip #

Patch Set 30 : update to tip #

Unified diffs Side-by-side diffs Delta from patch set Stats (+138 lines, -13 lines) Patch
M CHANGELOG View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 1 chunk +2 lines, -0 lines 0 comments Download
M bower.json View 1 1 chunk +2 lines, -1 line 0 comments Download
M index.html View 1 1 chunk +1 line, -0 lines 0 comments Download
M src/sao.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 2 chunks +104 lines, -0 lines 0 comments Download
M src/tab.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 2 chunks +18 lines, -1 line 0 comments Download
M src/view/form.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 11 chunks +11 lines, -11 lines 0 comments Download

Messages

Total messages: 91
perilla
7 months, 2 weeks ago (2016-12-10 04:46:54 UTC) #1
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
7 months, 2 weeks ago (2016-12-10 05:02:14 UTC) #2
ced
Why is moustrap class added? How does it work when there are many tabs open? ...
7 months, 2 weeks ago (2016-12-10 09:36:20 UTC) #3
perilla
Shorcut on tab apply only on last tab. Definition on screen apply to all
7 months, 2 weeks ago (2016-12-10 12:59:07 UTC) #4
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
7 months, 2 weeks ago (2016-12-10 13:01:56 UTC) #5
perilla
On 2016/12/10 09:36:20, ced wrote: > Why is moustrap class added? > > How does ...
7 months, 2 weeks ago (2016-12-10 13:03:21 UTC) #6
ced
On 2016/12/10 13:03:21, perilla wrote: > On 2016/12/10 09:36:20, ced wrote: > > Why is ...
7 months, 2 weeks ago (2016-12-10 13:14:58 UTC) #7
perilla
Register shorcuts once on current tab
7 months, 2 weeks ago (2016-12-11 03:11:11 UTC) #8
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
7 months, 2 weeks ago (2016-12-11 03:25:36 UTC) #9
perilla
Adding global search shortcut
7 months, 2 weeks ago (2016-12-11 05:20:56 UTC) #10
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
7 months, 2 weeks ago (2016-12-11 05:24:30 UTC) #11
ced
I do not think shortcuts should be set on tab creation. Indeed I think they ...
7 months, 2 weeks ago (2016-12-11 10:57:52 UTC) #12
perilla
Register shorcuts on document ready
7 months, 2 weeks ago (2016-12-12 03:24:04 UTC) #13
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
7 months, 2 weeks ago (2016-12-12 03:34:44 UTC) #14
perilla
shortcut definition on separate function in order to reuse when create dialog
7 months, 2 weeks ago (2016-12-12 03:40:01 UTC) #15
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
7 months, 2 weeks ago (2016-12-12 03:53:06 UTC) #16
perilla
Callbacks only on allowed view types, example (pagedown and pageup only on tree view)
7 months, 2 weeks ago (2016-12-13 13:21:05 UTC) #17
perilla
Evaluating if there is at least one view open, if shortcut is not global
7 months, 2 weeks ago (2016-12-13 13:30:10 UTC) #18
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
7 months, 2 weeks ago (2016-12-13 13:33:31 UTC) #19
perilla
Support for next and previous tab with alt+pagedown and alt+pageup
7 months, 1 week ago (2016-12-14 10:52:04 UTC) #20
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
7 months, 1 week ago (2016-12-14 11:02:46 UTC) #21
pokoli
https://tryton-rietveld.appspot.com/32881002/diff/150001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/150001/src/sao.js#newcode587 src/sao.js:587: jQuery(document).ready(function() { is really required to call aonther time ...
7 months, 1 week ago (2016-12-14 11:24:53 UTC) #22
perilla
Reuse jQuery(document)
7 months, 1 week ago (2016-12-14 12:51:02 UTC) #23
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
7 months, 1 week ago (2016-12-14 12:55:33 UTC) #24
perilla
dont use anonymous function
7 months, 1 week ago (2016-12-14 18:06:37 UTC) #25
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
7 months, 1 week ago (2016-12-14 18:29:07 UTC) #26
perilla
Added global search on set_shortcuts
7 months, 1 week ago (2016-12-14 20:01:37 UTC) #27
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
7 months, 1 week ago (2016-12-14 20:03:01 UTC) #28
pokoli
https://tryton-rietveld.appspot.com/32881002/diff/210001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/210001/src/sao.js#newcode625 src/sao.js:625: //TODO Add global shorcuts (global search, ...) This is ...
7 months, 1 week ago (2016-12-15 08:45:05 UTC) #29
perilla
Define global shortcuts one by one, and dont use mousetrap inside dropdown menus
7 months, 1 week ago (2016-12-15 13:01:17 UTC) #30
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
7 months, 1 week ago (2016-12-15 13:01:55 UTC) #31
ced
https://tryton-rietveld.appspot.com/32881002/diff/230001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/230001/src/sao.js#newcode588 src/sao.js:588: .on('ready', set_shortcuts); I think it is better to use ...
7 months ago (2016-12-26 15:43:32 UTC) #32
perilla
update to tip, and apply ced comments
6 months, 2 weeks ago (2017-01-08 04:21:16 UTC) #33
perilla
https://tryton-rietveld.appspot.com/32881002/diff/230001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/230001/src/sao.js#newcode588 src/sao.js:588: .on('ready', set_shortcuts); On 2016/12/26 15:43:31, ced wrote: > I ...
6 months, 2 weeks ago (2017-01-08 04:25:19 UTC) #34
perilla
https://tryton-rietveld.appspot.com/32881002/diff/250001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/250001/src/sao.js#newcode641 src/sao.js:641: current_tab.el.find('a[id="' + func + '"]').click(); find button link on ...
6 months, 2 weeks ago (2017-01-08 04:31:06 UTC) #35
perilla
6 months, 2 weeks ago (2017-01-08 04:31:34 UTC) #36
perilla
6 months, 2 weeks ago (2017-01-08 04:31:59 UTC) #37
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
6 months, 2 weeks ago (2017-01-08 04:45:02 UTC) #38
ced
https://tryton-rietveld.appspot.com/32881002/diff/230001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/230001/src/sao.js#newcode638 src/sao.js:638: document.activeElement.tagName != 'A') { On 2017/01/08 04:25:19, perilla wrote: ...
6 months, 2 weeks ago (2017-01-08 10:33:05 UTC) #39
perilla
On 2017/01/08 10:33:05, ced wrote: > https://tryton-rietveld.appspot.com/32881002/diff/230001/src/sao.js > File src/sao.js (right): > > https://tryton-rietveld.appspot.com/32881002/diff/230001/src/sao.js#newcode638 > ...
6 months, 2 weeks ago (2017-01-08 11:47:25 UTC) #40
ced
On 2017/01/08 11:47:25, perilla wrote: > On 2017/01/08 10:33:05, ced wrote: > > https://tryton-rietveld.appspot.com/32881002/diff/230001/src/sao.js > ...
6 months, 2 weeks ago (2017-01-08 11:56:15 UTC) #41
perilla
Replace up and down by alt+up and alt+down, remove search for 'a' tag
6 months, 2 weeks ago (2017-01-08 17:42:18 UTC) #42
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
6 months, 2 weeks ago (2017-01-08 17:48:28 UTC) #43
ced
Missing a dialog which explain all the shortcuts. https://tryton-rietveld.appspot.com/32881002/diff/270001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/270001/src/sao.js#newcode588 src/sao.js:588: jQuery(document).ready(function() ...
6 months, 2 weeks ago (2017-01-10 10:29:22 UTC) #44
ced
On 2017/01/10 10:29:22, ced wrote: > Missing a dialog which explain all the shortcuts. And ...
6 months, 2 weeks ago (2017-01-10 10:29:46 UTC) #45
perilla
https://tryton-rietveld.appspot.com/32881002/diff/270001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/270001/src/sao.js#newcode588 src/sao.js:588: jQuery(document).ready(function() { On 2017/01/10 10:29:22, ced wrote: > Why ...
6 months, 1 week ago (2017-01-20 04:53:42 UTC) #46
perilla
Add changelog, modal dialog
6 months, 1 week ago (2017-01-20 04:54:48 UTC) #47
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
6 months, 1 week ago (2017-01-20 05:11:59 UTC) #48
perilla
Remove kbd css, add missing semicolon
6 months ago (2017-01-20 13:12:05 UTC) #49
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
6 months ago (2017-01-20 13:13:51 UTC) #50
perilla
Update to tip, remove kbd css
6 months ago (2017-01-20 14:25:33 UTC) #51
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
6 months ago (2017-01-20 14:38:33 UTC) #52
ced
You should write the final commit message. https://tryton-rietveld.appspot.com/32881002/diff/270001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/270001/src/sao.js#newcode620 src/sao.js:620: [Sao.i18n.gettext('Next'), 'next', ...
6 months ago (2017-01-21 00:31:50 UTC) #53
perilla
''
6 months ago (2017-01-23 21:33:03 UTC) #54
perilla
https://tryton-rietveld.appspot.com/32881002/diff/330001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/330001/src/sao.js#newcode590 src/sao.js:590: }); On 2017/01/21 00:31:49, ced wrote: > I still ...
6 months ago (2017-01-23 21:36:24 UTC) #55
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
6 months ago (2017-01-23 21:39:03 UTC) #56
ced
https://tryton-rietveld.appspot.com/32881002/diff/330001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/330001/src/sao.js#newcode700 src/sao.js:700: ['Previous tab', 'alt+pageup'], On 2017/01/23 21:36:23, perilla wrote: > ...
6 months ago (2017-01-23 21:58:15 UTC) #57
perilla
Translated global shortcuts
6 months ago (2017-01-23 22:28:30 UTC) #58
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
6 months ago (2017-01-23 22:40:27 UTC) #59
pokoli
https://tryton-rietveld.appspot.com/32881002/diff/370001/CHANGELOG File CHANGELOG (right): https://tryton-rietveld.appspot.com/32881002/diff/370001/CHANGELOG#newcode1 CHANGELOG:1: * Add keyboard shortcuts Must be updated as there ...
6 months ago (2017-01-26 15:19:39 UTC) #60
perilla
Fix comments
5 months, 4 weeks ago (2017-01-27 15:16:36 UTC) #61
perilla
https://tryton-rietveld.appspot.com/32881002/diff/370001/CHANGELOG File CHANGELOG (right): https://tryton-rietveld.appspot.com/32881002/diff/370001/CHANGELOG#newcode1 CHANGELOG:1: * Add keyboard shortcuts On 2017/01/26 15:19:39, pokoli wrote: ...
5 months, 4 weeks ago (2017-01-27 15:47:50 UTC) #62
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
5 months, 4 weeks ago (2017-01-27 15:49:00 UTC) #63
perilla
Close, switch, reload available on all views
5 months, 4 weeks ago (2017-01-27 16:00:53 UTC) #64
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
5 months, 4 weeks ago (2017-01-27 16:17:16 UTC) #65
ced
The description of this review has a better place on the tracker. Here it should ...
5 months, 4 weeks ago (2017-01-28 23:58:21 UTC) #66
perilla
https://tryton-rietveld.appspot.com/32881002/diff/410001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/410001/src/sao.js#newcode617 src/sao.js:617: [Sao.i18n.gettext('Search'), 'search', 'alt+f', ['tree']], On 2017/01/28 23:58:20, ced wrote: ...
5 months, 3 weeks ago (2017-02-01 14:05:17 UTC) #67
perilla
Fix comments and optimizations
5 months, 3 weeks ago (2017-02-01 17:02:57 UTC) #68
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
5 months, 3 weeks ago (2017-02-01 17:17:35 UTC) #69
ced
https://tryton-rietveld.appspot.com/32881002/diff/430001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/430001/src/sao.js#newcode604 src/sao.js:604: // [type, name, id of button or function, shortcut] ...
5 months, 2 weeks ago (2017-02-07 23:25:49 UTC) #70
ced
https://tryton-rietveld.appspot.com/32881002/diff/430001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/430001/src/sao.js#newcode637 src/sao.js:637: jQuery(document).ready(function() { On 2017/02/07 23:25:49, ced wrote: > I ...
5 months, 2 weeks ago (2017-02-08 08:51:44 UTC) #71
perilla
Fix comments
5 months, 2 weeks ago (2017-02-12 04:03:31 UTC) #72
perilla
https://tryton-rietveld.appspot.com/32881002/diff/430001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/430001/src/sao.js#newcode604 src/sao.js:604: // [type, name, id of button or function, shortcut] ...
5 months, 2 weeks ago (2017-02-12 04:10:06 UTC) #73
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
5 months, 2 weeks ago (2017-02-12 04:14:52 UTC) #74
perilla
Update to tip
5 months, 1 week ago (2017-02-14 13:05:51 UTC) #75
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
5 months, 1 week ago (2017-02-14 13:39:36 UTC) #76
ced
https://tryton-rietveld.appspot.com/32881002/diff/430001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/430001/src/sao.js#newcode650 src/sao.js:650: e.preventDefault(); On 2017/02/12 04:10:06, perilla wrote: > On 2017/02/07 ...
5 months, 1 week ago (2017-02-14 22:34:03 UTC) #77
perilla
https://tryton-rietveld.appspot.com/32881002/diff/430001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/430001/src/sao.js#newcode650 src/sao.js:650: e.preventDefault(); On 2017/02/14 22:34:03, ced wrote: > On 2017/02/12 ...
5 months ago (2017-02-20 13:26:42 UTC) #78
perilla
Fix shortcut documentation, update to tip
5 months ago (2017-02-20 13:29:20 UTC) #79
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
5 months ago (2017-02-20 13:37:33 UTC) #80
ced
https://tryton-rietveld.appspot.com/32881002/diff/430001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/430001/src/sao.js#newcode650 src/sao.js:650: e.preventDefault(); On 2017/02/20 13:26:42, perilla wrote: > On 2017/02/14 ...
5 months ago (2017-02-20 14:54:06 UTC) #81
perilla
https://tryton-rietveld.appspot.com/32881002/diff/430001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/430001/src/sao.js#newcode650 src/sao.js:650: e.preventDefault(); On 2017/02/20 14:54:06, ced wrote: > On 2017/02/20 ...
5 months ago (2017-02-20 16:35:42 UTC) #82
ced
https://tryton-rietveld.appspot.com/32881002/diff/430001/src/sao.js File src/sao.js (right): https://tryton-rietveld.appspot.com/32881002/diff/430001/src/sao.js#newcode650 src/sao.js:650: e.preventDefault(); On 2017/02/20 16:35:42, perilla wrote: > On 2017/02/20 ...
4 months, 3 weeks ago (2017-03-01 19:06:11 UTC) #83
perilla
Adding return false on all cases
4 months, 2 weeks ago (2017-03-07 20:34:05 UTC) #84
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
4 months, 2 weeks ago (2017-03-07 20:35:54 UTC) #85
perilla
update to tip
3 months, 4 weeks ago (2017-03-28 15:38:01 UTC) #86
reviewbot
flake8 OK URL: https://codereview.tryton.org/32881002
3 months, 4 weeks ago (2017-03-28 16:07:57 UTC) #87
perilla
update to tip
2 months, 1 week ago (2017-05-19 20:13:38 UTC) #88
perilla
update to tip
2 months, 1 week ago (2017-05-19 20:13:57 UTC) #89
perilla
update to tip
2 months, 1 week ago (2017-05-19 20:23:13 UTC) #90
reviewbot
2 months, 1 week ago (2017-05-19 20:40:35 UTC) #91
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cd18842