Skip to content

Commit 1b87212

Browse files
birdofpreyrubirdofpreyru
committed
A better fix for <Link> and <NavLink>
1 parent 6c02d4a commit 1b87212

File tree

7 files changed

+135
-71
lines changed

7 files changed

+135
-71
lines changed

‎CHANGELOG.md

+3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
# Topcoder React Utils Changelog
22

3+
### v0.7.2
4+
Fix of `<Link>` and `<NavLink>` logic.
5+
36
### v0.7.0
47
Fixes various sins left in the previous version.
58

‎__tests__/shared/components/GenericLink.jsx

+69-47
Original file line numberDiff line numberDiff line change
@@ -32,68 +32,90 @@ Link.propTypes = {
3232
onClick: PT.func.isRequired,
3333
};
3434

35-
describe('Matches snapshots', () => {
36-
test('in a minimalistic example', () => {
37-
snapshot((
38-
<GenericLink
39-
routerLinkType={Link}
40-
to="SOME/TEST/URL"
41-
>
42-
LINK
43-
</GenericLink>
44-
));
45-
});
46-
test('when rendered as <a> element', () => {
47-
snapshot((
48-
<GenericLink
49-
enforceA
50-
routerLinkType={Link}
51-
to="SOME/TEST/URL"
52-
>
53-
LINK
54-
</GenericLink>
55-
));
56-
snapshot((
57-
<GenericLink
58-
openNewTab
59-
routerLinkType={Link}
60-
to="SOME/TEST/URL"
61-
>
62-
LINK
63-
</GenericLink>
64-
));
65-
});
35+
test('Absolute link, starting with http://', () => {
36+
snapshot((
37+
<GenericLink
38+
routerLinkType={Link}
39+
to="http://www.domain.com/test"
40+
>
41+
ABSOLUTE LINK
42+
</GenericLink>
43+
));
6644
});
6745

68-
test('onClick(..) works when rendered as custom <Link>', () => {
69-
window.scroll = jest.fn();
70-
const clickHandler = jest.fn();
71-
let doc = renderDom((
46+
test('Absolute link, starting with https://', () => {
47+
snapshot((
7248
<GenericLink
73-
className="LINK"
74-
onClick={clickHandler}
49+
onClick={() => {}}
7550
routerLinkType={Link}
76-
to="SOME/TEST/URL"
51+
to="https://www.domain.com/test"
7752
>
78-
LINK
53+
ABSOLUTE LINK
7954
</GenericLink>
8055
));
81-
let link = findInDomByClass(doc, 'LINK');
82-
simulate.click(link);
83-
expect(clickHandler).toHaveBeenCalled();
56+
});
57+
58+
test('Relative link', () => {
59+
snapshot((
60+
<GenericLink
61+
routerLinkType={Link}
62+
to="http/relative/link"
63+
>
64+
RELATIVE LINK
65+
</GenericLink>
66+
));
67+
});
68+
69+
test('Relative link, with `enforceA`', () => {
70+
snapshot((
71+
<GenericLink
72+
enforceA
73+
routerLinkType={Link}
74+
to="/relative/link"
75+
>
76+
RELATIVE LINK
77+
</GenericLink>
78+
));
79+
});
80+
81+
test('Relative link, with `openNewTab`', () => {
82+
snapshot((
83+
<GenericLink
84+
openNewTab
85+
routerLinkType={Link}
86+
to="relative/link"
87+
>
88+
RELATIVE LINL
89+
</GenericLink>
90+
));
91+
});
92+
93+
test('Anchor link', () => {
94+
snapshot((
95+
<GenericLink
96+
routerLinkType={Link}
97+
to="#anchor"
98+
>
99+
ANCHOR LINK
100+
</GenericLink>
101+
));
102+
});
84103

85-
const domain = 'https://some.domain.com';
86-
doc = renderDom((
104+
test('onClick(..) callback in custom <Link>', () => {
105+
window.scroll = jest.fn();
106+
const clickHandler = jest.fn();
107+
const doc = renderDom((
87108
<GenericLink
88109
className="LINK"
110+
onClick={clickHandler}
89111
routerLinkType={Link}
90-
to={domain}
112+
to="SOME/TEST/URL"
91113
>
92114
LINK
93115
</GenericLink>
94116
));
95-
link = findInDomByClass(doc, 'LINK');
117+
const link = findInDomByClass(doc, 'LINK');
96118
simulate.click(link);
97-
119+
expect(clickHandler).toHaveBeenCalled();
98120
expect(window.scroll).toHaveBeenCalledTimes(1);
99121
});

‎__tests__/shared/components/__snapshots__/GenericLink.jsx.snap

+47-8
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,76 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3-
exports[`Matches snapshots in a minimalistic example 1`] = `
3+
exports[`Absolute link, starting with http:// 1`] = `
4+
<a
5+
className={null}
6+
href="http://www.domain.com/test"
7+
onClick={null}
8+
onMouseDown={null}
9+
rel="noopener noreferrer"
10+
target=""
11+
>
12+
ABSOLUTE LINK
13+
</a>
14+
`;
15+
16+
exports[`Absolute link, starting with https:// 1`] = `
17+
<a
18+
className={null}
19+
href="https://www.domain.com/test"
20+
onClick={[Function]}
21+
onMouseDown={null}
22+
rel="noopener noreferrer"
23+
target=""
24+
>
25+
ABSOLUTE LINK
26+
</a>
27+
`;
28+
29+
exports[`Anchor link 1`] = `
30+
<a
31+
className={null}
32+
href="#anchor"
33+
onClick={null}
34+
onMouseDown={null}
35+
rel="noopener noreferrer"
36+
target=""
37+
>
38+
ANCHOR LINK
39+
</a>
40+
`;
41+
42+
exports[`Relative link 1`] = `
443
<button
544
className={null}
645
onClick={[Function]}
746
type="button"
847
>
9-
{"to":"SOME/TEST/URL","className":null,"onMouseDown":null,"replace":false,"children":"LINK"}
48+
{"to":"http/relative/link","className":null,"onMouseDown":null,"replace":false,"children":"RELATIVE LINK"}
1049
</button>
1150
`;
1251

13-
exports[`Matches snapshots when rendered as <a> element 1`] = `
52+
exports[`Relative link, with \`enforceA\` 1`] = `
1453
<a
1554
className={null}
16-
href="SOME/TEST/URL"
55+
href="/relative/link"
1756
onClick={null}
1857
onMouseDown={null}
1958
rel="noopener noreferrer"
2059
target=""
2160
>
22-
LINK
61+
RELATIVE LINK
2362
</a>
2463
`;
2564

26-
exports[`Matches snapshots when rendered as <a> element 2`] = `
65+
exports[`Relative link, with \`openNewTab\` 1`] = `
2766
<a
2867
className={null}
29-
href="SOME/TEST/URL"
68+
href="relative/link"
3069
onClick={null}
3170
onMouseDown={null}
3271
rel="noopener noreferrer"
3372
target="_blank"
3473
>
35-
LINK
74+
RELATIVE LINL
3675
</a>
3776
`;

‎docs/link-and-navlink.md

+6-8
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,14 @@ external or internal), and also when you have to frequently change the actual
1818
link addresses, as it often happens during active development / prototyping.
1919

2020
Our `<Link>` and `<NavLink>` are rendered as simple `<a>` elements when:
21-
1. Explicitly opted by the `enforceA` property;
22-
2. The link should be opened in a new tab (`openNewTab` property);
23-
3. The link points to an anchor (starts with `#` symbol).
21+
1. The link is absolute, i.e. starts with `http://` or `https://`;
22+
2. The link points to an anchor, i.e. starts with `#` symbol;
23+
3. The link should be opened in a new tab (`openNewTab` property);
24+
4. Explicitly opted by the `enforceA` property;
2425

2526
Otherwise, `<Link>` and `<NavLink>` are rendered as the corresponding React
26-
Router's components. If the URL provided via `to` property points to a different
27-
domain, it is treated as an external link and transition is handled assigning it
28-
to the `document.location`, otherwise transition is handled by the React Router.
29-
In case an extrenal link shares the same domain with the application, you can
30-
use `enforceA` property to handle such case.
27+
Router's components. Additionally in this case, the links to the current page,
28+
when clicked, scroll the page to the top.
3129

3230
Both `<Link>` and `<NavLink>` support all properties of the underlying React
3331
Router's components, along with some additional props:

‎package-lock.json

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -121,5 +121,5 @@
121121
"mkDistDir:prod": "mkdir -p dist/prod/shared/utils && mkdir -p dist/prod/client",
122122
"test": "npm run lint && npm run jest"
123123
},
124-
"version": "0.7.1"
124+
"version": "0.7.2"
125125
}

‎src/shared/components/GenericLink.jsx

+8-6
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import _ from 'lodash';
1212
import PT from 'prop-types';
1313
import React from 'react';
14-
import Url from 'url-parse';
1514

1615
export default function GenericLink(props) {
1716
const {
@@ -26,11 +25,12 @@ export default function GenericLink(props) {
2625
to,
2726
} = props;
2827

29-
const url = new Url(to);
30-
31-
/* Renders the link as <a> element if either opted explicitely, or the link
32-
* should open a new tab, or it is an anchor reference. */
33-
if (enforceA || openNewTab || to.startsWith('#') || url.host) {
28+
/* Renders Link as <a> element if:
29+
* - It is opted explicitely by `enforceA` prop;
30+
* - It should be opened in a new tab;
31+
* - It is an absolte URL (starts with http:// or https://);
32+
* - It is anchor link (starts with #). */
33+
if (enforceA || openNewTab || to.match(/^(#|https?:\/\/)/)) {
3434
return (
3535
<a
3636
className={className}
@@ -59,6 +59,8 @@ export default function GenericLink(props) {
5959
onClick: (e) => {
6060
/* If a custom onClick(..) handler was provided we execute it. */
6161
if (onClick) onClick(e);
62+
63+
/* The link to the current page will scroll to the top of the page. */
6264
window.scroll(0, 0);
6365
},
6466
}, children);

0 commit comments

Comments
 (0)