Handle discovery route changes #128
Conversation
angusmcleod
requested review from
jumagura and
merefield
and removed request for
jumagura
There was a problem hiding this comment.
@angusmcleod this is not working for me on latest - I just get a blank page when I navigate to a a Category Calendar.
there are no associated errors in console, but there is a deprecation warning:
deprecated.js:52 [PLUGIN discourse-events] Deprecation notice: [registerUnbound event-label] registerUnbound is deprecated. Instead, you should export a default function from 'discourse/helpers/event-label.js'. If the helper is also used in raw-hbs, you can register it using 'registerRawHelper'. [deprecation id: discourse.register-unbound]
|
@angusmcleod OK using the associated Discourse branch, Additionally there appears to be a bigger problem, I'm getting this whenever navigating to a Topic:
|
| renderTemplate() { | ||
| this.render("discovery/calendar"); |
There was a problem hiding this comment.
This isn't strictly related to the discovery refactoring, but I think it's worth noting that this.render() is deprecated in Ember and is removed in Ember 4.x+. (indeed, this is the reason core is refactoring the discovery routes to remove named outlets and this.render() calls)
Right now the deprecation is silenced in Discourse core, so it won't appear in the console.
I think in this case, you're not actually using a 'named outlet', so you should be able to resolve the issue fairly easily by removing this renderTemplate() function and instead setting
templateName: "discovery/calendar"
on the controller. That is still supported into Ember 4.x and beyond.
(see https://deprecations.emberjs.com/v3.x#toc_route-render-template for more info)
There was a problem hiding this comment.
Right now the deprecation is silenced in Discourse core, so it won't appear in the console.
(I just updated the discovery-named-outlets branch of Discourse so that the relevant deprecation is no longer silenced)
There was a problem hiding this comment.
@davidtaylorhq Experimenting with this this morning, I couldn't get templateName to work using the plugin api (i.e. modifyClass). An entirely separate route file using buildCategoryRoute does work however: https://github.com/paviliondev/discourse-events/blob/2d61a1a302101d40b6126f095baf15a46cd6a6ab/assets/javascripts/discourse/routes/discovery/calendar-category.js
cc @merefield
There was a problem hiding this comment.
I couldn't get templateName to work using the plugin api (i.e. modifyClass).
Ah yes, that'll be because 'native class fields' will always take precedence over properties set via Ember's legacy object-literal authoring format (which modifyClass currently uses). Moving it to a dedicated class definition seems like a good solution, and a good general improvement anyway 👌
See (internal): https://coop.pavilion.tech/t/plugins-affected-by-discovery-route-changes/3144