Skip to content

Fix mobile navigation overflow #1811

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
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
79 changes: 36 additions & 43 deletions components/Sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ export const SidebarLayout = ({ children }: { children: React.ReactNode }) => {
const [rotateChevron, setRotateChevron] = useState(false);
const handleRotate = () => setRotateChevron(!rotateChevron);
const rotate = rotateChevron ? 'rotate(180deg)' : 'rotate(0)';
const pathWtihoutFragment = extractPathWithoutFragment(router.asPath);
const pathWithoutFragment = extractPathWithoutFragment(router.asPath);
useEffect(() => {
if (window) {
window.addEventListener('resize', () => {
Expand All @@ -213,19 +213,19 @@ export const SidebarLayout = ({ children }: { children: React.ReactNode }) => {
setOpen(!open);
}}
>
{getDocsPath.includes(pathWtihoutFragment) && (
{getDocsPath.includes(pathWithoutFragment) && (
<h3 className='text-white ml-12'>Introduction</h3>
)}
{getStartedPath.includes(pathWtihoutFragment) && (
{getStartedPath.includes(pathWithoutFragment) && (
<h3 className='text-white ml-12'>Get started</h3>
)}
{getGuidesPath.includes(pathWtihoutFragment) && (
{getGuidesPath.includes(pathWithoutFragment) && (
<h3 className='text-white ml-12'>Guides</h3>
)}
{getReferencePath.includes(pathWtihoutFragment) && (
{getReferencePath.includes(pathWithoutFragment) && (
<h3 className='text-white ml-12'>Reference</h3>
)}
{getSpecificationPath.includes(pathWtihoutFragment) && (
{getSpecificationPath.includes(pathWithoutFragment) && (
<h3 className='text-white ml-12'>Specification</h3>
)}
{router.pathname === null && (
Expand Down Expand Up @@ -280,6 +280,7 @@ export const SidebarLayout = ({ children }: { children: React.ReactNode }) => {
};

export const DocsNav = ({
open, // eslint-disable-line @typescript-eslint/no-unused-vars
setOpen,
}: {
open: boolean;
Expand All @@ -294,24 +295,23 @@ export const DocsNav = ({
getSpecification: false,
});
useEffect(() => {
const pathWtihoutFragment = extractPathWithoutFragment(router.asPath);
const pathWithoutFragment = extractPathWithoutFragment(router.asPath);
const newActive = {
getDocs: false,
getStarted: false,
getGuides: false,
getReference: false,
getSpecification: false,
};
if (getDocsPath.includes(pathWtihoutFragment)) {
if (getDocsPath.includes(pathWithoutFragment)) {
newActive.getDocs = true;
} else if (getStartedPath.includes(pathWtihoutFragment)) {
} else if (getStartedPath.includes(pathWithoutFragment)) {
newActive.getStarted = true;
setActive({ ...active, getStarted: true });
} else if (getReferencePath.includes(pathWtihoutFragment)) {
} else if (getReferencePath.includes(pathWithoutFragment)) {
newActive.getReference = true;
} else if (getSpecificationPath.includes(pathWtihoutFragment)) {
} else if (getSpecificationPath.includes(pathWithoutFragment)) {
newActive.getSpecification = true;
} else if (getGuidesPath.includes(pathWtihoutFragment)) {
} else if (getGuidesPath.includes(pathWithoutFragment)) {
newActive.getGuides = true;
}
setActive(newActive);
Expand Down Expand Up @@ -339,17 +339,30 @@ export const DocsNav = ({
}
}, [resolvedTheme]);

const handleAccordion = (section: keyof typeof active) => (open: boolean) => {
if (open) {
setActive({
getDocs: false,
getStarted: false,
getGuides: false,
getReference: false,
getSpecification: false,
[section]: true,
});
} else {
setActive((prev) => ({
...prev,
[section]: false,
}));
}
};

return (
<div id='sidebar' className='lg:mt-8 w-4/5 mx-auto lg:ml-4'>
{/* Introduction */}
<Collapsible
open={active.getDocs}
onOpenChange={(open) =>
setActive((prev) => ({
...prev,
getDocs: open,
}))
}
onOpenChange={handleAccordion('getDocs')}
className='my-2 bg-slate-200 dark:bg-slate-900 border-white border lg:border-hidden p-3 rounded transition-all duration-300 group'
>
<CollapsibleTrigger asChild>
Expand Down Expand Up @@ -451,12 +464,7 @@ export const DocsNav = ({
{/* Get Started */}
<Collapsible
open={active.getStarted}
onOpenChange={(open) =>
setActive((prev) => ({
...prev,
getStarted: open,
}))
}
onOpenChange={handleAccordion('getStarted')}
className='mb-2 bg-slate-200 dark:bg-slate-900 p-3 rounded border border-white lg:border-hidden transition-all duration-300 group'
>
<CollapsibleTrigger asChild>
Expand Down Expand Up @@ -555,12 +563,7 @@ export const DocsNav = ({
{/* Guides */}
<Collapsible
open={active.getGuides}
onOpenChange={(open) =>
setActive((prev) => ({
...prev,
getGuides: open,
}))
}
onOpenChange={handleAccordion('getGuides')}
className='mb-2 bg-slate-200 dark:bg-slate-900 p-3 rounded border border-white lg:border-hidden transition-all duration-300 group'
>
<CollapsibleTrigger asChild>
Expand Down Expand Up @@ -628,12 +631,7 @@ export const DocsNav = ({
{/* Reference */}
<Collapsible
open={active.getReference}
onOpenChange={(open) =>
setActive((prev) => ({
...prev,
getReference: open,
}))
}
onOpenChange={handleAccordion('getReference')}
className='mb-2 bg-slate-200 dark:bg-slate-900 p-3 rounded border border-white lg:border-hidden transition-all duration-300 group'
>
<CollapsibleTrigger asChild>
Expand Down Expand Up @@ -811,12 +809,7 @@ export const DocsNav = ({
{/* Specification */}
<Collapsible
open={active.getSpecification}
onOpenChange={(open) =>
setActive((prev) => ({
...prev,
getSpecification: open,
}))
}
onOpenChange={handleAccordion('getSpecification')}
className='mb-2 bg-slate-200 dark:bg-slate-900 p-3 rounded border border-white lg:border-hidden transition-all duration-300 group'
>
<CollapsibleTrigger asChild>
Expand Down
87 changes: 55 additions & 32 deletions cypress/components/Sidebar.cy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -192,14 +192,8 @@ describe('Sidebar Component', () => {
});

it('should render all navigation links correctly', () => {
// Expand all sections to check links
cy.contains('Introduction').parent().click();
cy.contains('Get Started').parent().click();
cy.contains('Guides').parent().click();
cy.contains('Reference').parent().click();
cy.contains('Specification').parent().click();

// Check Introduction links
cy.contains('Introduction').parent().click();
cy.contains('Overview').should('exist');
cy.contains('What is JSON Schema?').should('exist');
cy.contains('Roadmap').should('exist');
Expand All @@ -213,13 +207,20 @@ describe('Sidebar Component', () => {
cy.contains('Code of conduct').should('exist');

// Check Get Started links
cy.contains('Get Started').parent().click();
cy.contains('What is a schema?').should('exist');
cy.contains('The basics').should('exist');
cy.contains('Create your first schema').should('exist');
cy.contains('Tour of JSON Schema').should('exist');
cy.contains('JSON Schema glossary').should('exist');

// Check Guides links
cy.contains('Guides').parent().click();
cy.contains('For implementers').should('exist');
cy.contains('Common interfaces across implementations').should('exist');

// Check Reference links
cy.contains('Reference').parent().click();
cy.contains('JSON Schema keywords').should('exist');
cy.contains('JSON data types').should('exist');
cy.contains('array').should('exist');
Expand All @@ -231,6 +232,7 @@ describe('Sidebar Component', () => {
cy.contains('string').should('exist');

// Check Specification links
cy.contains('Specification').parent().click();
cy.contains('2020-12').should('exist');
cy.contains('2019-09').should('exist');
cy.contains('draft-07').should('exist');
Expand All @@ -239,23 +241,23 @@ describe('Sidebar Component', () => {
});

it('should handle external links correctly', () => {
// Expand sections to access external links
// Expand Introduction to access external links and check attributes and icons
cy.contains('Introduction').parent().click();
cy.contains('Get Started').parent().click();
cy.contains('Reference').parent().click();

// Check external links have correct attributes
cy.contains('Landscape').should('have.attr', 'target', '_blank');
cy.contains('Landscape').find('svg').should('exist');

// Expand Get Started to access external links and check attributes and icons
cy.contains('Get Started').parent().click();
cy.contains('Tour of JSON Schema').should(
'have.attr',
'target',
'_blank',
);
cy.contains('Learn JSON Schema').should('have.attr', 'target', '_blank');

// Check external link icons
cy.contains('Landscape').find('svg').should('exist');
cy.contains('Tour of JSON Schema').find('svg').should('exist');

// Expand Reference to access external links and check attributes and icons
cy.contains('Reference').parent().click();
cy.contains('Learn JSON Schema').should('have.attr', 'target', '_blank');
cy.contains('Learn JSON Schema').find('svg').should('exist');
});

Expand Down Expand Up @@ -300,19 +302,27 @@ describe('Sidebar Component', () => {

it('should call onClick and setOpen when DocLinkBlank is clicked', () => {
const mockSetOpen = cy.stub().as('mockSetOpen');

// Mount DocsNav with mocked setOpen
cy.mount(<DocsNav open={false} setOpen={mockSetOpen} />);

// Expand sections to reveal external links
// Test Introduction external link (Landscape)
cy.contains('Introduction').parent().click();
cy.contains('Landscape').trigger('click', { force: true });
cy.get('@mockSetOpen').should('have.been.calledWith', false);

// Reset the stub for next test
mockSetOpen.resetHistory();

// Test Get Started external link (Tour of JSON Schema)
cy.contains('Get Started').parent().click();
cy.contains('Reference').parent().click();
cy.contains('Tour of JSON Schema').trigger('click', { force: true });
cy.get('@mockSetOpen').should('have.been.calledWith', false);

// Trigger the onClick event on the external link without causing navigation
cy.contains('Landscape').trigger('click', { force: true });
// Reset the stub for next test
mockSetOpen.resetHistory();

// Verify setOpen was called with false
// Test Reference external link (Learn JSON Schema)
cy.contains('Reference').parent().click();
cy.contains('Learn JSON Schema').trigger('click', { force: true });
cy.get('@mockSetOpen').should('have.been.calledWith', false);
});

Expand All @@ -338,15 +348,25 @@ describe('Sidebar Component', () => {
// Mount DocsNav with mocked setOpen
cy.mount(<DocsNav open={false} setOpen={mockSetOpen} />);

// Expand sections to reveal external links
// Test Introduction external link (Landscape)
cy.contains('Introduction').parent().click();
cy.contains('Landscape').trigger('click', { force: true });
cy.get('@mockSetOpen').should('have.been.calledWith', false);

// Reset the stub for next test
mockSetOpen.resetHistory();

// Test Get Started external link (Tour of JSON Schema)
cy.contains('Get Started').parent().click();
cy.contains('Reference').parent().click();
cy.contains('Tour of JSON Schema').trigger('click', { force: true });
cy.get('@mockSetOpen').should('have.been.calledWith', false);

// Trigger the onClick event on the external link without causing navigation
cy.contains('Landscape').trigger('click', { force: true });
// Reset the stub for next test
mockSetOpen.resetHistory();

// Verify setOpen was called with false even without custom onClick
// Test Reference external link (Learn JSON Schema)
cy.contains('Reference').parent().click();
cy.contains('Learn JSON Schema').trigger('click', { force: true });
cy.get('@mockSetOpen').should('have.been.calledWith', false);
});

Expand Down Expand Up @@ -470,13 +490,16 @@ describe('Sidebar Component', () => {
});

it('should handle section subtitles correctly', () => {
// Expand sections to see subtitles
// Test Get Started section subtitles
cy.contains('Get Started').parent().click();
cy.contains('Examples').should('have.class', 'italic');

// Test Reference section subtitles (if any exist)
cy.contains('Reference').parent().click();
cy.contains('Specification').parent().click();
// Note: Based on code, Reference section doesn't seem to have subtitles

// Check subtitles are rendered with correct styling
cy.contains('Examples').should('have.class', 'italic');
// Test Specification section subtitles
cy.contains('Specification').parent().click();
cy.contains('Versions').should('have.class', 'italic');
});

Expand Down