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] Make sidebar an aside element #550

Merged
merged 6 commits into from
May 8, 2023
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
34 changes: 29 additions & 5 deletions web/src/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,19 @@ import { useInstallerClient } from "~/context/installer";
import { STARTUP, INSTALL } from "~/client/phase";
import { BUSY } from "~/client/status";

import {
About,
Disclosure,
Installation,
IssuesLink,
LoadingEnvironment,
LogsButton,
ShowLogButton,
ShowTerminalButton,
Sidebar
} from "~/components/core";
import { ChangeProductLink } from "~/components/software";
import { Layout, Title, DBusError } from "~/components/layout";
import { Installation, LoadingEnvironment } from "~/components/core";

function App() {
const client = useInstallerClient();
Expand Down Expand Up @@ -71,10 +82,23 @@ function App() {
};

return (
<Layout>
<Title>Agama</Title>
<Content />
</Layout>
<>
<Sidebar>
<ChangeProductLink />
<IssuesLink />
<Disclosure label="Diagnostic tools" data-keep-sidebar-open>
<ShowLogButton />
<LogsButton data-keep-sidebar-open="true" />
<ShowTerminalButton />
</Disclosure>
<About />
</Sidebar>

<Layout>
<Title>Agama</Title>
<Content />
</Layout>
</>
);
}

Expand Down
1 change: 1 addition & 0 deletions web/src/App.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ jest.mock("~/components/layout/DBusError", () => mockComponent("D-BusError Mock"
jest.mock("~/components/core/LoadingEnvironment", () => mockComponent("LoadingEnvironment Mock"));
jest.mock("~/components/questions/Questions", () => mockComponent("Questions Mock"));
jest.mock("~/components/core/Installation", () => mockComponent("Installation Mock"));
jest.mock("~/components/core/Sidebar", () => mockComponent("Sidebar Mock"));

// this object holds the mocked callbacks
const callbacks = {};
Expand Down
10 changes: 9 additions & 1 deletion web/src/assets/styles/app.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
#root {
height: 100%;
position: relative;
/** block-size fallbacks **/
height: 100vh;
height: 100dvb;
block-size: 100vh;
/** END of block-size fallbacks **/
block-size: 100dvb;
max-inline-size: var(--ui-max-inline-size);
margin-inline: auto;
}

// Make proposal actions compact
Expand Down
6 changes: 1 addition & 5 deletions web/src/assets/styles/blocks.scss
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ section > .content {
right: 0;
z-index: 1;
inline-size: 70%;
box-shadow: 0 0 20px 10px var(--color-primary);
box-shadow: -10px 10px 20px 0 var(--color-primary);
}

.sidebar header {
Expand All @@ -95,10 +95,6 @@ section > .content {
border-top: 1px solid var(--color-gray);
}

.sidebar > div {
margin-inline-start: var(--pf-global--spacer--md);
}

.sidebar a, .sidebar button {
font-size: 16px;
font-weight: var(--fw-bold);
Expand Down
3 changes: 1 addition & 2 deletions web/src/assets/styles/layout.scss
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@
block-size: 100vh;
/** END of block-size fallbacks **/
block-size: 100dvb;
max-inline-size: 1024px;
margin-inline: auto;

background: var(--wrapper-background);

svg {
Expand Down
2 changes: 2 additions & 0 deletions web/src/assets/styles/variables.scss
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
--lh-medium: 1.6;
--lh-large: 1.7;

--ui-max-inline-size: 1024px;

--spacer-small: 0.5rem;
--spacer-normal: 1rem;
--spacer-medium: 1.5rem;
Expand Down
12 changes: 6 additions & 6 deletions web/src/components/core/Sidebar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ 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}
>
<If
Expand All @@ -104,10 +104,10 @@ export default function Sidebar ({ children }) {
</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 @@ -133,7 +133,7 @@ export default function Sidebar ({ children }) {
</a>
{ targetInfo }
</footer>
</nav>
</aside>
</>
);
}
46 changes: 25 additions & 21 deletions web/src/components/core/Sidebar.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,15 @@

import React from "react";
import { screen, within } from "@testing-library/react";
import { installerRender, mockLayout, withNotificationProvider } from "~/test-utils";
import { installerRender, mockLayout, mockComponent, withNotificationProvider } from "~/test-utils";
import { Sidebar } from "~/components/core";
import { createClient } from "~/client";

// Mock layout
jest.mock("~/components/layout/Layout", () => mockLayout());
// Mock some components using contexts and not relevant for below tests
jest.mock("~/components/core/LogsButton", () => mockComponent("LogsButton Mock"));
jest.mock("~/components/software/ChangeProductLink", () => mockComponent("ChangeProductLink Mock"));

let hasIssues = false;

Expand All @@ -45,19 +49,19 @@ beforeEach(() => {
it("renders the sidebar initially hidden", async () => {
installerRender(withNotificationProvider(<Sidebar />));

const nav = await screen.findByRole("navigation", { name: /options/i });
const nav = await screen.findByRole("complementary", { name: /options/i });
expect(nav).toHaveAttribute("data-state", "hidden");
});

it("renders a link for displaying the sidebar", async () => {
const { user } = installerRender(withNotificationProvider(<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 @@ -66,12 +70,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 Down Expand Up @@ -100,36 +104,36 @@ 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");
});
});

Expand Down
24 changes: 1 addition & 23 deletions web/src/components/layout/Layout.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,6 @@ import React from "react";

import logoUrl from "~/assets/suse-horizontal-logo.svg";
import { createTeleporter } from "react-teleporter";
import { ChangeProductLink } from "~/components/software";
import {
About,
Disclosure,
IssuesLink,
LogsButton,
Sidebar,
ShowLogButton,
ShowTerminalButton
} from "~/components/core";

const PageTitle = createTeleporter();
const PageActions = createTeleporter();
Expand Down Expand Up @@ -89,20 +79,8 @@ function Layout({ children }) {
<PageActions.Target as="span" />
<HeaderActions.Target as="span" />
</div>
</header>

<Sidebar>
<>
<ChangeProductLink />
<IssuesLink />
<Disclosure label="Diagnostic tools" data-keep-sidebar-open>
<ShowLogButton />
<LogsButton data-keep-sidebar-open="true" />
<ShowTerminalButton />
</Disclosure>
<About />
</>
</Sidebar>
</header>

<main className="stack">
{children}
Expand Down