Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow extensions to use route resolvers #2275

Merged
merged 14 commits into from
Oct 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions js/src/common/compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -138,4 +139,5 @@ export default {
'helpers/username': username,
'helpers/userOnline': userOnline,
'helpers/listItems': listItems,
'resolvers/DefaultResolver': DefaultResolver,
};
34 changes: 34 additions & 0 deletions js/src/common/resolvers/DefaultResolver.ts
Original file line number Diff line number Diff line change
@@ -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() }];
}
}
19 changes: 12 additions & 7 deletions js/src/common/utils/mapRoutes.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions js/src/forum/compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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,
});
47 changes: 47 additions & 0 deletions js/src/forum/resolver/DiscussionPageResolver.ts
Original file line number Diff line number Diff line change
@@ -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'))) {
askvortsov1 marked this conversation as resolved.
Show resolved Hide resolved
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);
}
}
5 changes: 3 additions & 2 deletions js/src/forum/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 },
Expand Down