core#1398: Option to open navigation item in new window (if present)

[monishdeb]

Overview

Currently, the target attribute of submenus are ignored whereas the menu item adds this property in the anchor links here

Before

The target attribute was ignored even if added using navigationMenu hook

After

The target atrribute is not ignored for (existing/new) sub-menu items

Comments

ping @colemanw @lcdservices @eileenmcnaughton

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[mlutfy]

jenkins, test this please

[colemanw]

Looking at the attr() function, maybe instead of patching this line you could do this:

diff --git a/js/crm.menubar.js b/js/crm.menubar.js
index 152a6e376e..9fa49f63d6 100644
--- a/js/crm.menubar.js
+++ b/js/crm.menubar.js
@@ -471,7 +471,7 @@
   }
 
   function attr(el, item) {
-    var ret = [], attr = _.cloneDeep(item.attr || {}), a = ['rel', 'accesskey'];
+    var ret = [], attr = _.cloneDeep(item.attr || {}), a = ['rel', 'accesskey', 'target'];
     if (el === 'a') {
       attr = _.pick(attr, a);
       attr.href = item.url || "#";

[seamuslee001]

@monishdeb any thoughts?

[monishdeb]

Thanks @colemanw for suggesting the tweak, about how we can use existing attr parameter to add target attribute to menus. I have submitted a doc PR civicrm/civicrm-dev-docs#734 to extend an example about how we can use the attr parameter to add anchor tag attributes (here target). Please have a look

[colemanw]

Yes @monishdeb that looks right. Can you squash this PR please?

[monishdeb]

done :)

[colemanw]

FYI @monishdeb I tried to find where the function is used that you referenced in this PR description... and it's not. So I've filed #15956

[colemanw]

Unrelated fail