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

[WEB-4242] Mobile navigation fixes #2486

Merged
merged 13 commits into from
Mar 21, 2025
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: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
"no-githooks": "git config --unset core.hooksPath"
},
"dependencies": {
"@ably/ui": "15.7.0",
"@ably/ui": "15.8.1",
"@codesandbox/sandpack-react": "^2.9.0",
"@mdx-js/react": "^2.3.0",
"@react-hook/media-query": "^1.1.1",
Expand Down
2 changes: 1 addition & 1 deletion src/components/Article/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, { FunctionComponent as FC } from 'react';
import { ArticleFooter } from './ArticleFooter';

const Article: FC<{ children: React.ReactNode }> = ({ children }) => (
<article className="flex-1 overflow-x-hidden">
<article className="flex-1 overflow-x-hidden relative z-10">
{children}
<ArticleFooter />
</article>
Expand Down
25 changes: 25 additions & 0 deletions src/components/Layout/Breadcrumbs.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,29 @@ describe('Breadcrumbs', () => {
expect(screen.getByText('Current Page')).toHaveClass('text-gui-unavailable');
expect(screen.getByText('Current Page')).toHaveClass('pointer-events-none');
});

it('removes duplicate links from breadcrumb nodes', () => {
mockUseLayoutContext.mockReturnValue({
activePage: {
tree: [
{ page: { name: 'Section 1', link: '/section-1' } },
{ page: { name: 'Duplicate Section', link: '/section-1' } },
{ page: { name: 'Current Page', link: '/section-1/page-1' } },
],
},
});

render(<Breadcrumbs />);

// Should only show one instance of the duplicate link
const section1Links = screen.getAllByText('Section 1');
expect(section1Links).toHaveLength(1);

// Should not render the duplicate with different text
expect(screen.queryByText('Duplicate Section')).not.toBeInTheDocument();

// Should still render other breadcrumb elements
expect(screen.getByText('Home')).toBeInTheDocument();
expect(screen.getByText('Current Page')).toBeInTheDocument();
});
});
17 changes: 12 additions & 5 deletions src/components/Layout/Breadcrumbs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,22 @@ import { useLayoutContext } from 'src/contexts/layout-context';
import Link from '../Link';
import Icon from '@ably/ui/core/Icon';
import cn from '@ably/ui/core/utils/cn';
import { hierarchicalKey } from './utils/nav';
import { hierarchicalKey, PageTreeNode } from './utils/nav';

const Breadcrumbs: React.FC = () => {
const { activePage } = useLayoutContext();

const breadcrumbNodes = useMemo(
() => activePage?.tree.filter((node) => node.page.link !== '#') ?? [],
[activePage.tree],
);
const breadcrumbNodes = useMemo(() => {
const filteredNodes = activePage?.tree.filter((node) => node.page.link !== '#') ?? [];
const uniqueNodes = filteredNodes.reduce((acc: PageTreeNode[], current) => {
const isDuplicate = acc.some((item) => item.page.link === current.page.link);
if (!isDuplicate) {
acc.push(current);
}
return acc;
}, []);
return uniqueNodes;
}, [activePage?.tree]);

if (breadcrumbNodes.length === 0) {
return null;
Expand Down
14 changes: 13 additions & 1 deletion src/components/Layout/Header.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ jest.mock('./LeftSidebar', () => ({
default: jest.fn(() => <div>LeftSidebar</div>),
}));

jest.mock('@reach/router', () => ({
useLocation: jest.fn(),
}));

jest.mock('./LanguageSelector', () => ({
LanguageSelector: jest.fn(() => <div>LanguageSelector</div>),
}));

describe('Header', () => {
beforeEach(() => {
document.body.innerHTML = '';
Expand Down Expand Up @@ -92,7 +100,11 @@ describe('Header', () => {
value={{
sessionState: {
signedIn: true,
account: { links: { dashboard: { href: '/dashboard', text: 'Dashboard' } } },
account: {
id: 'test-id',
name: 'Test Account',
links: { dashboard: { href: '/dashboard', text: 'Dashboard' } },
},
},
apps: [],
}}
Expand Down
32 changes: 5 additions & 27 deletions src/components/Layout/Header.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, { useState, useEffect, useContext } from 'react';
import React, { useContext } from 'react';
import { useLocation } from '@reach/router';
import Icon from '@ably/ui/core/Icon';
import AblyHeader from '@ably/ui/core/Header';
import { SearchBar } from '../SearchBar';
Expand All @@ -10,7 +11,7 @@ type HeaderProps = {
};

const Header: React.FC<HeaderProps> = ({ searchBar = true }) => {
const [showMenu, setShowMenu] = useState(false);
const location = useLocation();
const userContext = useContext(UserContext);
const sessionState = {
...userContext.sessionState,
Expand All @@ -33,30 +34,6 @@ const Header: React.FC<HeaderProps> = ({ searchBar = true }) => {
// }
// };

useEffect(() => {
Copy link
Member Author

Choose a reason for hiding this comment

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

This now lives in ably-ui and was left behind after the back-merge

const handleResize = () => {
if (window.innerWidth >= 1040) {
setShowMenu(false);
}
};

window.addEventListener('resize', handleResize);
return () => window.removeEventListener('resize', handleResize);
}, []);

useEffect(() => {
if (showMenu) {
document.body.classList.add('overflow-hidden');
} else {
document.body.classList.remove('overflow-hidden');
}

// Cleanup on unmount
return () => {
document.body.classList.remove('overflow-hidden');
};
}, [showMenu]);

return (
<AblyHeader
// TODO: reenable when examples are ready to be released
Expand Down Expand Up @@ -86,7 +63,7 @@ const Header: React.FC<HeaderProps> = ({ searchBar = true }) => {
mobileNav={<LeftSidebar inHeader key="nav-mobile-documentation-tab" />}
searchButton={
<button
className="cursor-pointer focus-base rounded"
className="cursor-pointer focus-base rounded px-0 pt-4 text-neutral-1300 dark:text-neutral-000"
Copy link
Member Author

Choose a reason for hiding this comment

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

🍎 overrides button colouring with its own blue, how thoughtful

aria-label="Toggle search"
onClick={() => {
const searchContainer = document.querySelector('#inkeep-search > div');
Expand Down Expand Up @@ -125,6 +102,7 @@ const Header: React.FC<HeaderProps> = ({ searchBar = true }) => {
]}
sessionState={sessionState}
logoHref="/docs"
location={location}
Copy link
Member Author

Choose a reason for hiding this comment

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

Piping this in instructs the header menu to close when it changes.

/>
);
};
Expand Down
52 changes: 32 additions & 20 deletions src/components/Layout/LanguageSelector.tsx
Original file line number Diff line number Diff line change
@@ -1,22 +1,17 @@
import { useEffect, useMemo, useRef, useState } from 'react';
import { useEffect, useMemo, useRef, useState, MouseEvent, TouchEvent } from 'react';
import { useLocation } from '@reach/router';
import Select from 'react-select';
import Badge from '@ably/ui/core/Badge';
import Icon from '@ably/ui/core/Icon';
import { IconName } from '@ably/ui/core/Icon/types';
import cn from '@ably/ui/core/utils/cn';
import { componentMaxHeight, HEADER_BOTTOM_MARGIN, HEADER_HEIGHT } from '@ably/ui/core/utils/heights';
import { languageData, languageInfo } from 'src/data/languages';
import { LanguageKey } from 'src/data/languages/types';
import { useOnClickOutside } from 'src/hooks';
import { useLayoutContext } from 'src/contexts/layout-context';
import { navigate } from '../Link';
import {
componentMaxHeight,
HEADER_HEIGHT,
HEADER_BOTTOM_MARGIN,
LANGUAGE_SELECTOR_HEIGHT,
INKEEP_ASK_BUTTON_HEIGHT,
} from './utils/heights';
import { LANGUAGE_SELECTOR_HEIGHT, INKEEP_ASK_BUTTON_HEIGHT } from './utils/heights';

type LanguageSelectorOptionData = {
label: LanguageKey;
Expand All @@ -38,6 +33,16 @@ const LanguageSelectorOption = ({ isOption, setMenuOpen, langParam, ...props }:
const lang = languageInfo[props.data.label];
const location = useLocation();

const handleClick = (e: MouseEvent<HTMLDivElement> | TouchEvent<HTMLDivElement>) => {
e.preventDefault();

if (isOption) {
navigate(`${location.pathname}?lang=${props.data.label}`);
}

setMenuOpen(!props.selectProps.menuIsOpen);
};

return (
<div
className={cn(
Expand All @@ -46,13 +51,8 @@ const LanguageSelectorOption = ({ isOption, setMenuOpen, langParam, ...props }:
'p-8 hover:bg-neutral-100 dark:hover:bg-neutral-1200 cursor-pointer': isOption,
},
)}
onClick={() => {
if (isOption) {
navigate(`${location.pathname}?lang=${props.data.label}`);
}

setMenuOpen(!props.selectProps.menuIsOpen);
}}
onClick={handleClick}
onTouchEnd={handleClick}
role="menuitem"
>
<div className={cn('flex items-center gap-8', { 'flex-1': isOption })}>
Expand Down Expand Up @@ -111,8 +111,16 @@ export const LanguageSelector = () => {
setSelectedOption(defaultOption);
}, [langParam, options]);

const handleClick = (e: MouseEvent<HTMLButtonElement> | TouchEvent<HTMLButtonElement>) => {
e.preventDefault();
setMenuOpen(!menuOpen);
};

return (
<div ref={selectRef} className="absolute top-0 right-0 md:relative w-full text-right md:text-left mb-24 focus-base">
<div
ref={selectRef}
className="md:relative w-full text-right md:text-left mb-24 focus-base -mt-4 md:mt-0 -mr-4 md:mr-0"
>
<Select
options={options}
value={selectedOption}
Expand All @@ -134,10 +142,12 @@ export const LanguageSelector = () => {
),
SingleValue: (props) => <LanguageSelectorOption {...props} setMenuOpen={setMenuOpen} langParam={langParam} />,
IndicatorSeparator: null,
DropdownIndicator: (props) => (
DropdownIndicator: () => (
<button
className="flex items-center pl-8 text-red-orange"
onClick={() => setMenuOpen(!props.selectProps.menuIsOpen)}
role="button"
className="flex items-center pl-8 text-red-orange cursor-pointer"
onClick={handleClick}
onTouchEnd={handleClick}
aria-label="Toggle language dropdown"
>
<Icon
Expand All @@ -161,7 +171,9 @@ export const LanguageSelector = () => {
}}
role="menu"
>
<p className="ui-text-overline2 py-16 px-8 text-neutral-700 dark:text-neutral-600">Code Language</p>
<p className="ui-text-overline2 text-left py-16 px-8 text-neutral-700 dark:text-neutral-600">
Code Language
</p>
{children}
</div>
),
Expand Down
2 changes: 1 addition & 1 deletion src/components/Layout/Layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const Layout: React.FC<LayoutProps> = ({ children, pageContext }) => {
return (
<GlobalLoading template={template}>
<Header searchBar={searchBar} />
<div className="flex pt-64 gap-80 justify-center ui-standard-container mx-auto">
<div className="flex pt-64 md:gap-48 lg:gap-64 xl:gap-80 justify-center ui-standard-container mx-auto">
{sidebar ? <LeftSidebar /> : null}
<Container as="main" className="flex-1">
{sidebar ? <Breadcrumbs /> : null}
Expand Down
69 changes: 45 additions & 24 deletions src/components/Layout/LeftSidebar.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useMemo, useState, useEffect, useRef } from 'react';
import { useLocation } from '@reach/router';
import { navigate, useLocation } from '@reach/router';
import cn from '@ably/ui/core/utils/cn';
import Accordion from '@ably/ui/core/Accordion';
import Icon from '@ably/ui/core/Icon';
Expand Down Expand Up @@ -130,6 +130,26 @@ const constructProductNavData = (
return {
name: product.name,
icon: activePageTree[0]?.page.name === product.name ? product.icon.open : product.icon.closed,
onClick: () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

When a top-level accordion item is clicked, then it tries to bring the header row into view if it isn't visible - this happens after the opening animation has concluded, to ensure that the scrollTo has the right point of reference.

// When a product is clicked, find and scroll to any open accordion element
if (typeof document !== 'undefined') {
// Use setTimeout to ensure the DOM has updated after the click and animation has completed
setTimeout(() => {
const targetAccordion = window.innerWidth >= 1040 ? 'left-nav' : 'mobile-nav';
const menuContainer = document.getElementById(targetAccordion);
const openAccordion: HTMLElement | null = menuContainer
? menuContainer.querySelector('[data-state="open"] > button')
: null;

if (openAccordion) {
menuContainer?.scrollTo({
top: openAccordion.offsetTop,
behavior: 'smooth',
});
}
}, 200);
}
},
content: (
<div key={product.name} className="flex flex-col gap-20 px-16 pt-12">
{product.showJumpLink ? (
Expand Down Expand Up @@ -163,6 +183,18 @@ const constructProductNavData = (
};
});

// Add a Home entry at the start of navData if inHeader is true
if (inHeader) {
navData.unshift({
name: 'Home',
content: null,
onClick: () => {
navigate('/docs');
},
interactive: false,
});
}

return navData;
};

Expand Down Expand Up @@ -192,29 +224,18 @@ const LeftSidebar = ({ inHeader = false }: LeftSidebarProps) => {
);

return (
<>
{inHeader ? (
<a
href="/docs"
aria-label="Home"
className="flex w-full items-center focus-base text-neutral-1000 dark:text-neutral-300 hover:text-neutral-1100 active:text-neutral-1000 transition-colors h-40 ui-text-menu1 font-bold px-16 mt-16"
>
Home
</a>
) : null}
<Accordion
ref={sidebarRef}
className={cn(
!inHeader && [sidebarAlignmentClasses, 'hidden md:block md:-mx-16'],
'overflow-y-auto',
hasScrollbar ? 'md:pr-8' : 'md:pr-16',
)}
style={sidebarAlignmentStyles}
id="left-nav"
data={productNavData}
{...commonAccordionOptions(null, activePage.tree[0]?.index, true, inHeader)}
/>
</>
<Accordion
ref={sidebarRef}
className={cn(
!inHeader && [sidebarAlignmentClasses, 'hidden md:block md:-mx-16'],
'overflow-y-auto',
hasScrollbar ? 'md:pr-8' : 'md:pr-16',
)}
style={sidebarAlignmentStyles}
id={inHeader ? 'mobile-nav' : 'left-nav'}
data={productNavData}
{...commonAccordionOptions(null, activePage.tree[0]?.index, true, inHeader)}
/>
);
};

Expand Down
3 changes: 2 additions & 1 deletion src/components/Layout/RightSidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ import { useLocation, WindowLocation } from '@reach/router';
import cn from '@ably/ui/core/utils/cn';
import Icon from '@ably/ui/core/Icon';
import { IconName } from '@ably/ui/core/Icon/types';
import { componentMaxHeight, HEADER_HEIGHT, HEADER_BOTTOM_MARGIN } from '@ably/ui/core/utils/heights';

import { LanguageSelector } from './LanguageSelector';
import { useLayoutContext } from 'src/contexts/layout-context';
import { languageInfo } from 'src/data/languages';
import { PageTreeNode, sidebarAlignmentClasses, sidebarAlignmentStyles } from './utils/nav';
import { componentMaxHeight, HEADER_HEIGHT, HEADER_BOTTOM_MARGIN, INKEEP_ASK_BUTTON_HEIGHT } from './utils/heights';
import { INKEEP_ASK_BUTTON_HEIGHT } from './utils/heights';

type SidebarHeader = {
id: string;
Expand Down
Loading