From 3f0b3c789ff2854b55b080c1ef42ffb19c7d8efd Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov <38059171+askvortsov1@users.noreply.github.com> Date: Thu, 15 Oct 2020 18:01:17 -0400 Subject: [PATCH] Allow extensions to use route resolvers (#2275) - mapRoutes: don't wrap components in resolvers if they are already resolvers - Extract defaultResolver into its own class - Allow either route resolver instances, or components with an optional resolverClass which should accept the component and route name in its constructor. - Introduce a resolver for DiscussionPage, so that routing from one post to another on the same discussion triggers a scroll instead of rerendering --- framework/core/js/src/common/compat.js | 2 + .../src/common/resolvers/DefaultResolver.ts | 34 ++++++++++++++ .../core/js/src/common/utils/mapRoutes.js | 19 +++++--- framework/core/js/src/forum/compat.js | 2 + .../forum/resolver/DiscussionPageResolver.ts | 47 +++++++++++++++++++ framework/core/js/src/forum/routes.js | 5 +- 6 files changed, 100 insertions(+), 9 deletions(-) create mode 100644 framework/core/js/src/common/resolvers/DefaultResolver.ts create mode 100644 framework/core/js/src/forum/resolver/DiscussionPageResolver.ts diff --git a/framework/core/js/src/common/compat.js b/framework/core/js/src/common/compat.js index 268f33982..4716c1219 100644 --- a/framework/core/js/src/common/compat.js +++ b/framework/core/js/src/common/compat.js @@ -67,6 +67,7 @@ import username from './helpers/username'; import userOnline from './helpers/userOnline'; import listItems from './helpers/listItems'; import Fragment from './Fragment'; +import DefaultResolver from './resolvers/DefaultResolver'; export default { extend: extend, @@ -138,4 +139,5 @@ export default { 'helpers/username': username, 'helpers/userOnline': userOnline, 'helpers/listItems': listItems, + 'resolvers/DefaultResolver': DefaultResolver, }; diff --git a/framework/core/js/src/common/resolvers/DefaultResolver.ts b/framework/core/js/src/common/resolvers/DefaultResolver.ts new file mode 100644 index 000000000..024bacd2f --- /dev/null +++ b/framework/core/js/src/common/resolvers/DefaultResolver.ts @@ -0,0 +1,34 @@ +import Mithril from 'mithril'; + +/** + * Generates a route resolver for a given component. + * In addition to regular route resolver functionality: + * - It provide the current route name as an attr + * - It sets a key on the component so a rerender will be triggered on route change. + */ +export default class DefaultResolver { + component: Mithril.Component; + routeName: string; + + constructor(component, routeName) { + this.component = component; + this.routeName = routeName; + } + + /** + * When a route change results in a changed key, a full page + * rerender occurs. This method can be overriden in subclasses + * to prevent rerenders on some route changes. + */ + makeKey() { + return this.routeName + JSON.stringify(m.route.param()); + } + + onmatch(args, requestedPath, route) { + return this.component; + } + + render(vnode) { + return [{ ...vnode, routeName: this.routeName, key: this.makeKey() }]; + } +} diff --git a/framework/core/js/src/common/utils/mapRoutes.js b/framework/core/js/src/common/utils/mapRoutes.js index ab4f0500d..703beb137 100644 --- a/framework/core/js/src/common/utils/mapRoutes.js +++ b/framework/core/js/src/common/utils/mapRoutes.js @@ -1,6 +1,9 @@ +import DefaultResolver from '../resolvers/DefaultResolver'; + /** * The `mapRoutes` utility converts a map of named application routes into a - * format that can be understood by Mithril. + * format that can be understood by Mithril, and wraps them in route resolvers + * to provide each route with the current route name. * * @see https://mithril.js.org/route.html#signature * @param {Object} routes @@ -13,12 +16,14 @@ export default function mapRoutes(routes, basePath = '') { for (const routeName in routes) { const route = routes[routeName]; - map[basePath + route.path] = { - render() { - const key = routeName + JSON.stringify(m.route.param()); - return [m(route.component, { routeName, key })]; - }, - }; + if ('resolver' in route) { + map[basePath + route.path] = route.resolver; + } else if ('component' in route) { + const resolverClass = 'resolverClass' in route ? route.resolverClass : DefaultResolver; + map[basePath + route.path] = new resolverClass(route.component, routeName); + } else { + throw new Error(`Either a resolver or a component must be provided for the route [${routeName}]`); + } } return map; diff --git a/framework/core/js/src/forum/compat.js b/framework/core/js/src/forum/compat.js index f04d89375..273e4ce3a 100644 --- a/framework/core/js/src/forum/compat.js +++ b/framework/core/js/src/forum/compat.js @@ -71,6 +71,7 @@ import Search from './components/Search'; import DiscussionListItem from './components/DiscussionListItem'; import LoadingPost from './components/LoadingPost'; import PostsUserPage from './components/PostsUserPage'; +import DiscussionPageResolver from './resolver/DiscussionPageResolver'; import routes from './routes'; import ForumApplication from './ForumApplication'; @@ -146,6 +147,7 @@ export default Object.assign(compat, { 'components/DiscussionListItem': DiscussionListItem, 'components/LoadingPost': LoadingPost, 'components/PostsUserPage': PostsUserPage, + 'resolvers/DiscussionPageResolver': DiscussionPageResolver, routes: routes, ForumApplication: ForumApplication, }); diff --git a/framework/core/js/src/forum/resolver/DiscussionPageResolver.ts b/framework/core/js/src/forum/resolver/DiscussionPageResolver.ts new file mode 100644 index 000000000..fc7d1d1c5 --- /dev/null +++ b/framework/core/js/src/forum/resolver/DiscussionPageResolver.ts @@ -0,0 +1,47 @@ +import DefaultResolver from '../../common/resolvers/DefaultResolver'; + +/** + * This isn't exported as it is a temporary measure. + * A more robust system will be implemented alongside UTF-8 support in beta 15. + */ +function getDiscussionIdFromSlug(slug: string | undefined) { + if (!slug) return; + return slug.split('-')[0]; +} + +/** + * A custom route resolver for DiscussionPage that generates the same key to all posts + * on the same discussion. It triggers a scroll when going from one post to another + * in the same discussion. + */ +export default class DiscussionPageResolver extends DefaultResolver { + static scrollToPostNumber: number | null = null; + + makeKey() { + const params = { ...m.route.param() }; + if ('near' in params) { + delete params.near; + } + params.id = getDiscussionIdFromSlug(params.id); + return this.routeName.replace('.near', '') + JSON.stringify(params); + } + + onmatch(args, requestedPath, route) { + if (route.includes('/d/:id') && getDiscussionIdFromSlug(args.id) === getDiscussionIdFromSlug(m.route.param('id'))) { + DiscussionPageResolver.scrollToPostNumber = parseInt(args.near); + } + + return super.onmatch(args, requestedPath, route); + } + + render(vnode) { + if (DiscussionPageResolver.scrollToPostNumber !== null) { + const number = DiscussionPageResolver.scrollToPostNumber; + // Scroll after a timeout to avoid clashes with the render. + setTimeout(() => app.current.get('stream').goToNumber(number)); + DiscussionPageResolver.scrollToPostNumber = null; + } + + return super.render(vnode); + } +} diff --git a/framework/core/js/src/forum/routes.js b/framework/core/js/src/forum/routes.js index 7f5675b2b..330fbac74 100644 --- a/framework/core/js/src/forum/routes.js +++ b/framework/core/js/src/forum/routes.js @@ -4,6 +4,7 @@ import PostsUserPage from './components/PostsUserPage'; import DiscussionsUserPage from './components/DiscussionsUserPage'; import SettingsPage from './components/SettingsPage'; import NotificationsPage from './components/NotificationsPage'; +import DiscussionPageResolver from './resolver/DiscussionPageResolver'; /** * The `routes` initializer defines the forum app's routes. @@ -14,8 +15,8 @@ export default function (app) { app.routes = { index: { path: '/all', component: IndexPage }, - discussion: { path: '/d/:id', component: DiscussionPage }, - 'discussion.near': { path: '/d/:id/:near', component: DiscussionPage }, + discussion: { path: '/d/:id', component: DiscussionPage, resolverClass: DiscussionPageResolver }, + 'discussion.near': { path: '/d/:id/:near', component: DiscussionPage, resolverClass: DiscussionPageResolver }, user: { path: '/u/:username', component: PostsUserPage }, 'user.posts': { path: '/u/:username', component: PostsUserPage },