Skip to content

Commit

Permalink
[web] Render sidebar as aside instead of nav
Browse files Browse the repository at this point in the history
The main navigation is no longer there, as we thought it would be.
Altough there still general or complementary links, we have extracted
almost all navigation links to navigate from one page to another into a
page-specific dropdown menu (see PageOptions component).

If needed, we can add `nav` children to the Sidebar later.
  • Loading branch information
dgdavid committed Apr 27, 2023
1 parent bdfe4e4 commit 5c8483d
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 27 deletions.
12 changes: 6 additions & 6 deletions web/src/components/core/Sidebar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -90,19 +90,19 @@ export default function Sidebar ({ children }) {
<button
onClick={open}
className="plain-control"
aria-label="Show navigation and other options"
aria-controls="navigation-and-options"
aria-label="Show global options"
aria-controls="global-options"
aria-expanded={isOpen}
>
<NotificationMark />
<Icon name="menu" />
</button>
</AppActions>

<nav
id="navigation-and-options"
<aside
id="global-options"
className="wrapper sidebar"
aria-label="Navigation and other options"
aria-label="Global options"
data-state={isOpen ? "visible" : "hidden"}
>
<header className="split justify-between">
Expand All @@ -128,7 +128,7 @@ export default function Sidebar ({ children }) {
</a>
{ targetInfo }
</footer>
</nav>
</aside>
</>
);
}
42 changes: 21 additions & 21 deletions web/src/components/core/Sidebar.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,19 @@ jest.mock("~/components/layout/Layout", () => mockLayout());

it("renders the sidebar initially hidden", async () => {
plainRender(<Sidebar />);
const nav = await screen.findByRole("navigation", { name: /options/i });
expect(nav).toHaveAttribute("data-state", "hidden");
const sidebar = await screen.findByRole("complementary", { name: /options/i });
expect(sidebar).toHaveAttribute("data-state", "hidden");
});

it("renders a link for displaying the sidebar", async () => {
const { user } = plainRender(<Sidebar />);

const link = await screen.findByLabelText(/Show/i);
const nav = await screen.findByRole("navigation", { name: /options/i });
const sidebar = await screen.findByRole("complementary", { name: /options/i });

expect(nav).toHaveAttribute("data-state", "hidden");
expect(sidebar).toHaveAttribute("data-state", "hidden");
await user.click(link);
expect(nav).toHaveAttribute("data-state", "visible");
expect(sidebar).toHaveAttribute("data-state", "visible");
});

it("renders a link for hiding the sidebar", async () => {
Expand All @@ -49,12 +49,12 @@ it("renders a link for hiding the sidebar", async () => {
const openLink = await screen.findByLabelText(/Show/i);
const closeLink = await screen.findByLabelText(/Hide/i);

const nav = await screen.findByRole("navigation", { name: /options/i });
const sidebar = await screen.findByRole("complementary", { name: /options/i });

await user.click(openLink);
expect(nav).toHaveAttribute("data-state", "visible");
expect(sidebar).toHaveAttribute("data-state", "visible");
await user.click(closeLink);
expect(nav).toHaveAttribute("data-state", "hidden");
expect(sidebar).toHaveAttribute("data-state", "hidden");
});

it("moves the focus to the close action after opening it", async () => {
Expand All @@ -81,35 +81,35 @@ describe("onClick bubbling", () => {

const openLink = screen.getByLabelText(/Show/i);
await user.click(openLink);
const nav = screen.getByRole("navigation", { name: /options/i });
expect(nav).toHaveAttribute("data-state", "visible");
const sidebar = screen.getByRole("complementary", { name: /options/i });
expect(sidebar).toHaveAttribute("data-state", "visible");

// user clicks in the sidebar body
await user.click(nav);
expect(nav).toHaveAttribute("data-state", "visible");
await user.click(sidebar);
expect(sidebar).toHaveAttribute("data-state", "visible");

// user clicks on a button set for keeping the sidebar open
const keepOpenButton = within(nav).getByRole("button", { name: "Keep it open!" });
const keepOpenButton = within(sidebar).getByRole("button", { name: "Keep it open!" });
await user.click(keepOpenButton);
expect(nav).toHaveAttribute("data-state", "visible");
expect(sidebar).toHaveAttribute("data-state", "visible");

// user clicks a button NOT set for keeping the sidebar open
const button = within(nav).getByRole("button", { name: "Do something" });
const button = within(sidebar).getByRole("button", { name: "Do something" });
await user.click(button);
expect(nav).toHaveAttribute("data-state", "hidden");
expect(sidebar).toHaveAttribute("data-state", "hidden");

// open it again
await user.click(openLink);
expect(nav).toHaveAttribute("data-state", "visible");
expect(sidebar).toHaveAttribute("data-state", "visible");

// user clicks on link set for keeping the sidebar open
const keepOpenLink = within(nav).getByRole("link", { name: "Keep it open!" });
const keepOpenLink = within(sidebar).getByRole("link", { name: "Keep it open!" });
await user.click(keepOpenLink);
expect(nav).toHaveAttribute("data-state", "visible");
expect(sidebar).toHaveAttribute("data-state", "visible");

// user clicks on link NOT set for keeping the sidebar open
const link = within(nav).getByRole("link", { name: "Goes somewhere" });
const link = within(sidebar).getByRole("link", { name: "Goes somewhere" });
await user.click(link);
expect(nav).toHaveAttribute("data-state", "hidden");
expect(sidebar).toHaveAttribute("data-state", "hidden");
});
});

0 comments on commit 5c8483d

Please sign in to comment.