Skip to content

Commit

Permalink
fix: fixed localized links and next links redirection (#5474)
Browse files Browse the repository at this point in the history
  • Loading branch information
ovflowd authored Jul 5, 2023
1 parent fabef79 commit cacfb31
Show file tree
Hide file tree
Showing 15 changed files with 1,034 additions and 978 deletions.
19 changes: 0 additions & 19 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,6 @@
"plugin:storybook/recommended"
],
"overrides": [
{
"files": ["**/*.{mjs,js,ts,tsx}"],
"rules": {
"import/order": [
"warn",
{
"groups": [
"builtin",
"external",
"internal",
"sibling",
"parent",
"index",
"type"
]
}
]
}
},
{
"files": ["**/*.ts?(x)"],
"plugins": ["@typescript-eslint"],
Expand Down
10 changes: 5 additions & 5 deletions components/Common/ActiveLocalizedLink/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import { useRouter } from 'next/router';
import { useState, useEffect, type FC } from 'react';
import classNames from 'classnames';
import LocalizedLink from '../../LocalizedLink';
import type { LinkProps } from 'next/link';
import type { PropsWithChildren } from 'react';
import type Link from 'next/link';
import type { ComponentProps } from 'react';

type ActiveLocalizedLinkProps = PropsWithChildren<
LinkProps & { className?: string; activeClassName: string }
>;
type ActiveLocalizedLinkProps = ComponentProps<typeof Link> & {
activeClassName: string;
};

const ActiveLocalizedLink: FC<ActiveLocalizedLinkProps> = ({
children,
Expand Down
3 changes: 1 addition & 2 deletions components/Downloads/DownloadReleasesTable.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { FormattedMessage } from 'react-intl';
import Link from 'next/link';
import { getNodejsChangelog } from '../../util/getNodeJsChangelog';
import { getNodeApiLink } from '../../util/getNodeApiLink';
import { useNodeReleases } from '../../hooks/useNodeReleases';
Expand All @@ -18,7 +17,7 @@ const DownloadReleasesTable: FC = () => {
<td>V8</td>
<td>npm</td>
<td>
NODE_MODULE_VERSION<Link href="#ref-1">[1]</Link>
NODE_MODULE_VERSION<a href="#ref-1">[1]</a>
<span id="backref-1"></span>
</td>
<td></td>
Expand Down
6 changes: 4 additions & 2 deletions components/Downloads/DownloadToggle/index.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
background: var(--black0);
border: none;
border-radius: 32px;
box-shadow: inset 1px -1px 1px rgba(0, 0, 0, 0.08),
box-shadow:
inset 1px -1px 1px rgba(0, 0, 0, 0.08),
inset 0px 1px 2px rgba(0, 0, 0, 0.08);
box-sizing: border-box;
color: var(--black7);
Expand All @@ -44,7 +45,8 @@
width: 133px;

&.current {
box-shadow: inset 1px -1px 1px rgba(0, 0, 0, 0.08),
box-shadow:
inset 1px -1px 1px rgba(0, 0, 0, 0.08),
inset 0px 1px 2px rgba(0, 0, 0, 0.08);
margin-left: -30px;
}
Expand Down
5 changes: 2 additions & 3 deletions components/Home/Banner.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import Link from 'next/link';
import { useSiteConfig } from '../../hooks/useSiteConfig';
import { dateIsBetween } from '../../util/dateIsBetween';

Expand All @@ -17,14 +16,14 @@ const Banner = () => {
if (showBanner && indexBanner.text) {
return (
<p className="home-version home-version-banner">
<Link href={indexBanner.link}>{indexBanner.text}</Link>
<a href={indexBanner.link}>{indexBanner.text}</a>
</p>
);
}

if (showBanner && indexBanner.html) {
return (
<Link
<a
href={indexBanner.link}
dangerouslySetInnerHTML={{ __html: indexBanner.html }}
/>
Expand Down
13 changes: 7 additions & 6 deletions components/Home/HomeDownloadButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ const HomeDownloadButton: FC<NodeRelease> = ({
const nodeDownloadLink = downloadUrlByOS(versionWithPrefix, os, bitness);
const nodeApiLink = `https://nodejs.org/dist/latest-v${major}.x/docs/api/`;
const nodeAllDownloadsLink = `/download${isLts ? '/' : '/current'}`;
const nodeDownloadTitle =
`${labels?.download} ${version}` +
` ${labels?.[isLts ? 'lts' : 'current']}`;

const nodeDownloadTitle = `${labels?.download} ${version} ${labels?.[
isLts ? 'lts' : 'current'
]}`;

return (
<div className="home-downloadblock">
Expand All @@ -44,12 +45,12 @@ const HomeDownloadButton: FC<NodeRelease> = ({
</LocalizedLink>
</li>
<li>
<LocalizedLink href={getNodejsChangelog(versionWithPrefix)}>
<a href={getNodejsChangelog(versionWithPrefix)}>
{labels?.changelog}
</LocalizedLink>
</a>
</li>
<li>
<LocalizedLink href={nodeApiLink}>{labels?.api}</LocalizedLink>
<a href={nodeApiLink}>{labels?.api}</a>
</li>
</ul>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ exports[`Learn/PreviousNextLink Default smoke-test 1`] = `
<ul class="PreviousNextLink_prevNextLink__9SAYu">
<li>
<a rel="prev"
href="/previous"
href="/en/previous"
>
<svg stroke="currentColor"
fill="currentColor"
Expand All @@ -23,7 +23,7 @@ exports[`Learn/PreviousNextLink Default smoke-test 1`] = `
</li>
<li>
<a rel="next"
href="/next"
href="/en/next"
>
NEXT
<svg stroke="currentColor"
Expand All @@ -47,7 +47,7 @@ exports[`Learn/PreviousNextLink WithoutNext smoke-test 1`] = `
<ul class="PreviousNextLink_prevNextLink__9SAYu">
<li>
<a rel="prev"
href="/previous"
href="/en/previous"
>
<svg stroke="currentColor"
fill="currentColor"
Expand Down Expand Up @@ -75,7 +75,7 @@ exports[`Learn/PreviousNextLink WithoutPrevious smoke-test 1`] = `
</li>
<li>
<a rel="next"
href="/next"
href="/en/next"
>
NEXT
<svg stroke="currentColor"
Expand Down
17 changes: 13 additions & 4 deletions components/Learn/PreviousNextLink/__tests__/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@ import { render, screen } from '@testing-library/react';
import { IntlProvider } from 'react-intl';
import PrevNextLink from '..';

jest.mock('next/router', () => ({
useRouter() {
return {
isReady: true,
asPath: '/link',
};
},
}));

describe('PrevNextLink component', () => {
test('renders nothing if neither previous nor next are provided', () => {
render(<PrevNextLink />);
Expand All @@ -17,7 +26,7 @@ describe('PrevNextLink component', () => {
</IntlProvider>
);
const link = screen.getByRole('link');
expect(link).toHaveAttribute('href', previous.slug);
expect(link).toHaveAttribute('href', `/en${previous.slug}`);
});

test('renders next link if next is provided', () => {
Expand All @@ -28,7 +37,7 @@ describe('PrevNextLink component', () => {
</IntlProvider>
);
const link = screen.getByRole('link');
expect(link).toHaveAttribute('href', next.slug);
expect(link).toHaveAttribute('href', `/en${next.slug}`);
});

test('renders both previous and next links if both are provided', () => {
Expand All @@ -40,7 +49,7 @@ describe('PrevNextLink component', () => {
</IntlProvider>
);
const links = screen.getAllByRole('link');
expect(links[0]).toHaveAttribute('href', previous.slug);
expect(links[1]).toHaveAttribute('href', next.slug);
expect(links[0]).toHaveAttribute('href', `/en${previous.slug}`);
expect(links[1]).toHaveAttribute('href', `/en${next.slug}`);
});
});
13 changes: 7 additions & 6 deletions components/Learn/PreviousNextLink/index.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import Link from 'next/link';
import { FaAngleDoubleLeft, FaAngleDoubleRight } from 'react-icons/fa';
import { FormattedMessage } from 'react-intl';
import styles from './index.module.scss';
import LocalizedLink from '../../LocalizedLink';
import type { LinkInfo } from '../../../types';
import type { FC } from 'react';

import styles from './index.module.scss';

type PreviousNextLinkProps = {
previous?: LinkInfo;
next?: LinkInfo;
Expand All @@ -19,18 +20,18 @@ const PreviousNextLink: FC<PreviousNextLinkProps> = ({ previous, next }) => {
<ul className={styles.prevNextLink}>
<li>
{previous && (
<Link href={previous.slug} rel="prev">
<LocalizedLink href={previous.slug} rel="prev">
<FaAngleDoubleLeft size="1em" style={{ marginRight: '5px' }} />
<FormattedMessage id="components.learn.previousNextLink.previous" />
</Link>
</LocalizedLink>
)}
</li>
<li>
{next && (
<Link href={next.slug} rel="next">
<LocalizedLink href={next.slug} rel="next">
<FormattedMessage id="components.learn.previousNextLink.next" />
<FaAngleDoubleRight style={{ marginLeft: '5px' }} />
</Link>
</LocalizedLink>
)}
</li>
</ul>
Expand Down
34 changes: 24 additions & 10 deletions components/LocalizedLink.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,41 @@ import { useLocale } from '../hooks/useLocale';
import { linkWithLocale } from '../util/linkWithLocale';
import type { FC, ComponentProps } from 'react';

// This is a wrapper on HTML's `a` tag
const HtmlLink: FC<JSX.IntrinsicElements['a']> = ({ children, ...extra }) => (
<a {...extra}>{children}</a>
);

// This is Next.js's Link Component but with pre-fetch disabled
const NextLink: FC<ComponentProps<typeof Link>> = ({ children, ...extra }) => (
<Link {...extra} prefetch={false}>
{children}
</Link>
);

const LocalizedLink: FC<ComponentProps<typeof Link>> = ({
href,
children,
...extra
}) => {
const { currentLocale } = useLocale();

const localizedUrl = linkWithLocale(currentLocale.code);
const { Component, finalHref } = useMemo(() => {
if (/^https?:\/\//.test(href.toString())) {
return { Component: HtmlLink, finalHref: href.toString() };
}

const finalHref = useMemo(
() =>
/^https?:\/\//.test(href.toString())
? href.toString()
: localizedUrl(href),
[href, localizedUrl]
);
const addLocaleToHref = linkWithLocale(currentLocale.code);

return { Component: NextLink, finalHref: addLocaleToHref(href) };
// We only need to check if the toString() variant of URL has changed
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [currentLocale.code, href.toString()]);

return (
<Link {...extra} href={finalHref}>
<Component {...extra} href={finalHref}>
{children}
</Link>
</Component>
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ exports[`Sections/NewHeader Default smoke-test 1`] = `
<div class="NewHeader_container__tyM2Y">
<div class="NewHeader_startWrapper__teZG4">
<a aria-label="Homepage"
href="/"
href="/en"
>
<div class="NewHeader_logo__mB_8v">
<div>
Expand Down
9 changes: 5 additions & 4 deletions components/Sections/NewHeader/index.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
import { FormattedMessage } from 'react-intl';
import Image from 'next/image';
import Link from 'next/link';
import { FaGithub } from 'react-icons/fa';
import styles from './index.module.scss';
import LocalizedLink from '../../LocalizedLink';
import ActiveLocalizedLink from '../../Common/ActiveLocalizedLink';
import DarkModeToggle from '../../Common/DarkModeToggle';
import LanguageSelector from '../../Common/LanguageSelector';
import type { FC } from 'react';

import styles from './index.module.scss';

const Header: FC = () => (
<nav aria-label="Primary" className={styles.header}>
<div className={styles.container}>
<div className={styles.startWrapper}>
<Link href="/" aria-label="Homepage">
<LocalizedLink href="/" aria-label="Homepage">
<div className={styles.logo}>
<div>
<Image
Expand All @@ -33,7 +34,7 @@ const Header: FC = () => (
/>
</div>
</div>
</Link>
</LocalizedLink>
</div>

<ul className={styles.tabs}>
Expand Down
Loading

2 comments on commit cacfb31

@vercel
Copy link

@vercel vercel bot commented on cacfb31 Jul 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vercel
Copy link

@vercel vercel bot commented on cacfb31 Jul 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

nodejs-org-stories – ./

nodejs-org-stories-openjs.vercel.app
nodejs-org-stories-git-main-openjs.vercel.app
nodejs-org-storybook.vercel.app

Please sign in to comment.