From 3d5386e51470ab08216ee97b2b8686de7401284b Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 18 Jan 2016 16:39:19 -0800 Subject: [PATCH 1/6] Honor `created()` function for an item in a submenu. Fixes #10483. --- src/context-menu-manager.coffee | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/context-menu-manager.coffee b/src/context-menu-manager.coffee index 869076beb..88cdc67b2 100644 --- a/src/context-menu-manager.coffee +++ b/src/context-menu-manager.coffee @@ -136,12 +136,9 @@ class ContextMenuManager for itemSet in matchingItemSets for item in itemSet.items - continue if item.devMode and not @devMode - item = Object.create(item) - if typeof item.shouldDisplay is 'function' - continue unless item.shouldDisplay(event) - item.created?(event) - MenuHelpers.merge(currentTargetItems, item, itemSet.specificity) + itemForEvent = @cloneItemForEvent(item, event) + if itemForEvent + MenuHelpers.merge(currentTargetItems, itemForEvent, itemSet.specificity) for item in currentTargetItems MenuHelpers.merge(template, item, false) @@ -150,6 +147,19 @@ class ContextMenuManager template + # Returns an object compatible with `::add()` or `null`. + cloneItemForEvent: (item, event) -> + return null if item.devMode and not @devMode + item = Object.create(item) + if typeof item.shouldDisplay is 'function' + return null unless item.shouldDisplay(event) + item.created?(event) + if Array.isArray(item.submenu) + item.submenu = item.submenu + .map((item) => @cloneItemForEvent(item, event)) + .filter((item) -> item isnt null) + return item + convertLegacyItemsBySelector: (legacyItemsBySelector, devMode) -> itemsBySelector = {} From 0a4817693f0feb79b029516b9713297ee4372b46 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 18 Jan 2016 16:50:16 -0800 Subject: [PATCH 2/6] Add unit test. Cannot seem to run locally because of nodegit but will see what happens in CI. --- spec/context-menu-manager-spec.coffee | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/spec/context-menu-manager-spec.coffee b/spec/context-menu-manager-spec.coffee index ddced8452..6eb0c4390 100644 --- a/spec/context-menu-manager-spec.coffee +++ b/spec/context-menu-manager-spec.coffee @@ -156,3 +156,30 @@ describe "ContextMenuManager", -> catch error addError = error expect(addError.message).toContain('<>') + + it "recursively applies templateForEvent on submenu items", -> + created = (event) -> @label = 'D' + item = { + label: 'A', + command: 'B', + submenu: [ + { + label: 'C', + created, + } + ] + } + contextMenu.add('.grandchild': [item]) + + dispatchedEvent = {target: grandchild} + expect(contextMenu.templateForEvent(dispatchedEvent)).toEqual( + [ + label: 'A', + command: 'B', + submenu: [ + { + label: 'D', + created, + } + ] + ]) From 0e4a8303ae566b4be514c9045f39c216439a4779 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 18 Jan 2016 18:32:37 -0800 Subject: [PATCH 3/6] Apparently the created function is stripped as part of the clone? --- spec/context-menu-manager-spec.coffee | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/context-menu-manager-spec.coffee b/spec/context-menu-manager-spec.coffee index 6eb0c4390..c2fabc8fb 100644 --- a/spec/context-menu-manager-spec.coffee +++ b/spec/context-menu-manager-spec.coffee @@ -179,7 +179,6 @@ describe "ContextMenuManager", -> submenu: [ { label: 'D', - created, } ] ]) From 9423ec8b3a84daecfc2e9b3db7737e3d8fb6cb19 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 19 Jan 2016 12:37:38 -0800 Subject: [PATCH 4/6] Renamed local variables as recommended by @ssorallen. --- src/context-menu-manager.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/context-menu-manager.coffee b/src/context-menu-manager.coffee index 88cdc67b2..1d80dec09 100644 --- a/src/context-menu-manager.coffee +++ b/src/context-menu-manager.coffee @@ -156,8 +156,8 @@ class ContextMenuManager item.created?(event) if Array.isArray(item.submenu) item.submenu = item.submenu - .map((item) => @cloneItemForEvent(item, event)) - .filter((item) -> item isnt null) + .map((submenuItem) => @cloneItemForEvent(submenuItem, event)) + .filter((submenuItem) -> submenuItem isnt null) return item convertLegacyItemsBySelector: (legacyItemsBySelector, devMode) -> From ee4408976e1cb08a1e34dd0bd6a689ec4df8e688 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 21 Jan 2016 10:18:04 -0800 Subject: [PATCH 5/6] Tightened up a test now that I confirmed that MenuHelpers.merge() intentionally strips the created field. --- spec/context-menu-manager-spec.coffee | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/context-menu-manager-spec.coffee b/spec/context-menu-manager-spec.coffee index c2fabc8fb..5fb97f8fd 100644 --- a/spec/context-menu-manager-spec.coffee +++ b/spec/context-menu-manager-spec.coffee @@ -158,14 +158,13 @@ describe "ContextMenuManager", -> expect(addError.message).toContain('<>') it "recursively applies templateForEvent on submenu items", -> - created = (event) -> @label = 'D' item = { label: 'A', command: 'B', submenu: [ { label: 'C', - created, + created: (event) -> @label = 'D', } ] } From 83d31686097cec4a6b53dee18db8ecba84e749e5 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 25 Jan 2016 13:19:58 -0800 Subject: [PATCH 6/6] Rename test function as suggested by @maxbrunsfeld. --- spec/context-menu-manager-spec.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/context-menu-manager-spec.coffee b/spec/context-menu-manager-spec.coffee index 5fb97f8fd..b336361f8 100644 --- a/spec/context-menu-manager-spec.coffee +++ b/spec/context-menu-manager-spec.coffee @@ -157,7 +157,7 @@ describe "ContextMenuManager", -> addError = error expect(addError.message).toContain('<>') - it "recursively applies templateForEvent on submenu items", -> + it "calls `created` hooks for submenu items", -> item = { label: 'A', command: 'B',