FIX: uses simpler pattern for custom href on extra nav items (#8015)

FIX: uses simpler pattern for custom href on extra nav items (#8015)

THe main advantage of this solution is that it will be called on each rerendered whereas the other is not once href has been set.

Example API:

      api.addNavigationBarItem({
        name: "foo",
        displayName: "Foo",
        customHref: function(category, args) {
          const router = api.container.lookup("service:router");
          const queryParams = { bar: "1" };
          return router.urlFor(router.currentRouteName, category, {
            queryParams
          });
        }
      });
diff --git a/app/assets/javascripts/discourse/models/nav-item.js.es6 b/app/assets/javascripts/discourse/models/nav-item.js.es6
index b4c725d..799039d 100644
--- a/app/assets/javascripts/discourse/models/nav-item.js.es6
+++ b/app/assets/javascripts/discourse/models/nav-item.js.es6
@@ -101,18 +101,8 @@ const NavItem = Discourse.Model.extend({
 });
 
 const ExtraNavItem = NavItem.extend({
-  href: Ember.computed({
-    set(key, value) {
-      let customHref;
-      NavItem.customNavItemHrefs.forEach(function(cb) {
-        customHref = cb.call(this, this);
-        if (customHref) {
-          return false;
-        }
-      }, this);
-      return customHref || value;
-    }
-  }),
+  @computed("href")
+  href: href => href,
 
   customFilter: null
 });
@@ -189,6 +179,11 @@ NavItem.reopenClass({
       return item.customFilter.call(this, category, args);
     });
 
+    extraItems.forEach(item => {
+      if (!item.customHref) return;
+      item.set("href", item.customHref.call(this, category, args));
+    });
+
     return items.concat(extraItems);
   }
 });

GitHub sha: 897cdfb5

1 Like

I think it’s asking a lot from people who use the api to look up the router every time isn’t it? Perhaps the customHref can be called with the router as an object parameter?

yes mostly theres already customFilter and I tried to keep the same API. SO I’m unsure what to do here, but I could pass router for sure this would be cleaner, but then I can’t do (category, args, router), so having (category, router, args) would change the API.

Scratch this, args is an object, so I can do (category, args, router) for both customFilter and customHref. Works for you @eviltrout ?

Im still unsure, nav-item is a model. Looks weird to me, to pass a router from a model.

Yeah passing a router from a model is indeed weird. However, you could look up the router in the plugin-api object itself and curry it into the function like:


  let customHref = item.customHref;
  if (customHref) {
    let router = this.container.lookup("service:router");
    item.customHref = function(category, args) {
       return customHref(category, args, router);
    }
  }

I wrote that from memory so I doubt it’ll work the first time :slight_smile:

1 Like

Seems like a good solution, will do it :+1: