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.

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

Fix navbar unintended hamburger #317

Merged
merged 3 commits into from
Sep 25, 2023
Merged

Fix navbar unintended hamburger #317

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3dc2e10
[CCUBE-444][KT] Fix mobile menu item appearing even if no items or ac…
Sep 20, 2023
35c4be0
[CCUBE-444][KT] Add tests for Navbar
Sep 20, 2023
7e93d5b
[CCUBE-444][KT] Handle if there are uncollapsible action buttons
Sep 22, 2023
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
6 changes: 5 additions & 1 deletion src/navbar/navbar-items.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,11 @@ export const NavbarItems = <T,>({
{...options}
>
<LinkLabel>{children}</LinkLabel>
{selected && <LinkIndicator />}
{selected && (
<LinkIndicator
data-testid={`${testId}-indicator`}
/>
)}
{mobile && item.subMenu && (
<LinkIconContainer>
<ExpandCollapseButton
Expand Down
30 changes: 22 additions & 8 deletions src/navbar/navbar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,27 @@ const Component = <T,>(
</NavBrandContainer>
);

const renderMobileMenuButton = () => {
if (
(items.mobile && items.mobile.length > 0) ||
(items.desktop && items.desktop.length > 0) ||
actionButtons
keithtxw marked this conversation as resolved.
Show resolved Hide resolved
) {
return (
<MobileMenuButton
aria-label="Open nav menu"
data-testid="button__mobile-menu"
onClick={handleMobileMenuButtonClick}
focusHighlight={false}
>
<MobileMenuIcon />
</MobileMenuButton>
);
}

return null;
};

const renderNavbar = () => {
return (
<Layout.Content stretch={isStretch}>
Expand All @@ -244,14 +265,7 @@ const Component = <T,>(
actionButtons={actionButtons}
onActionButtonClick={handleActionButtonClick}
/>
<MobileMenuButton
aria-label="Open nav menu"
data-testid="button__mobile-menu"
onClick={handleMobileMenuButtonClick}
focusHighlight={false}
>
<MobileMenuIcon />
</MobileMenuButton>
{renderMobileMenuButton()}
</NavElementsContainer>
)}
</Nav>
Expand Down
147 changes: 147 additions & 0 deletions tests/navbar/navbar.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
import { render } from "@testing-library/react";
import { Navbar } from "src/navbar";

describe("Navbar", () => {
describe("Basic functions", () => {
it("should render the items (desktop and mobile) if specified", () => {
const rendered = render(
<Navbar
items={{
desktop: MOCK_ITEMS,
mobile: MOCK_ITEMS,
}}
/>
);

const { getByTestId } = rendered;

expect(getByTestId("link__1")).toBeInTheDocument();
expect(getByTestId("link__2")).toBeInTheDocument();
expect(getByTestId("link__mobile-1")).toBeInTheDocument();
expect(getByTestId("link__mobile-2")).toBeInTheDocument();
});

it("should render the mobile items even if mobile items are not specified", () => {
const rendered = render(
<Navbar
items={{
desktop: MOCK_ITEMS,
}}
/>
);

const { getByTestId } = rendered;

expect(getByTestId("link__1")).toBeInTheDocument();
expect(getByTestId("link__2")).toBeInTheDocument();
expect(getByTestId("link__mobile-1")).toBeInTheDocument();
expect(getByTestId("link__mobile-2")).toBeInTheDocument();
});

it("should render the indicator on the correct item when clicked", () => {
const rendered = render(
<Navbar
items={{
desktop: MOCK_ITEMS,
}}
selectedId="first"
/>
);

const { getByTestId } = rendered;
expect(getByTestId("link__1-indicator")).toBeInTheDocument();
});

it("should render the primary brand", () => {
const rendered = render(
<Navbar
items={{
desktop: MOCK_ITEMS,
}}
/>
);

const { getByTestId } = rendered;
expect(getByTestId("main__brand")).toBeInTheDocument();
});

it("should render the secondary brand if specified", () => {
const rendered = render(
<Navbar
items={{
desktop: MOCK_ITEMS,
}}
resources={{
secondary: {
brandName: "Another",
logoSrc:
"https://assets.life.gov.sg/lifesg/logo-lifesg.svg",
},
}}
/>
);

const { getByTestId } = rendered;
expect(getByTestId("main__brand-secondary")).toBeInTheDocument();
});

it("should render the mobile menu button if there are items specified", () => {
const rendered = render(
<Navbar
items={{
desktop: MOCK_ITEMS,
}}
/>
);

const { getByTestId } = rendered;
expect(getByTestId("button__mobile-menu")).toBeInTheDocument();
});

it("should not render the links and mobile menu button if there no items are specified", () => {
const rendered = render(
<Navbar
items={{
desktop: [],
}}
/>
);

const { queryByTestId } = rendered;
expect(queryByTestId("link__1")).not.toBeInTheDocument();
expect(
queryByTestId("button__mobile-menu")
).not.toBeInTheDocument();
});

it("should not render the links and mobile menu button if hideNavElements is set to true", () => {
const rendered = render(
<Navbar
items={{
desktop: MOCK_ITEMS,
}}
hideNavElements
/>
);

const { queryByTestId } = rendered;
expect(queryByTestId("link__1")).not.toBeInTheDocument();
expect(
queryByTestId("button__mobile-menu")
).not.toBeInTheDocument();
});
});
});
// =============================================================================
// MOCKS
// =============================================================================
const MOCK_ITEMS = [
{
id: "first",
children: "First",
},
{
id: "second",
children: "Second",
},
];