Skip to content

Fix sublinks display on mobile #2620

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Dec 11, 2024
Merged

Fix sublinks display on mobile #2620

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d9383ac
Fix sublinks display on mobile
gregberge Dec 11, 2024
d8d7855
Merge branch 'main' into greg/rnd-5468-the-sublink-of-header-will-dis…
gregberge Dec 11, 2024
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
43 changes: 25 additions & 18 deletions packages/gitbook/src/components/Header/Dropdown.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export function Dropdown<E extends HTMLElement>(props: {
aria-labelledby={dropdownId}
className={tcls(
'w-52',
'max-h-56',
'max-h-80',
'flex',
'absolute',
'top-full',
Expand Down Expand Up @@ -111,31 +111,38 @@ export function DropdownMenu(props: { children: React.ReactNode }) {
* Menu item in a dropdown.
*/
export function DropdownMenuItem(props: {
href: string;
href: string | null;
active?: boolean;
className?: ClassValue;
Copy link
Member

Choose a reason for hiding this comment

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

I'm not familiar with the GBO dev env yet : why isn't that a string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We manipulate clsx values, but IMO we should not. All components should only accept a className?: string and if you need to merge thing you can do it when you pass the prop: <Any className={clsx(...)} /> like we do in the app.

children: React.ReactNode;
}) {
const { children, active = false, href } = props;
const { children, active = false, href, className } = props;

if (href) {
return (
<Link
href={href}
prefetch={false}
className={tcls(
Copy link
Member

Choose a reason for hiding this comment

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

Curious to know why we went for tcls instead of clsx on GBO?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a mistake, but it's like that from the beginning. At some point we should change it to improve perfs.

'px-3 py-1 text-sm rounded straight-corners:rounded-sm',
active ? 'bg-primary/3 dark:bg-light/2 text-primary-600' : null,
'hover:bg-dark/2 dark:hover:bg-light/2',
className,
)}
>
{children}
</Link>
);
}

return (
<Link
href={href}
prefetch={false}
<div
className={tcls(
'flex',
'flex-row',
'items-center',
'text-sm',
'px-3',
'py-1',
'rounded',
'straight-corners:rounded-sm',
active
? ['bg-primary/3', 'dark:bg-light/2', 'text-primary-600']
: ['hover:bg-dark/2', 'dark:hover:bg-light/2'],
'text-xs px-3 py-1 font-medium text-dark/8 dark:text-light/8',
className,
)}
>
{children}
</Link>
</div>
);
}
24 changes: 18 additions & 6 deletions packages/gitbook/src/components/Header/HeaderLinkMore.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
CustomizationContentLink,
CustomizationHeaderItem,
CustomizationHeaderPreset,
CustomizationSettings,
Expand Down Expand Up @@ -55,14 +56,25 @@ export function HeaderLinkMore(props: {
);
}

async function MoreMenuLink(props: { context: ContentRefContext; link: CustomizationHeaderItem }) {
async function MoreMenuLink(props: {
context: ContentRefContext;
link: CustomizationHeaderItem | CustomizationContentLink;
}) {
const { context, link } = props;

const target = link.to ? await resolveContentRef(link.to, context) : null;

if (!target) {
return null;
}

return <DropdownMenuItem href={target.href}>{link.title}</DropdownMenuItem>;
return (
<>
{'links' in link && link.links.length > 0 && (
<hr className="first:hidden border-t border-light-3 dark:border-dark-3 my-1 -mx-2" />
)}
<DropdownMenuItem href={target?.href ?? null}>{link.title}</DropdownMenuItem>
{'links' in link
? link.links.map((subLink, index) => (
<MoreMenuLink key={index} {...props} link={subLink} />
))
: null}
</>
);
}
Loading