Skip to content

Commit

Permalink
SWED-2230 tweaks on Topbar v2 PR code-review
Browse files Browse the repository at this point in the history
  • Loading branch information
goldenraphti committed Oct 11, 2023
1 parent 72f96c0 commit dfc45e3
Show file tree
Hide file tree
Showing 16 changed files with 1,122 additions and 1,135 deletions.
10 changes: 3 additions & 7 deletions src/App/AppHeader/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,11 @@ import routes from "../routes/all";

import SearchBox from "../utils/SearchBox";

import swedbankpayLogo from "@src/img/swedbankpay/logo/swedbankpay-logo-h.svg";
import payexLogo from "@src/img/payex/logo/payex-logo.svg";

const brand = process.env.brand;
const devLogo = brand === "swedbankpay" ? swedbankpayLogo : payexLogo;
const isDev = process.env.version === "LOCAL_DEV";

const basename = process.env.basename;
const brand = process.env.brand;

// feature toggle. Switch once we switch default to topbar v2 (and update rest of docs and code)
// other corresponding feature toggle for topbar at "src/App/utils/SelectPanel/index.js"
const useTopbarLegacy = true;

// mobile & tablet topbar and hamburger menu
Expand Down
24 changes: 4 additions & 20 deletions src/App/ComponentsDocumentation/components/Topbar/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ export const topbarShowcase = {
title: "Desktop",
description: (
<p>
{/* TODO: mention it's legacy topbar and it all involve to use this in terms of breaking changes */}
The minimum requirement in a Topbar is to include the Swedbank Pay
Logotype, common additional functionality is a list with navigation
links. On desktop use{" "}
Expand All @@ -133,7 +132,6 @@ export const topbarShowcase = {
description: (
<>
<p>
{/* TODO: mention it's legacy topbar and it all involve to use this in terms of breaking changes */}
The minimum requirement in a Topbar is to include the Swedbank Pay
Logotype, common additional functionality is a list with navigation
links. On smaller screens use a menu button to toggle a vertical
Expand All @@ -155,11 +153,8 @@ export const topbarShowcase = {
title: "Desktop",
description: (
<p>
The minimum requirement in a Topbar is to include the Swedbank Pay
Logotype, common additional functionality is a list with navigation
links. On desktop use{" "}
<CodeTags type="secondary" code=".topbar-xl-wide" /> to show the links
listed horizontally in the topbar.{" "}
The experimental desktop topbar involves breaking changes. Make sure
to check them out before switching.
</p>
),
},
Expand All @@ -169,19 +164,8 @@ export const topbarShowcase = {
title: "Mobile/tablet",
description: (
<>
<p>
The minimum requirement in a Topbar is to include the Swedbank Pay
Logotype, common additional functionality is a list with navigation
links. On smaller screens use a menu button to toggle a vertical
navigation drawer with links when the menu button is clicked.
</p>
<p>
Be aware; The <CodeTags code="max-height" type="primary" />{" "}
attribute for element <CodeTags type="secondary" code=".nav-ul" />{" "}
when it is active is set to 2500px. This is to ensure animation for
the transition to happen. You can easily alter this by creating
custom css.
</p>
The experimental desktop topbar involves breaking changes. Make sure
to check them out before switching.
</>
),
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ test("Topbar page exist", async ({ page }) => {
name: "Components Find all currently available components here",
})
.click();
await expect(page.getByRole("link", { name: "Topbar" })).toHaveCount(
page.viewportSize().width < 991 ? 1 : 2
);
await expect(
page.getByRole("link", { name: "Topbar", exact: true })
).toHaveCount(page.viewportSize().width < 991 ? 1 : 2);
await page.getByText("call_to_actionTopbararrow_forward").click();
await expect(page).toHaveTitle(/Topbar/);
await expect(
Expand Down
Binary file not shown.
Binary file not shown.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React, { useEffect } from "react";
import { Link } from "react-router-dom";
import React from "react";

import { EditableComponentPreview, DocContainer } from "@docutils";
import CodeTags from "@components/CodeTags";
Expand Down Expand Up @@ -80,7 +79,8 @@ export const Migration = () => (
{"\n"}
<div class="nav-container">
{"\n"}
{/* <!-- open/close menu button --> */}
{"// open/close menu button "}
{"\n"}
<button
type="button"
class="topbar-btn"
Expand All @@ -104,8 +104,8 @@ export const Migration = () => (
{"\n"}
</button>
{"\n"}

{/* <!-- logo link to homepage --> */}
{"// logo link to homepage "}
{"\n"}
<a class="topbar-logo" href="/" aria-label="To homepage">
{"\n"}
<img
Expand All @@ -116,13 +116,14 @@ export const Migration = () => (
{"\n"}
</a>
{"\n"}

{/* <!-- topbar/modal content --> */}
{"// topbar/modal content "}
{"\n"}
<nav class="topbar-nav">
{"\n"}
<div class="topbar-link-container">
{"\n"}
{/* <!-- content --> */}
{"// content "}
{"\n"}
</div>
{"\n"}
</nav>
Expand All @@ -142,7 +143,8 @@ export const Migration = () => (
{"\n"}
<div class="nav-container">
{"\n"}
{/* <!-- logo link to homepage --> */}
{"// logo link to homepage "}
{"\n"}
<a class="topbar-logo" href="/" aria-label="To homepage">
{"\n"}
<img
Expand All @@ -154,18 +156,21 @@ export const Migration = () => (
</a>
{"\n"}

{/* <!-- topbar/modal content --> */}
{"// topbar/modal content "}
{"\n"}
<nav class="topbar-nav">
{"\n"}
<div class="topbar-link-container">
{"\n"}
{/* <!-- links content --> */}
{"// links content "}
{"\n"}
</div>
{"\n"}
</nav>
{"\n"}

{/* <!-- open menu button --> */}
{"// open menu button "}
{"\n"}
<button
type="button"
class="topbar-btn"
Expand Down
44 changes: 21 additions & 23 deletions src/App/components/Topbar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,31 +21,29 @@ const getDevLogo = (legacy) => {
}
};

const TopbarBtn = ({ legacy = true }) => {
return (
<>
<button
type="button"
className="topbar-btn"
aria-label="Open menu"
aria-haspopup="menu"
aria-expanded="false"
aria-controls="topbar-nav"
>
{"\n\t\t"}
<i className="material-icons topbar-btn-icon">menu</i>
const TopbarBtn = ({ legacy = true }) => (
<>
<button
type="button"
className="topbar-btn"
aria-label="Open menu"
aria-haspopup="menu"
aria-expanded="false"
aria-controls="topbar-nav"
>
{"\n\t\t"}
<i className="material-icons topbar-btn-icon">menu</i>
{"\n\t\t"}
</button>
{legacy && (
<button type="button" className="topbar-close" aria-label="Close menu">
{"\n"}
<i className="material-icons topbar-btn-icon">close</i>
{"\n\t\t"}
</button>
{legacy && (
<button type="button" className="topbar-close" aria-label="Close menu">
{"\n"}
<i className="material-icons topbar-btn-icon">close</i>
{"\n\t\t"}
</button>
)}
</>
);
};
)}
</>
);

const ConditionalWrapper = ({ condition, wrapper, children }) =>
condition ? wrapper(children) : children;
Expand Down
2 changes: 1 addition & 1 deletion src/App/routes/components.js
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ module.exports = [
componentPath: "components/TopbarMigratingToV2",
icon: "call_to_action",
outlined: true,
statusBadges: ["javascript"],
statusBadges: ["javascript", "new"],
},
],
},
Expand Down
Loading

0 comments on commit dfc45e3

Please sign in to comment.