From e711fca20e3fbf6fbb1d4105af9da9444fdf32fb Mon Sep 17 00:00:00 2001 From: tundebabzy Date: Tue, 23 Jan 2018 10:57:49 +0100 Subject: [PATCH 1/4] Prevent Duplicate Buttons In `add_inner_button` (#4858) * refactor add_inner_button: 1. create new function - is_in_group_button_dropdown 2. rewrite the code using checks with is_in_group_button_dropdown * adds JSDoc documentation * use `is_in_group_button_dropdown` in `add_dropdown_item` for DRY * clean up --- frappe/public/js/frappe/ui/page.js | 41 ++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/frappe/public/js/frappe/ui/page.js b/frappe/public/js/frappe/ui/page.js index 728e9a6034..076b443e32 100644 --- a/frappe/public/js/frappe/ui/page.js +++ b/frappe/public/js/frappe/ui/page.js @@ -256,17 +256,14 @@ frappe.ui.Page = Class.extend({ * @param {object} parent - DOM object representing the parent of the drop down item lists */ add_dropdown_item: function(label, click, standard, parent) { - const is_already_added = () => { - let found_lists = $(parent).find('li > a.grey-link:contains(' + label + ')'); - return found_lists.length > 0; - } + let item_selector = 'li > a.grey-link'; parent.parent().removeClass("hide"); var $li = $('
  • '+ label +'
  • '), $link = $li.find("a").on("click", click); - if (is_already_added()) return; + if (this.is_in_group_button_dropdown(parent, `${item_selector}:contains('${label}')`, label)) return; if(standard===true) { $li.appendTo(parent); @@ -281,6 +278,21 @@ frappe.ui.Page = Class.extend({ return $link; }, + /* + * Check if there already exists a button with a specified label in a specified button group + * @param {object} parent - This should be the `ul` of the button group. + * @param {string} selector - CSS Selector of the button to be searched for. By default, it is `li`. + * @param {string} label - Label of the button + */ + is_in_group_button_dropdown: function(parent, selector, label){ + if (!selector) selector = 'li'; + + if (!label || !parent) return false; + + const result = $(parent).find(`${selector}:contains('${label}')`); + return result.length > 0; + }, + clear_btn_group: function(parent) { parent.empty(); parent.parent().addClass("hide"); @@ -323,6 +335,15 @@ frappe.ui.Page = Class.extend({ } }, + /* + * Add button to button group. If there exists another button with the same label, + * `add_inner_button` will not add the new button to the button group even if the callback + * function is different. + * + * @param {string} label - Label of the button to be added to the group + * @param {object} action - function to be called when button is clicked + * @param {string} group - Label of the group button + */ add_inner_button: function(label, action, group) { var me = this; let _action = function() { @@ -333,9 +354,13 @@ frappe.ui.Page = Class.extend({ if(group) { var $group = this.get_or_add_inner_group_button(group); $(this.inner_toolbar).removeClass("hide"); - return $('
  • '+label+'
  • ') - .on('click', _action) - .appendTo($group.find(".dropdown-menu")); + + if (!this.is_in_group_button_dropdown($group.find(".dropdown-menu"), 'li', label)) { + return $('
  • '+label+'
  • ') + .on('click', _action) + .appendTo($group.find(".dropdown-menu")); + } + } else { return $('