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

Use Link component for links instead of mithril route patch #2315

Merged
merged 7 commits into from
Oct 2, 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 @@ -47,6 +47,7 @@ import FieldSet from './components/FieldSet';
import Select from './components/Select';
import Navigation from './components/Navigation';
import Alert from './components/Alert';
import Link from './components/Link';
import LinkButton from './components/LinkButton';
import Checkbox from './components/Checkbox';
import SelectDropdown from './components/SelectDropdown';
Expand Down Expand Up @@ -118,6 +119,7 @@ export default {
'components/Select': Select,
'components/Navigation': Navigation,
'components/Alert': Alert,
'components/Link': Link,
'components/LinkButton': LinkButton,
'components/Checkbox': Checkbox,
'components/SelectDropdown': SelectDropdown,
Expand Down
47 changes: 47 additions & 0 deletions js/src/common/components/Link.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import Component from '../Component';
import extract from '../utils/extract';

/**
* The link component enables both internal and external links.
* It will return a regular HTML link for any links to external sites,
* and it will use Mithril's m.route.Link for any internal links.
*
* Links will default to internal; the 'external' attr must be set to
* `true` for the link to be external.
*/
export default class Link extends Component {
view(vnode) {
let { options = {}, ...attrs } = vnode.attrs;

attrs.href = attrs.href || '';

// For some reason, m.route.Link does not like vnode.text, so if present, we
// need to convert it to text vnodes and store it in children.
const children = vnode.children || { tag: '#', children: vnode.text };

if (attrs.external) {
return <a {...attrs}>{children}</a>;
}

// If the href URL of the link is the same as the current page path
// we will not add a new entry to the browser history.
// This allows us to still refresh the Page component
// without adding endless history entries.
if (attrs.href === m.route.get()) {
if (!('replace' in options)) options.replace = true;
}

// Mithril 2 does not completely rerender the page if a route change leads to the same route
// (or the same component handling a different route).
// Here, the `force` parameter will use Mithril's key system to force a full rerender
// see https://mithril.js.org/route.html#key-parameter
if (extract(attrs, 'force')) {
if (!('state' in options)) options.state = {};
if (!('key' in options.state)) options.state.key = Date.now();
}

attrs.options = options;

return <m.route.Link {...attrs}>{children}</m.route.Link>;
}
}
3 changes: 2 additions & 1 deletion js/src/common/components/LinkButton.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Button from './Button';
import Link from './Link';

/**
* The `LinkButton` component defines a `Button` which links to a route.
Expand All @@ -22,7 +23,7 @@ export default class LinkButton extends Button {
view(vnode) {
const vdom = super.view(vnode);

vdom.tag = m.route.Link;
vdom.tag = Link;
vdom.attrs.active = String(vdom.attrs.active);

return vdom;
Expand Down
53 changes: 13 additions & 40 deletions js/src/common/utils/patchMithril.js
Original file line number Diff line number Diff line change
@@ -1,43 +1,15 @@
import extract from './extract';
import Link from '../components/Link';
import withAttr from './withAttr';
import Stream from './Stream';

let deprecatedMPropWarned = false;
let deprecatedMWithAttrWarned = false;

let deprecatedRouteWarned = false;

export default function patchMithril(global) {
const defaultMithril = global.m;

/**
* If the href URL of the link is the same as the current page path
* we will not add a new entry to the browser history.
*
* This allows us to still refresh the Page component
* without adding endless history entries.
*
* We also add the `force` attribute that adds a custom state key
* for when you want to force a complete refresh of the Page
*/
const defaultLinkView = defaultMithril.route.Link.view;
const modifiedLink = {
view: function (vnode) {
let { href, options = {} } = vnode.attrs;

if (href === m.route.get()) {
if (!('replace' in options)) options.replace = true;
}

if (extract(vnode.attrs, 'force')) {
if (!('state' in options)) options.state = {};
if (!('key' in options.state)) options.state.key = Date.now();
}

vnode.attrs.options = options;

return defaultLinkView(vnode);
},
};

const modifiedMithril = function (comp, ...args) {
const node = defaultMithril.apply(this, arguments);

Expand All @@ -48,28 +20,29 @@ export default function patchMithril(global) {
modifiedMithril.bidi(node, node.attrs.bidi);
}

// DEPRECATED, REMOVE BETA 15

// Allows us to use a "route" attr on links, which will automatically convert the link to one which
// supports linking to other pages in the SPA without refreshing the document.
if (node.attrs.route) {
node.attrs.href = node.attrs.route;
node.tag = modifiedLink;

// For some reason, m.route.Link does not like vnode.text, so if present, we
// need to convert it to text vnodes and store it in children.
if (node.text) {
node.children = { tag: '#', children: node.text };
if (!deprecatedRouteWarned) {
deprecatedRouteWarned = true;
console.warn('The route attr patch for links is deprecated, please use the Link component (flarum/components/Link) instead.');
}

node.attrs.href = node.attrs.route;
node.tag = Link;

delete node.attrs.route;
}

// END DEPRECATED

return node;
};

Object.keys(defaultMithril).forEach((key) => (modifiedMithril[key] = defaultMithril[key]));

modifiedMithril.route.Link = modifiedLink;

// BEGIN DEPRECATED MITHRIL 2 BC LAYER
modifiedMithril.prop = modifiedMithril.stream = function (...args) {
if (!deprecatedMPropWarned) {
Expand Down
11 changes: 6 additions & 5 deletions js/src/forum/components/DiscussionListItem.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Component from '../../common/Component';
import Link from '../../common/components/Link';
import avatar from '../../common/helpers/avatar';
import listItems from '../../common/helpers/listItems';
import highlight from '../../common/helpers/highlight';
Expand Down Expand Up @@ -98,8 +99,8 @@ export default class DiscussionListItem extends Component {
</a>

<div className={'DiscussionListItem-content Slidable-content' + (isUnread ? ' unread' : '') + (isRead ? ' read' : '')}>
<a
route={user ? app.route.user(user) : '#'}
<Link
href={user ? app.route.user(user) : '#'}
className="DiscussionListItem-author"
title={extractText(
app.translator.trans('core.forum.discussion_list.started_text', { user: user, ago: humanTime(discussion.createdAt()) })
Expand All @@ -109,14 +110,14 @@ export default class DiscussionListItem extends Component {
}}
>
{avatar(user, { title: '' })}
</a>
</Link>

<ul className="DiscussionListItem-badges badges">{listItems(discussion.badges().toArray())}</ul>

<a route={app.route.discussion(discussion, jumpTo)} className="DiscussionListItem-main">
<Link href={app.route.discussion(discussion, jumpTo)} className="DiscussionListItem-main">
<h3 className="DiscussionListItem-title">{highlight(discussion.title(), this.highlightRegExp)}</h3>
<ul className="DiscussionListItem-info">{listItems(this.infoItems().toArray())}</ul>
</a>
</Link>

<span
className="DiscussionListItem-count"
Expand Down
5 changes: 3 additions & 2 deletions js/src/forum/components/DiscussionsSearchSource.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import highlight from '../../common/helpers/highlight';
import LinkButton from '../../common/components/LinkButton';
import Link from '../../common/components/Link';

/**
* The `DiscussionsSearchSource` finds and displays discussion search results in
Expand Down Expand Up @@ -47,10 +48,10 @@ export default class DiscussionsSearchSource {

return (
<li className="DiscussionSearchResult" data-index={'discussions' + discussion.id()}>
<a route={app.route.discussion(discussion, mostRelevantPost && mostRelevantPost.number())}>
<Link href={app.route.discussion(discussion, mostRelevantPost && mostRelevantPost.number())}>
<div className="DiscussionSearchResult-title">{highlight(discussion.title(), query)}</div>
{mostRelevantPost ? <div className="DiscussionSearchResult-excerpt">{highlight(mostRelevantPost.contentPlain(), query, 100)}</div> : ''}
</a>
</Link>
</li>
);
}),
Expand Down
5 changes: 3 additions & 2 deletions js/src/forum/components/EditPostComposer.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import ComposerBody from './ComposerBody';
import Button from '../../common/components/Button';
import Link from '../../common/components/Link';
import icon from '../../common/helpers/icon';

function minimizeComposerIfFullScreen(e) {
Expand Down Expand Up @@ -39,9 +40,9 @@ export default class EditPostComposer extends ComposerBody {
'title',
<h3>
{icon('fas fa-pencil-alt')}{' '}
<a route={app.route.discussion(post.discussion(), post.number())} onclick={minimizeComposerIfFullScreen}>
<Link href={app.route.discussion(post.discussion(), post.number())} onclick={minimizeComposerIfFullScreen}>
{app.translator.trans('core.forum.composer_edit.post_link', { number: post.number(), discussion: post.discussion().title() })}
</a>
</Link>
</h3>
);

Expand Down
5 changes: 3 additions & 2 deletions js/src/forum/components/EventPost.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import Post from './Post';
import { ucfirst } from '../../common/utils/string';
import usernameHelper from '../../common/helpers/username';
import icon from '../../common/helpers/icon';
import Link from '../../common/components/Link';

/**
* The `EventPost` component displays a post which indicating a discussion
Expand Down Expand Up @@ -29,9 +30,9 @@ export default class EventPost extends Post {
const data = Object.assign(this.descriptionData(), {
user,
username: user ? (
<a className="EventPost-user" route={app.route.user(user)}>
<Link className="EventPost-user" href={app.route.user(user)}>
{username}
</a>
</Link>
) : (
username
),
Expand Down
11 changes: 5 additions & 6 deletions js/src/forum/components/Notification.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import avatar from '../../common/helpers/avatar';
import icon from '../../common/helpers/icon';
import humanTime from '../../common/helpers/humanTime';
import Button from '../../common/components/Button';
import Link from '../../common/components/Link';

/**
* The `Notification` component abstract displays a single notification.
Expand All @@ -19,13 +20,11 @@ export default class Notification extends Component {
const notification = this.attrs.notification;
const href = this.href();

const linkAttrs = {};
linkAttrs[href.indexOf('://') === -1 ? 'route' : 'href'] = href;

return (
<a
<Link
className={'Notification Notification--' + notification.contentType() + ' ' + (!notification.isRead() ? 'unread' : '')}
{...linkAttrs}
href={href}
external={href.indexOf('://') === -1}
onclick={this.markAsRead.bind(this)}
>
{!notification.isRead() &&
Expand All @@ -45,7 +44,7 @@ export default class Notification extends Component {
<span className="Notification-content">{this.content()}</span>
{humanTime(notification.createdAt())}
<div className="Notification-excerpt">{this.excerpt()}</div>
</a>
</Link>
);
}

Expand Down
5 changes: 3 additions & 2 deletions js/src/forum/components/NotificationList.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Component from '../../common/Component';
import listItems from '../../common/helpers/listItems';
import Button from '../../common/components/Button';
import Link from '../../common/components/Link';
import LoadingIndicator from '../../common/components/LoadingIndicator';
import Discussion from '../../common/models/Discussion';

Expand Down Expand Up @@ -63,10 +64,10 @@ export default class NotificationList extends Component {
return (
<div className="NotificationGroup">
{group.discussion ? (
<a className="NotificationGroup-header" route={app.route.discussion(group.discussion)}>
<Link className="NotificationGroup-header" href={app.route.discussion(group.discussion)}>
{badges && badges.length ? <ul className="NotificationGroup-badges badges">{listItems(badges)}</ul> : ''}
{group.discussion.title()}
</a>
</Link>
) : (
<div className="NotificationGroup-header">{app.forum.attribute('title')}</div>
)}
Expand Down
5 changes: 3 additions & 2 deletions js/src/forum/components/PostPreview.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Component from '../../common/Component';
import Link from '../../common/components/Link';
import avatar from '../../common/helpers/avatar';
import username from '../../common/helpers/username';
import highlight from '../../common/helpers/highlight';
Expand All @@ -18,12 +19,12 @@ export default class PostPreview extends Component {
const excerpt = highlight(post.contentPlain(), this.attrs.highlight, 300);

return (
<a className="PostPreview" route={app.route.post(post)} onclick={this.attrs.onclick}>
<Link className="PostPreview" href={app.route.post(post)} onclick={this.attrs.onclick}>
<span className="PostPreview-content">
{avatar(user)}
{username(user)} <span className="PostPreview-excerpt">{excerpt}</span>
</span>
</a>
</Link>
);
}
}
5 changes: 3 additions & 2 deletions js/src/forum/components/PostUser.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Component from '../../common/Component';
import Link from '../../common/components/Link';
import UserCard from './UserCard';
import avatar from '../../common/helpers/avatar';
import username from '../../common/helpers/username';
Expand Down Expand Up @@ -40,11 +41,11 @@ export default class PostUser extends Component {
return (
<div className="PostUser">
<h3>
<a route={app.route.user(user)}>
<Link href={app.route.user(user)}>
{avatar(user, { className: 'PostUser-avatar' })}
{userOnline(user)}
{username(user)}
</a>
</Link>
</h3>
<ul className="PostUser-badges badges">{listItems(user.badges().toArray())}</ul>
{card}
Expand Down
3 changes: 2 additions & 1 deletion js/src/forum/components/PostsUserPage.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import UserPage from './UserPage';
import LoadingIndicator from '../../common/components/LoadingIndicator';
import Button from '../../common/components/Button';
import Link from '../../common/components/Link';
import Placeholder from '../../common/components/Placeholder';
import CommentPost from './CommentPost';

Expand Down Expand Up @@ -73,7 +74,7 @@ export default class PostsUserPage extends UserPage {
<li>
<div className="PostsUserPage-discussion">
{app.translator.trans('core.forum.user.in_discussion_text', {
discussion: <a route={app.route.post(post)}>{post.discussion().title()}</a>,
discussion: <Link href={app.route.post(post)}>{post.discussion().title()}</Link>,
})}
</div>

Expand Down
5 changes: 3 additions & 2 deletions js/src/forum/components/ReplyComposer.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import ComposerBody from './ComposerBody';
import Button from '../../common/components/Button';
import Link from '../../common/components/Link';
import icon from '../../common/helpers/icon';
import extractText from '../../common/utils/extractText';

Expand Down Expand Up @@ -36,9 +37,9 @@ export default class ReplyComposer extends ComposerBody {
'title',
<h3>
{icon('fas fa-reply')}{' '}
<a route={app.route.discussion(discussion)} onclick={minimizeComposerIfFullScreen}>
<Link href={app.route.discussion(discussion)} onclick={minimizeComposerIfFullScreen}>
{discussion.title()}
</a>
</Link>
</h3>
);

Expand Down
Loading