Skip to content

Commit

Permalink
refactor: remove unnecessary hook (nodejs#6120)
Browse files Browse the repository at this point in the history
* refactor: remove unnecessary hook

* fix: promisify instead of ignoring

* Update util/getBitness.ts

Co-authored-by: canerakdas <canerakdas@gmail.com>
Signed-off-by: Claudio W <cwunder@gnome.org>

---------

Signed-off-by: Claudio W <cwunder@gnome.org>
Co-authored-by: canerakdas <canerakdas@gmail.com>
  • Loading branch information
ovflowd and canerakdas authored Nov 15, 2023
1 parent ea0e0a0 commit 33f1824
Show file tree
Hide file tree
Showing 14 changed files with 47 additions and 102 deletions.
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
import { render, screen } from '@testing-library/react';

import ActiveLocalizedLink from '..';
import ActiveLink from '..';

describe('ActiveLocalizedLink', () => {
describe('ActiveLink', () => {
it('renders as localized link', () => {
render(
<ActiveLocalizedLink
className="link"
activeClassName="active"
href="/link"
>
<ActiveLink className="link" activeClassName="active" href="/link">
Link
</ActiveLocalizedLink>
</ActiveLink>
);

expect(screen.findByText('Link')).resolves.toHaveAttribute(
Expand All @@ -22,27 +18,19 @@ describe('ActiveLocalizedLink', () => {

it('ignores active class when href not matches location.href', () => {
render(
<ActiveLocalizedLink
className="link"
activeClassName="active"
href="/not-link"
>
<ActiveLink className="link" activeClassName="active" href="/not-link">
Link
</ActiveLocalizedLink>
</ActiveLink>
);

expect(screen.findByText('Link')).resolves.toHaveAttribute('class', 'link');
});

it('sets active class when href matches location.href', () => {
render(
<ActiveLocalizedLink
className="link"
activeClassName="active"
href="/link"
>
<ActiveLink className="link" activeClassName="active" href="/link">
Link
</ActiveLocalizedLink>
</ActiveLink>
);

expect(screen.findByText('Link')).resolves.toHaveAttribute(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,24 @@ import Link from '@/components/Link';
import { usePathname } from '@/navigation.mjs';

type ActiveLocalizedLinkProps = ComponentProps<typeof Link> & {
activeClassName: string;
activeClassName?: string;
allowSubPath?: boolean;
};

const ActiveLocalizedLink: FC<ActiveLocalizedLinkProps> = ({
const ActiveLink: FC<ActiveLocalizedLinkProps> = ({
children,
activeClassName,
activeClassName = 'active',
allowSubPath = false,
className,
href = '',
...props
}) => {
const pathname = usePathname();

const linkURL = new URL(href.toString(), location.href);

const finalClassName = classNames(className, {
[activeClassName]: linkURL.pathname === pathname,
[activeClassName]: allowSubPath
? pathname.startsWith(href.toString())
: href.toString() === pathname,
});

return (
Expand All @@ -32,4 +34,4 @@ const ActiveLocalizedLink: FC<ActiveLocalizedLinkProps> = ({
);
};

export default ActiveLocalizedLink;
export default ActiveLink;
8 changes: 4 additions & 4 deletions components/Containers/NavItem/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import classNames from 'classnames';
import type { FC, PropsWithChildren } from 'react';
import { useMemo } from 'react';

import ActiveLocalizedLink from '@/components/Common/ActiveLocalizedLink';
import ActiveLink from '@/components/Common/ActiveLink';

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

Expand All @@ -22,19 +22,19 @@ const NavItem: FC<PropsWithChildren<NavItemProps>> = ({
className,
}) => {
const showIcon = useMemo(
() => type === 'nav' && !href.toString().startsWith('/'),
() => type === 'nav' && href.toString().startsWith('http'),
[href, type]
);

return (
<ActiveLocalizedLink
<ActiveLink
href={href}
className={classNames(styles.navItem, styles[type], className)}
activeClassName={styles.active}
>
<span className={styles.label}>{children}</span>
{showIcon && <ArrowUpRightIcon className={styles.icon} />}
</ActiveLocalizedLink>
</ActiveLink>
);
};

Expand Down
14 changes: 6 additions & 8 deletions components/Header.tsx
Original file line number Diff line number Diff line change
@@ -1,29 +1,25 @@
'use client';

import classNames from 'classnames';
import Image from 'next/image';
import { useTranslations } from 'next-intl';
import { useTheme } from 'next-themes';
import { useState } from 'react';

import ActiveLink from '@/components/Common/ActiveLink';
import Link from '@/components/Link';
import { useIsCurrentPathname, useSiteNavigation } from '@/hooks';
import { useSiteNavigation } from '@/hooks';
import { usePathname } from '@/navigation.mjs';
import { BASE_PATH } from '@/next.constants.mjs';
import { availableLocales } from '@/next.locales.mjs';

const Header = () => {
const { navigationItems } = useSiteNavigation();
const { isCurrentLocaleRoute } = useIsCurrentPathname();
const [showLangPicker, setShowLangPicker] = useState(false);
const { theme, setTheme } = useTheme();

const pathname = usePathname();
const t = useTranslations();

const getLinkClassName = (href: string) =>
classNames({ active: isCurrentLocaleRoute(href, href !== '/') });

const toggleLanguage = t('components.header.buttons.toggleLanguage');
const toggleDarkMode = t('components.header.buttons.toggleDarkMode');

Expand All @@ -43,8 +39,10 @@ const Header = () => {
<nav aria-label="primary">
<ul className="list-divider-pipe">
{navigationItems.map((item, key) => (
<li key={key} className={getLinkClassName(item.link)}>
<Link href={item.link}>{item.text}</Link>
<li key={key}>
<ActiveLink href={item.link} allowSubPath>
{item.text}
</ActiveLink>
</li>
))}
</ul>
Expand Down
21 changes: 7 additions & 14 deletions components/SideNavigation.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
'use client';

import classNames from 'classnames';
import type { RichTranslationValues } from 'next-intl';
import type { FC } from 'react';

import Link from '@/components/Link';
import { useIsCurrentPathname, useSiteNavigation } from '@/hooks';
import ActiveLink from '@/components/Common/ActiveLink';
import { useSiteNavigation } from '@/hooks/server';
import type { NavigationKeys } from '@/types';

type SideNavigationProps = {
Expand All @@ -18,25 +15,21 @@ const SideNavigation: FC<SideNavigationProps> = ({
context,
}) => {
const { getSideNavigation } = useSiteNavigation();
const { isCurrentLocaleRoute } = useIsCurrentPathname();

const sideNavigationItems = getSideNavigation(navigationKey, context);

const getLinkClasses = (href: string, level: number) =>
classNames({ active: isCurrentLocaleRoute(href), level });

return (
<nav aria-label="secondary">
<ul>
{sideNavigationItems.map(item => (
<li key={item.key} className={getLinkClasses(item.link, item.level)}>
<Link href={item.link}>{item.text}</Link>
<li key={item.key}>
<ActiveLink href={item.link}>{item.text}</ActiveLink>

{item.items.length > 0 && (
<ul>
{item.items.map(({ link, level, text, key }) => (
<li key={key} className={getLinkClasses(link, level)}>
<Link href={link}>{text}</Link>
{item.items.map(({ link, text, key }) => (
<li key={key}>
<ActiveLink href={link}>{text}</ActiveLink>
</li>
))}
</ul>
Expand Down
1 change: 0 additions & 1 deletion hooks/react-client/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
export { default as useCopyToClipboard } from './useCopyToClipboard';
export { default as useDetectOS } from './useDetectOS';
export { default as useIsCurrentPathname } from './useIsCurrentPathname';
export { default as useMediaQuery } from './useMediaQuery';
export { default as useNotification } from './useNotification';
export { default as useClientContext } from './useClientContext';
Expand Down
14 changes: 0 additions & 14 deletions hooks/react-client/useIsCurrentPathname.ts

This file was deleted.

1 change: 0 additions & 1 deletion hooks/react-server/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
export { default as useCopyToClipboard } from './useCopyToClipboard';
export { default as useDetectOS } from './useDetectOS';
export { default as useIsCurrentPathname } from './useIsCurrentPathname';
export { default as useMediaQuery } from './useMediaQuery';
export { default as useNotification } from './useNotification';
export { default as useClientContext } from './useClientContext';
Expand Down
17 changes: 0 additions & 17 deletions hooks/react-server/useIsCurrentPathname.ts

This file was deleted.

2 changes: 0 additions & 2 deletions layouts/DocsLayout.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
'use client';

import type { RichTranslationValues } from 'next-intl';
import type { FC, PropsWithChildren } from 'react';

Expand Down
6 changes: 2 additions & 4 deletions styles/old/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,9 @@ article a {
margin-left: -10px;
margin-right: -10px;
padding: 5px 10px;
}

.active {
> a,
> a:hover {
&.active,
&.active:hover {
background-color: $active-green;
color: $white;
}
Expand Down
9 changes: 5 additions & 4 deletions styles/old/layout/dark-theme.css
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,12 @@ html[data-theme='dark'] {
&:hover {
color: $white;
}
}

.active > a {
background-color: $active-green;
color: $white;
&.active,
&.active:hover {
background-color: $active-green;
color: $white;
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions styles/old/page-modules/header.css
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ header {

$border-width: 14px;

&.active::after {
&:has(a.active)::after {
border: solid transparent;
border-top-color: $node-gray;
border-width: $border-width;
Expand All @@ -156,7 +156,7 @@ header {
width: 0;
}

&.active:first-child::after {
&:has(a.active):first-child::after {
margin-left: -$border-width;
}
}
Expand Down
8 changes: 4 additions & 4 deletions util/getBitness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ export const getBitness = async () => {
'bitness',
]);

// Apparently in some cases this is not a Promise, then we should ignore
if (entropyValues instanceof Promise) {
return entropyValues.then(ua => ua.bitness);
}
// Apparently in some cases this is not a Promise, we can Promisify it.
return Promise.resolve(entropyValues)
.then(({ bitness }) => bitness)
.catch(() => undefined);
}

return undefined;
Expand Down

0 comments on commit 33f1824

Please sign in to comment.