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

Fix lifecyle method workarounds #2378

Merged
merged 16 commits into from
Oct 15, 2020
Merged
20 changes: 5 additions & 15 deletions js/src/common/components/Page.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ export default class Page extends Component {
oninit(vnode) {
super.oninit(vnode);

this.onNewRoute();
app.previous = app.current;
app.current = new PageState(this.constructor, { routeName: this.attrs.routeName });

app.drawer.hide();
app.modal.close();

/**
* A class name to apply to the body while the route is active.
Expand All @@ -20,20 +24,6 @@ export default class Page extends Component {
this.bodyClass = '';
}

/**
* A collections of actions to run when the route changes.
* This is extracted here, and not hardcoded in oninit, as oninit is not called
* when a different route is handled by the same component, but we still need to
* adjust the current route name.
*/
onNewRoute() {
app.previous = app.current;
app.current = new PageState(this.constructor, { routeName: this.attrs.routeName });

app.drawer.hide();
app.modal.close();
}

oncreate(vnode) {
super.oncreate(vnode);

Expand Down
7 changes: 4 additions & 3 deletions js/src/common/utils/mapRoutes.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@
export default function mapRoutes(routes, basePath = '') {
const map = {};

for (const key in routes) {
const route = routes[key];
for (const routeName in routes) {
const route = routes[routeName];

map[basePath + route.path] = {
render() {
return m(route.component, { routeName: key });
const key = routeName + JSON.stringify(m.route.param());
return [m(route.component, { routeName, key })];
},
};
}
Expand Down
33 changes: 0 additions & 33 deletions js/src/forum/components/DiscussionPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ export default class DiscussionPage extends Page {
app.history.push('discussion');

this.bodyClass = 'App--discussion';

this.prevRoute = m.route.get();
}

onremove() {
Expand Down Expand Up @@ -96,34 +94,6 @@ export default class DiscussionPage extends Page {
);
}

onbeforeupdate(vnode) {
w-4 marked this conversation as resolved.
Show resolved Hide resolved
super.onbeforeupdate(vnode);

if (m.route.get() !== this.prevRoute) {
this.prevRoute = m.route.get();

// If we have routed to the same discussion as we were viewing previously,
// cancel the unloading of this controller and instead prompt the post
// stream to jump to the new 'near' param.
if (this.discussion) {
const idParam = m.route.param('id');

if (idParam && idParam.split('-')[0] === this.discussion.id()) {
const near = m.route.param('near') || '1';

if (near !== String(this.near)) {
this.stream.goToNumber(near);
}

this.near = near;
} else {
this.onNewRoute();
this.oninit(vnode);
}
}
}
}

/**
* Load the discussion from the API or use the preloaded one.
*/
Expand Down Expand Up @@ -248,10 +218,7 @@ export default class DiscussionPage extends Page {
// replace it into the window's history and our own history stack.
const url = app.route.discussion(discussion, (this.near = startNumber));

this.prevRoute = url;
m.route.set(url, null, { replace: true });
w-4 marked this conversation as resolved.
Show resolved Hide resolved
window.history.replaceState(null, document.title, url);

app.history.push('discussion', discussion.title());

// If the user hasn't read past here before, then we'll update their read
Expand Down
20 changes: 0 additions & 20 deletions js/src/forum/components/IndexPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,26 +42,6 @@ export default class IndexPage extends Page {
app.history.push('index', app.translator.trans('core.forum.header.back_to_index_tooltip'));

this.bodyClass = 'App--index';

this.currentPath = m.route.get();
}

onbeforeupdate(vnode) {
super.onbeforeupdate(vnode);

const curPath = m.route.get();

if (this.currentPath !== curPath) {
this.onNewRoute();

app.discussions.clear();

app.discussions.refreshParams(app.search.params());

this.currentPath = curPath;

this.setTitle();
}
}

view() {
Expand Down
13 changes: 0 additions & 13 deletions js/src/forum/components/UserPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,6 @@ export default class UserPage extends Page {
this.user = null;

this.bodyClass = 'App--user';

this.prevUsername = m.route.param('username');
}

onbeforeupdate() {
const currUsername = m.route.param('username');
if (currUsername !== this.prevUsername) {
this.onNewRoute();

this.prevUsername = currUsername;

this.loadUser(currUsername);
}
}

view() {
Expand Down