Skip to content

Commit fe2c2bb

Browse files
committed
fix: keep shared navigation links canonical
Remove configurable `baseHref` so shared chrome cannot route canonical destinations into deployment subpaths.
1 parent 3d7de48 commit fe2c2bb

6 files changed

Lines changed: 18 additions & 63 deletions

File tree

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@openwdl/ui",
3-
"version": "1.4.0",
3+
"version": "1.5.0",
44
"description": "OpenWDL shared React design system and component library.",
55
"license": "BSD-3-Clause",
66
"type": "module",

src/components/Footer.test.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ describe("Footer", () => {
7070
expect(labelsIn("Contact")).toEqual(["Slack"]);
7171
});
7272

73-
it("puts the brand site under Explore and marks only that Explore link as external", () => {
73+
it("puts the Brand site under Explore without an external arrow", () => {
7474
render(<Footer logo={<span>OpenWDL</span>} />);
7575
const nav = screen.getByRole("navigation", { name: "Footer navigation" });
7676

@@ -83,10 +83,11 @@ describe("Footer", () => {
8383
"Blog",
8484
"Brand",
8585
]);
86-
expect(exploreLinks.slice(0, 3).every((link) => link.querySelector("svg") === null)).toBe(true);
86+
expect(exploreLinks.every((link) => link.querySelector("svg") === null)).toBe(true);
8787
expect(within(explore).getByRole("link", { name: "Brand" }))
8888
.toHaveAttribute("href", "https://brand.openwdl.org");
89-
expect(within(explore).getByRole("link", { name: "Brand" }).querySelector("svg"))
89+
const repositories = lists[1];
90+
expect(within(repositories).getByRole("link", { name: "Specification" }).querySelector("svg"))
9091
.toBeInTheDocument();
9192

9293
for (const list of within(nav).getAllByRole("list")) {

src/components/Footer.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ const COLUMNS: FooterColumn[] = [
180180
{ label: "Home", href: "https://openwdl.org" },
181181
{ label: "Documentation", href: "https://openwdl.org/docs/" },
182182
{ label: "Blog", href: "https://openwdl.org/blog/" },
183-
{ label: "Brand", href: "https://brand.openwdl.org", external: true },
183+
{ label: "Brand", href: "https://brand.openwdl.org" },
184184
],
185185
},
186186
{

src/components/NavBar.test.tsx

Lines changed: 2 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ describe("NavBar", () => {
3636

3737
it("renders every canonical OpenWDL destination", () => {
3838
render(<NavBar logo={<span>OpenWDL</span>} />);
39+
expect(screen.getByRole("link", { name: "OpenWDL home" }))
40+
.toHaveAttribute("href", "https://openwdl.org/");
3941
const navigation = screen.getByRole("navigation", { name: "Primary navigation" });
4042
const destinationLabels = within(navigation)
4143
.getAllByRole("link")
@@ -65,41 +67,6 @@ describe("NavBar", () => {
6567
.toHaveAttribute("href", "https://openwdl.org/docs/start/your-first-workflow/");
6668
});
6769

68-
it("keeps every primary destination internal to the deployment base", () => {
69-
render(<NavBar logo={<span>OpenWDL</span>} baseHref="/brand/" />);
70-
const navigation = screen.getByRole("navigation", { name: "Primary navigation" });
71-
72-
// No primary destination points off-site any more; the only external link
73-
// left in the nav is the GitHub utility.
74-
const offBase = within(navigation)
75-
.getAllByRole("link")
76-
.filter((link) => !/^\/brand\//.test(link.getAttribute("href") ?? ""))
77-
.map((link) => link.getAttribute("href"));
78-
expect(offBase).toEqual(["https://github.com/openwdl"]);
79-
});
80-
81-
it("keeps internal destinations within the deployment base", () => {
82-
render(<NavBar logo={<span>OpenWDL</span>} baseHref="/brand/" />);
83-
84-
expect(screen.getByRole("link", { name: "OpenWDL home" }))
85-
.toHaveAttribute("href", "/brand/");
86-
expect(screen.queryByRole("link", { name: "About" }))
87-
.not.toBeInTheDocument();
88-
expect(screen.queryByRole("link", { name: "Community" }))
89-
.not.toBeInTheDocument();
90-
expect(screen.getByRole("link", { name: "Documentation" }))
91-
.toHaveAttribute("href", "/brand/docs/");
92-
expect(screen.getByRole("link", { name: "Blog" }))
93-
.toHaveAttribute("href", "/brand/blog/");
94-
expect(screen.queryByRole("link", { name: "Modules" }))
95-
.not.toBeInTheDocument();
96-
expect(screen.queryByRole("link", { name: "Brand" }))
97-
.not.toBeInTheDocument();
98-
expect(screen.getByRole("link", { name: "Get started" }))
99-
.toHaveAttribute("href", "/brand/docs/start/your-first-workflow/");
100-
expect(screen.queryByRole("link", { name: "Spec" })).not.toBeInTheDocument();
101-
});
102-
10370
it("marks the active canonical destination", () => {
10471
render(<NavBar logo={<span>OpenWDL</span>} active="docs" />);
10572
expect(screen.getByRole("link", { name: "Documentation" })).toHaveAttribute(
@@ -119,13 +86,6 @@ describe("NavBar", () => {
11986
expect(current).toEqual(["Blog"]);
12087
});
12188

122-
123-
it("labels the logo link as the OpenWDL home, not a generic brand", () => {
124-
render(<NavBar logo={<span>OpenWDL</span>} />);
125-
expect(screen.getByRole("link", { name: "OpenWDL home" }))
126-
.toHaveAttribute("href", "https://openwdl.org/");
127-
});
128-
12989
it("opens the mobile menu and closes it after selecting a link", () => {
13090
render(<NavBar logo={<span>OpenWDL</span>} />);
13191
const button = screen.getByRole("button", { name: "Open navigation" });

src/components/NavBar.tsx

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,27 +24,23 @@ export interface NavBarProps extends HTMLAttributes<HTMLElement> {
2424
logo?: ReactNode;
2525
/** Current canonical destination. */
2626
active?: NavBarActive;
27-
/** Base URL for canonical destinations (default `"https://openwdl.org/"`). */
28-
baseHref?: string;
2927
/** Stick to the top with a frosted backdrop (default `true`). */
3028
sticky?: boolean;
3129
}
3230

3331
const DESTINATIONS = [
34-
{ key: "docs", label: "Documentation", path: "docs/" },
35-
{ key: "blog", label: "Blog", path: "blog/" },
32+
{ key: "docs", label: "Documentation", href: "https://openwdl.org/docs/" },
33+
{ key: "blog", label: "Blog", href: "https://openwdl.org/blog/" },
3634
] as const;
3735

36+
const HOME_HREF = "https://openwdl.org/";
37+
3838
/**
3939
* The primary call to action resolves into the documentation quickstart rather
4040
* than a separate landing page: the shared goal across OpenWDL sites is getting
4141
* a newcomer to a running workflow, and that tutorial is where it happens.
4242
*/
43-
const GET_STARTED_PATH = "docs/start/your-first-workflow/";
44-
45-
function normalizeBaseHref(baseHref: string) {
46-
return `${baseHref.replace(/\/+$/, "")}/`;
47-
}
43+
const GET_STARTED_HREF = "https://openwdl.org/docs/start/your-first-workflow/";
4844

4945
function assignRef<T>(ref: ForwardedRef<T>, value: T | null) {
5046
if (typeof ref === "function") {
@@ -62,15 +58,13 @@ export const NavBar = forwardRef<HTMLElement, NavBarProps>(function NavBar(
6258
{
6359
logo = <OpenWdlLogo height={24} />,
6460
active,
65-
baseHref = "https://openwdl.org/",
6661
sticky = true,
6762
className,
6863
onBlur,
6964
...props
7065
},
7166
forwardedRef,
7267
) {
73-
const base = normalizeBaseHref(baseHref);
7468
const [menuOpen, setMenuOpen] = useState(false);
7569
const navigationId = useId();
7670
const navigationRef = useRef<HTMLElement>(null);
@@ -139,7 +133,7 @@ export const NavBar = forwardRef<HTMLElement, NavBarProps>(function NavBar(
139133
>
140134
<div className={styles.inner}>
141135
{logo && (
142-
<a href={base} className={styles.brand} aria-label="OpenWDL home">
136+
<a href={HOME_HREF} className={styles.brand} aria-label="OpenWDL home">
143137
{logo}
144138
</a>
145139
)}
@@ -155,7 +149,7 @@ export const NavBar = forwardRef<HTMLElement, NavBarProps>(function NavBar(
155149
{DESTINATIONS.map((destination) => (
156150
<a
157151
key={destination.key}
158-
href={`${base}${destination.path}`}
152+
href={destination.href}
159153
className={styles.navLink}
160154
aria-current={destination.key === active ? "page" : undefined}
161155
>
@@ -171,7 +165,7 @@ export const NavBar = forwardRef<HTMLElement, NavBarProps>(function NavBar(
171165
</span>
172166
</nav>
173167

174-
<a href={`${base}${GET_STARTED_PATH}`} className={styles.action}>
168+
<a href={GET_STARTED_HREF} className={styles.action}>
175169
<span>Get started</span>
176170
<span className={styles.actionIcon} aria-hidden="true">
177171
<FiArrowRight />

0 commit comments

Comments
 (0)