From d44dbb373d084623d60dbc667bc90e0f9f98564a Mon Sep 17 00:00:00 2001 From: Damien Guard Date: Wed, 24 Aug 2016 21:45:21 -0700 Subject: [PATCH 1/2] Remove trailing context menu separator fixing #5390 --- spec/context-menu-manager-spec.coffee | 11 +++++++++++ src/context-menu-manager.coffee | 4 ++++ 2 files changed, 15 insertions(+) diff --git a/spec/context-menu-manager-spec.coffee b/spec/context-menu-manager-spec.coffee index b336361f8..63815d1dd 100644 --- a/spec/context-menu-manager-spec.coffee +++ b/spec/context-menu-manager-spec.coffee @@ -149,6 +149,17 @@ describe "ContextMenuManager", -> shouldDisplay = false expect(contextMenu.templateForEvent(dispatchedEvent)).toEqual [] + it "prunes a trailing separator", -> + contextMenu.add + '.grandchild': [ + {label: 'A', command: 'a'}, + {type: 'separator'}, + {label: 'B', command: 'b'}, + {type: 'separator'} + ] + + expect(contextMenu.templateForEvent({target: grandchild}).length).toBe(3) + it "throws an error when the selector is invalid", -> addError = null try diff --git a/src/context-menu-manager.coffee b/src/context-menu-manager.coffee index 936a9c6b6..1fe780db3 100644 --- a/src/context-menu-manager.coffee +++ b/src/context-menu-manager.coffee @@ -145,6 +145,10 @@ class ContextMenuManager currentTarget = currentTarget.parentElement + # Remove trailing separator + if template.length > 0 and template[template.length - 1].type is 'separator' + template.splice(template.length-1, 1) + template # Returns an object compatible with `::add()` or `null`. From 0f88949832c46ed6ecec40df9a929f198b7650c7 Mon Sep 17 00:00:00 2001 From: Damien Guard Date: Thu, 25 Aug 2016 12:49:29 -0700 Subject: [PATCH 2/2] Remove all redundant separators --- spec/context-menu-manager-spec.coffee | 40 ++++++++++++++++++++++++++- src/context-menu-manager.coffee | 17 ++++++++++-- 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/spec/context-menu-manager-spec.coffee b/spec/context-menu-manager-spec.coffee index 63815d1dd..94b45d4fa 100644 --- a/spec/context-menu-manager-spec.coffee +++ b/spec/context-menu-manager-spec.coffee @@ -149,7 +149,7 @@ describe "ContextMenuManager", -> shouldDisplay = false expect(contextMenu.templateForEvent(dispatchedEvent)).toEqual [] - it "prunes a trailing separator", -> + fit "prunes a trailing separator", -> contextMenu.add '.grandchild': [ {label: 'A', command: 'a'}, @@ -160,6 +160,44 @@ describe "ContextMenuManager", -> expect(contextMenu.templateForEvent({target: grandchild}).length).toBe(3) + fit "prunes a leading separator", -> + contextMenu.add + '.grandchild': [ + {type: 'separator'}, + {label: 'A', command: 'a'}, + {type: 'separator'}, + {label: 'B', command: 'b'} + ] + + expect(contextMenu.templateForEvent({target: grandchild}).length).toBe(3) + + fit "prunes duplicate separators", -> + contextMenu.add + '.grandchild': [ + {label: 'A', command: 'a'}, + {type: 'separator'}, + {type: 'separator'}, + {label: 'B', command: 'b'} + ] + + expect(contextMenu.templateForEvent({target: grandchild}).length).toBe(3) + + fit "prunes all redundant separators", -> + contextMenu.add + '.grandchild': [ + {type: 'separator'}, + {type: 'separator'}, + {label: 'A', command: 'a'}, + {type: 'separator'}, + {type: 'separator'}, + {label: 'B', command: 'b'} + {label: 'C', command: 'c'} + {type: 'separator'}, + {type: 'separator'}, + ] + + expect(contextMenu.templateForEvent({target: grandchild}).length).toBe(4) + it "throws an error when the selector is invalid", -> addError = null try diff --git a/src/context-menu-manager.coffee b/src/context-menu-manager.coffee index 1fe780db3..64360dd2e 100644 --- a/src/context-menu-manager.coffee +++ b/src/context-menu-manager.coffee @@ -145,12 +145,23 @@ class ContextMenuManager currentTarget = currentTarget.parentElement - # Remove trailing separator - if template.length > 0 and template[template.length - 1].type is 'separator' - template.splice(template.length-1, 1) + @pruneRedundantSeparators(template) template + pruneRedundantSeparators: (menu) -> + keepNextItemIfSeparator = false + index = 0 + while index < menu.length + if menu[index].type is 'separator' + if not keepNextItemIfSeparator or index is menu.length - 1 + menu.splice(index, 1) + else + index++ + else + keepNextItemIfSeparator = true + index++ + # Returns an object compatible with `::add()` or `null`. cloneItemForEvent: (item, event) -> return null if item.devMode and not @devMode