From 7e366809da686ab3fc6beb0b50a25f7187743823 Mon Sep 17 00:00:00 2001
From: Faris Ansari <netchamp.faris@gmail.com>
Date: Mon, 1 Jul 2019 19:05:12 +0530
Subject: [PATCH] fix: Object based API for add_shortcuts

shortcut:
key combination

action:
Trigger this function on shortcut

page:
If page is passed, the shortcut will be bound to that page

target:
Should be a jquery element, it will be clicked as the shortcut trigger

description:
Will show up in the Keyboard Shortcuts dialog

ignore_inputs:
If true, will trigger the keyboard shortcut even if inputs are focused
---
 frappe/public/js/frappe/list/list_view.js | 98 ++++++++++++++---------
 frappe/public/js/frappe/ui/keyboard.js    | 94 ++++++++++++++--------
 frappe/public/js/frappe/ui/page.js        |  7 +-
 3 files changed, 130 insertions(+), 69 deletions(-)

diff --git a/frappe/public/js/frappe/list/list_view.js b/frappe/public/js/frappe/list/list_view.js
index 12f358b0fc..1e414a527f 100644
--- a/frappe/public/js/frappe/list/list_view.js
+++ b/frappe/public/js/frappe/list/list_view.js
@@ -791,45 +791,71 @@ frappe.views.ListView = class ListView extends frappe.views.BaseList {
 			}
 		};
 
-		frappe.ui.keys.add_shortcut('down', () => {
-			return handle_navigation('down');
-		}, __('Navigate list down'), this.page);
+		frappe.ui.keys.add_shortcut({
+			shortcut: 'down',
+			action: () => handle_navigation('down'),
+			description: __('Navigate list down'),
+			page: this.page
+		});
 
-		frappe.ui.keys.add_shortcut('up', () => {
-			return handle_navigation('up');
-		}, __('Navigate list up'), this.page);
+		frappe.ui.keys.add_shortcut({
+			shortcut: 'up',
+			action: () => handle_navigation('up'),
+			description: __('Navigate list up'),
+			page: this.page
+		});
 
-		frappe.ui.keys.add_shortcut('shift+down', () => {
-			if (!is_current_page() || is_input_focused()) return false;
-			let $list_row = get_list_row_if_focused();
-			check_row($list_row);
-			focus_next();
-		}, __('Select multiple list items'), this.page);
-
-		frappe.ui.keys.add_shortcut('shift+up', () => {
-			if (!is_current_page() || is_input_focused()) return false;
-			let $list_row = get_list_row_if_focused();
-			check_row($list_row);
-			focus_prev();
-		}, __('Select multiple list items'), this.page);
-
-		frappe.ui.keys.add_shortcut('enter', () => {
-			let $list_row = get_list_row_if_focused();
-			if ($list_row) {
-				$list_row.find('a[data-name]')[0].click();
-				return true;
-			}
-			return false;
-		}, __('Open list item'), this.page);
-
-		frappe.ui.keys.add_shortcut('space', () => {
-			let $list_row = get_list_row_if_focused();
-			if ($list_row) {
+		frappe.ui.keys.add_shortcut({
+			shortcut: 'shift+down',
+			action: () => {
+				if (!is_current_page() || is_input_focused()) return false;
+				let $list_row = get_list_row_if_focused();
 				check_row($list_row);
-				return true;
-			}
-			return false;
-		}, __('Select list item'), this.page);
+				focus_next();
+			},
+			description: __('Select multiple list items'),
+			page: this.page
+		});
+
+		frappe.ui.keys.add_shortcut({
+			shortcut: 'shift+up',
+			action: () => {
+				if (!is_current_page() || is_input_focused()) return false;
+				let $list_row = get_list_row_if_focused();
+				check_row($list_row);
+				focus_prev();
+			},
+			description: __('Select multiple list items'),
+			page: this.page
+		});
+
+		frappe.ui.keys.add_shortcut({
+			shortcut: 'enter',
+			action: () => {
+				let $list_row = get_list_row_if_focused();
+				if ($list_row) {
+					$list_row.find('a[data-name]')[0].click();
+					return true;
+				}
+				return false;
+			},
+			description: __('Open list item'),
+			page: this.page
+		});
+
+		frappe.ui.keys.add_shortcut({
+			shortcut: 'space',
+			action: () => {
+				let $list_row = get_list_row_if_focused();
+				if ($list_row) {
+					check_row($list_row);
+					return true;
+				}
+				return false;
+			},
+			description: __('Select list item'),
+			page: this.page
+		});
 	}
 
 	setup_filterable() {
diff --git a/frappe/public/js/frappe/ui/keyboard.js b/frappe/public/js/frappe/ui/keyboard.js
index 103fd693e2..158278d00b 100644
--- a/frappe/public/js/frappe/ui/keyboard.js
+++ b/frappe/public/js/frappe/ui/keyboard.js
@@ -21,9 +21,9 @@ frappe.ui.keys.setup = function() {
 
 let standard_shortcuts = [];
 frappe.ui.keys.standard_shortcuts = standard_shortcuts;
-frappe.ui.keys.add_shortcut = (shortcut, action, description, page) => {
-	if (action instanceof jQuery) {
-		let $target = action;
+frappe.ui.keys.add_shortcut = ({shortcut, action, description, page, target, ignore_inputs = false} = {}) => {
+	if (target instanceof jQuery) {
+		let $target = target;
 		action = () => {
 			$target[0].click();
 		}
@@ -31,8 +31,9 @@ frappe.ui.keys.add_shortcut = (shortcut, action, description, page) => {
 	frappe.ui.keys.on(shortcut, (e) => {
 		let $focused_element = $(document.activeElement);
 		let is_input_focused = $focused_element.is('input, select, textarea, [contenteditable=true]');
+		if (is_input_focused && !ignore_inputs) return;
 
-		if (!is_input_focused && (!page || page.wrapper.is(':visible'))) {
+		if (!page || page.wrapper.is(':visible')) {
 			let prevent_default = action(e);
 			// prevent default if true is explicitly returned
 			// or nothing returned (undefined)
@@ -129,36 +130,61 @@ frappe.ui.keys.on = function(key, handler) {
 	frappe.ui.keys.handlers[key].push(handler);
 }
 
-frappe.ui.keys.add_shortcut('ctrl+s', function(e) {
-	frappe.app.trigger_primary_action();
-	e.preventDefault();
-	return false;
-}, __('Trigger Primary Action'));
+frappe.ui.keys.add_shortcut({
+	shortcut: 'ctrl+s',
+	action: function(e) {
+		frappe.app.trigger_primary_action();
+		e.preventDefault();
+		return false;
+	},
+	description: __('Trigger Primary Action'),
+	ignore_inputs: true
+});
 
-frappe.ui.keys.add_shortcut('ctrl+g', function(e) {
-	$("#navbar-search").focus();
-	e.preventDefault();
-	return false;
-}, __('Open Awesomebar'));
+frappe.ui.keys.add_shortcut({
+	shortcut: 'ctrl+g',
+	action: function(e) {
+		$("#navbar-search").focus();
+		e.preventDefault();
+		return false;
+	},
+	description: __('Open Awesomebar')
+});
 
-frappe.ui.keys.add_shortcut('ctrl+h', function(e) {
-	e.preventDefault();
-	$('.navbar-home img').click();
-}, __('Home'));
+frappe.ui.keys.add_shortcut({
+	shortcut: 'ctrl+h',
+	action: function(e) {
+		e.preventDefault();
+		$('.navbar-home img').click();
+	},
+	description: __('Home')
+});
 
-frappe.ui.keys.add_shortcut('alt+s', function(e) {
-	e.preventDefault();
-	$('.dropdown-navbar-user a').eq(0).click();
-}, __('Settings'));
+frappe.ui.keys.add_shortcut({
+	shortcut: 'alt+s',
+	action: function(e) {
+		e.preventDefault();
+		$('.dropdown-navbar-user a').eq(0).click();
+	},
+	description: __('Settings')
+});
 
-frappe.ui.keys.add_shortcut('shift+/', function() {
-	frappe.ui.keys.show_keyboard_shortcut_dialog();
-}, __('Keyboard Shortcuts'));
+frappe.ui.keys.add_shortcut({
+	shortcut: 'shift+/',
+	action: function() {
+		frappe.ui.keys.show_keyboard_shortcut_dialog();
+	},
+	description: __('Keyboard Shortcuts')
+});
 
-frappe.ui.keys.add_shortcut('alt+h', function(e) {
-	e.preventDefault();
-	$('.dropdown-help a').eq(0).click();
-}, __('Help'));
+frappe.ui.keys.add_shortcut({
+	shortcut: 'alt+h',
+	action: function(e) {
+		e.preventDefault();
+		$('.dropdown-help a').eq(0).click();
+	},
+	description: __('Help')
+});
 
 frappe.ui.keys.on('escape', function(e) {
 	close_grid_and_dialog();
@@ -184,9 +210,13 @@ frappe.ui.keys.on('ctrl+up', function(e) {
 	grid_row && grid_row.toggle_view(false, function() { grid_row.open_prev() });
 });
 
-frappe.ui.keys.add_shortcut('shift+ctrl+r', function() {
-	frappe.ui.toolbar.clear_cache();
-}, __('Clear Cache and Reload'));
+frappe.ui.keys.add_shortcut({
+	shortcut: 'shift+ctrl+r',
+	action: function() {
+		frappe.ui.toolbar.clear_cache();
+	},
+	description: __('Clear Cache and Reload')
+});
 
 frappe.ui.keys.key_map = {
 	8: 'backspace',
diff --git a/frappe/public/js/frappe/ui/page.js b/frappe/public/js/frappe/ui/page.js
index cad39cbabd..aee63c8af6 100644
--- a/frappe/public/js/frappe/ui/page.js
+++ b/frappe/public/js/frappe/ui/page.js
@@ -322,7 +322,12 @@ frappe.ui.Page = Class.extend({
 				<span class="text-muted pull-right">${shortcut_label}</span>
 			</a><li>`);
 			shortcut = shortcut.toLowerCase();
-			frappe.ui.keys.add_shortcut(shortcut, $li.find('a'), label, this);
+			frappe.ui.keys.add_shortcut({
+				shortcut,
+				target: $li.find('a'),
+				description: label,
+				page: this
+			});
 		} else {
 			$li = $(`<li><a class="grey-link dropdown-item" href="#" onClick="return false;">
 				<span class="menu-item-label">${label}</span></a><li>`);