Skip to content

Commit 39a6d6c

Browse files
committed
feat(popup): split repo header links and refine repo actions
add a repo-scoped notifications URL builder and use it to separate the repo icon and repo name links in popup headers update repo action hover and focus states to match the new header structure cover the new repo header behavior in renderer and URL builder tests
1 parent 5c9968c commit 39a6d6c

6 files changed

Lines changed: 243 additions & 22 deletions

src/lib/url-builder.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,16 @@ function buildDependabotUrl(fullName) {
138138
return `${GITHUB_SITE_BASE}/${fullName}/security/dependabot`;
139139
}
140140

141+
/**
142+
* Build GitHub notifications URL filtered to a single repository
143+
* @param {string|null|undefined} fullName - Repository full name (owner/repo)
144+
* @returns {string|null} Notifications URL or null if fullName is falsy
145+
*/
146+
export function buildRepoNotificationsUrl(fullName) {
147+
if (!fullName) return null;
148+
return `${GITHUB_SITE_BASE}/notifications?query=${encodeURIComponent(`repo:${fullName}`)}`;
149+
}
150+
141151
/**
142152
* Build GitHub profile URL from a username/login
143153
* @param {string|null|undefined} login - GitHub username

src/popup/notification-renderer.js

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
} from "../lib/constants.js";
1111
import { formatReason, getNotificationStatus } from "../lib/format-utils.js";
1212
import { getIconSVGElement } from "../lib/icons.js";
13-
import { buildProfileUrl } from "../lib/url-builder.js";
13+
import { buildProfileUrl, buildRepoNotificationsUrl } from "../lib/url-builder.js";
1414

1515
/**
1616
* Build icon class with state information
@@ -633,28 +633,45 @@ export function renderNotifications(notifications, shouldResort = true) {
633633
// Render each repository group
634634
for (const repoFullName of sortedRepos) {
635635
const group = groupedByRepo[repoFullName];
636+
const repoNotificationsUrl = buildRepoNotificationsUrl(repoFullName);
637+
const repoHomeUrl = group.repo.html_url || `https://github.com/${repoFullName}`;
636638

637-
const repoHeader = document.createElement("a");
639+
const repoHeader = document.createElement("div");
638640
repoHeader.className = "repo-group-header";
639-
repoHeader.href = group.repo.html_url;
640-
repoHeader.target = "_blank";
641-
repoHeader.rel = "noopener noreferrer";
642641
repoHeader.dataset.repo = repoFullName; // For identifying repository
643642

644643
// Create repo info section
645644
const repoInfoDiv = document.createElement("div");
646645
repoInfoDiv.className = "repo-info";
647646

647+
const repoHomeLink = document.createElement("a");
648+
repoHomeLink.className = "repo-home-link";
649+
repoHomeLink.href = repoHomeUrl;
650+
repoHomeLink.target = "_blank";
651+
repoHomeLink.rel = "noopener noreferrer";
652+
repoHomeLink.title = `Open ${repoFullName} repository`;
653+
repoHomeLink.setAttribute("aria-label", `Open ${repoFullName} repository`);
654+
648655
const repoIconSvg = getIconSVGElement("repo");
649656
repoIconSvg.setAttribute("width", "14");
650657
repoIconSvg.setAttribute("height", "14");
651658
repoIconSvg.classList.add("repo-icon");
652-
repoInfoDiv.appendChild(repoIconSvg);
659+
repoHomeLink.appendChild(repoIconSvg);
660+
repoInfoDiv.appendChild(repoHomeLink);
661+
662+
const repoNotificationsLink = document.createElement("a");
663+
repoNotificationsLink.className = "repo-notifications-link";
664+
repoNotificationsLink.href = repoNotificationsUrl;
665+
repoNotificationsLink.target = "_blank";
666+
repoNotificationsLink.rel = "noopener noreferrer";
667+
repoNotificationsLink.title = `Open ${repoFullName} notifications`;
668+
repoNotificationsLink.setAttribute("aria-label", `Open notifications for ${repoFullName}`);
653669

654670
const repoNameSpan = document.createElement("span");
655671
repoNameSpan.className = "repo-name";
656672
repoNameSpan.textContent = repoFullName;
657-
repoInfoDiv.appendChild(repoNameSpan);
673+
repoNotificationsLink.appendChild(repoNameSpan);
674+
repoInfoDiv.appendChild(repoNotificationsLink);
658675

659676
// Create repo actions section
660677
const repoActionsDiv = document.createElement("div");
@@ -679,8 +696,8 @@ export function renderNotifications(notifications, shouldResort = true) {
679696

680697
// Add event listener for mark as read button
681698
markReadBtn.addEventListener("click", (e) => {
682-
e.preventDefault(); // Prevent <a> default navigation
683-
e.stopPropagation(); // Stop event bubbling
699+
e.preventDefault();
700+
e.stopPropagation();
684701
config.onMarkRepoAsRead(repoFullName);
685702
});
686703

src/popup/popup.css

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -484,9 +484,7 @@ body.dark-theme .pat-input.input-error {
484484
position: sticky;
485485
top: 0;
486486
z-index: 10;
487-
text-decoration: none;
488487
color: inherit;
489-
cursor: pointer;
490488
transition:
491489
background-color 0.15s,
492490
box-shadow 0.15s;
@@ -500,7 +498,7 @@ body.dark-theme .pat-input.input-error {
500498
background: var(--bg-hover);
501499
}
502500

503-
.repo-group-header:focus-visible {
501+
.repo-group-header:focus-within {
504502
outline: none;
505503
background: var(--bg-hover);
506504
box-shadow: inset 0 0 0 1px var(--action-focus-border);
@@ -516,6 +514,37 @@ body.dark-theme .pat-input.input-error {
516514
opacity: 0.6;
517515
}
518516

517+
.repo-home-link,
518+
.repo-notifications-link {
519+
display: inline-flex;
520+
align-items: center;
521+
min-width: 0;
522+
padding: 1px 2px;
523+
margin: -1px -2px;
524+
color: inherit;
525+
text-decoration: none;
526+
border-radius: var(--radius-sm);
527+
}
528+
529+
.repo-home-link {
530+
flex-shrink: 0;
531+
}
532+
533+
.repo-notifications-link {
534+
min-width: 0;
535+
}
536+
537+
.repo-home-link:focus-visible,
538+
.repo-notifications-link:focus-visible {
539+
outline: none;
540+
background: var(--bg-hover);
541+
box-shadow: 0 0 0 3px var(--action-focus);
542+
}
543+
544+
.repo-notifications-link:focus-visible .repo-name {
545+
text-decoration: underline;
546+
}
547+
519548
.repo-icon {
520549
color: var(--text-secondary);
521550
flex-shrink: 0;
@@ -566,49 +595,48 @@ body.dark-theme .pat-input.input-error {
566595
width: 24px;
567596
height: 24px;
568597
padding: 0;
569-
border: none;
598+
border: 1px solid transparent;
570599
background: transparent;
571600
color: var(--text-secondary);
572601
cursor: pointer;
573602
border-radius: var(--radius-sm);
574603
flex-shrink: 0;
575604
opacity: 0;
576-
visibility: hidden;
577605
pointer-events: none;
578606
transition:
579607
opacity 0.15s ease,
580608
background-color 0.15s ease,
581609
color 0.15s ease,
610+
border-color 0.15s ease,
582611
box-shadow 0.15s ease;
583612
}
584613

585614
.repo-mark-read-btn:hover {
586-
background: var(--bg-secondary);
615+
background: var(--bg-hover);
587616
color: var(--text-primary);
617+
border-color: var(--border-color);
588618
}
589619

590620
.repo-mark-read-btn:active {
591621
transform: scale(0.95);
592622
}
593623

594624
.repo-group-header:hover .repo-count,
595-
.repo-group-header:focus-visible .repo-count,
596-
.repo-actions:has(.repo-mark-read-btn:focus-visible) .repo-count {
625+
.repo-group-header:focus-within .repo-count {
597626
opacity: 0;
598627
}
599628

600629
.repo-group-header:hover .repo-mark-read-btn,
601-
.repo-group-header:focus-visible .repo-mark-read-btn,
602-
.repo-mark-read-btn:focus-visible {
630+
.repo-group-header:focus-within .repo-mark-read-btn {
603631
opacity: 1;
604-
visibility: visible;
605632
pointer-events: auto;
606633
}
607634

608635
.repo-mark-read-btn:focus-visible {
609636
outline: none;
610-
background: var(--bg-secondary);
637+
background: var(--bg-hover);
611638
color: var(--text-primary);
639+
border-color: var(--border-color);
612640
box-shadow: 0 0 0 3px var(--action-focus);
613641
}
614642

tests/notification-renderer-mark-read.test.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ beforeAll(() => {
5252
// ── Mocks must be hoisted before any imports that use them ──────────────────
5353

5454
vi.mock("../src/lib/constants.js", () => ({
55+
GITHUB_SITE_BASE: "https://github.com",
5556
ANIMATION_DURATION: { FADE_OUT: 0, ERROR_BACKGROUND_FADE: 0 },
5657
NOTIFICATION_TYPES: { RELEASE: "Release" },
5758
MESSAGE_TYPES: {
Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { describe, it, expect, vi, beforeEach, beforeAll } from "vitest";
5+
6+
beforeAll(() => {
7+
if (typeof CSS === "undefined" || !CSS.escape) {
8+
// @ts-ignore
9+
globalThis.CSS = {
10+
escape(value) {
11+
const str = String(value);
12+
const length = str.length;
13+
let result = "";
14+
for (let i = 0; i < length; i++) {
15+
const code = str.charCodeAt(i);
16+
if (code === 0x0000) {
17+
result += "\uFFFD";
18+
continue;
19+
}
20+
if (
21+
(code >= 0x0001 && code <= 0x001f) ||
22+
code === 0x007f ||
23+
(i === 0 && code >= 0x0030 && code <= 0x0039) ||
24+
(i === 1 && code >= 0x0030 && code <= 0x0039 && str.charCodeAt(0) === 0x002d)
25+
) {
26+
result += `\\${code.toString(16)} `;
27+
continue;
28+
}
29+
if (
30+
code >= 0x80 ||
31+
code === 0x2d ||
32+
code === 0x5f ||
33+
(code >= 0x30 && code <= 0x39) ||
34+
(code >= 0x41 && code <= 0x5a) ||
35+
(code >= 0x61 && code <= 0x7a)
36+
) {
37+
result += str[i];
38+
} else {
39+
result += `\\${str[i]}`;
40+
}
41+
}
42+
return result;
43+
},
44+
};
45+
}
46+
});
47+
48+
vi.mock("../src/lib/constants.js", () => ({
49+
GITHUB_SITE_BASE: "https://github.com",
50+
ANIMATION_DURATION: { FADE_OUT: 0, ERROR_BACKGROUND_FADE: 0 },
51+
NOTIFICATION_TYPES: { RELEASE: "Release" },
52+
MESSAGE_TYPES: {
53+
MARK_AS_READ: "markAsRead",
54+
OPEN_NOTIFICATION: "openNotification",
55+
},
56+
TIME_CONVERSION: { MS_PER_MINUTE: 60000 },
57+
}));
58+
59+
vi.mock("../src/lib/format-utils.js", () => ({
60+
formatReason: vi.fn((reason) => reason),
61+
getNotificationStatus: vi.fn(() => "Status"),
62+
}));
63+
64+
vi.mock("../src/lib/icons.js", () => ({
65+
getIconSVGElement: vi.fn(() => {
66+
const svg = document.createElementNS("http://www.w3.org/2000/svg", "svg");
67+
return svg;
68+
}),
69+
}));
70+
71+
const sendMessage = vi.fn();
72+
const onMarkRepoAsRead = vi.fn();
73+
74+
const { initRenderer, renderNotifications, clearNotificationCache } =
75+
await import("../src/popup/notification-renderer.js");
76+
77+
function makeNotif(id) {
78+
return {
79+
id: String(id),
80+
title: `Notification ${id}`,
81+
reason: "mention",
82+
updated_at: new Date().toISOString(),
83+
icon: "issue",
84+
type: "Issue",
85+
url: "https://github.com/owner/repo/issues/1",
86+
repository: {
87+
full_name: "owner/repo",
88+
html_url: "https://github.com/owner/repo",
89+
},
90+
};
91+
}
92+
93+
describe("notification repo header links", () => {
94+
let notificationsList;
95+
let emptyState;
96+
let markAllBtn;
97+
98+
beforeEach(() => {
99+
vi.clearAllMocks();
100+
clearNotificationCache();
101+
102+
document.body.innerHTML = `
103+
<ul id="notifications-list"></ul>
104+
<div id="empty-state" hidden></div>
105+
<button id="mark-all-btn"></button>
106+
`;
107+
108+
notificationsList = document.getElementById("notifications-list");
109+
emptyState = document.getElementById("empty-state");
110+
markAllBtn = document.getElementById("mark-all-btn");
111+
112+
initRenderer({
113+
notificationsList,
114+
emptyState,
115+
markAllBtn,
116+
sendMessage,
117+
getShowHoverCards: () => false,
118+
onUserAction: vi.fn(),
119+
onMarkRepoAsRead,
120+
});
121+
});
122+
123+
it("renders separate homepage and notifications links for each repo header", () => {
124+
renderNotifications([makeNotif(1)]);
125+
126+
const repoHeader = notificationsList.querySelector(".repo-group-header");
127+
const repoHomeLink = repoHeader.querySelector(".repo-home-link");
128+
const repoNotificationsLink = repoHeader.querySelector(".repo-notifications-link");
129+
130+
expect(repoHeader.tagName).toBe("DIV");
131+
expect(repoHomeLink.getAttribute("href")).toBe("https://github.com/owner/repo");
132+
expect(repoNotificationsLink.getAttribute("href")).toBe(
133+
"https://github.com/notifications?query=repo%3Aowner%2Frepo",
134+
);
135+
expect(repoNotificationsLink.textContent).toBe("owner/repo");
136+
});
137+
138+
it("keeps mark repo as read independent from repo links", () => {
139+
renderNotifications([makeNotif(1)]);
140+
141+
const markReadBtn = notificationsList.querySelector(".repo-mark-read-btn");
142+
markReadBtn.click();
143+
144+
expect(onMarkRepoAsRead).toHaveBeenCalledWith("owner/repo");
145+
expect(sendMessage).not.toHaveBeenCalled();
146+
});
147+
});

tests/url-builder.test.js

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { describe, it, expect } from "vitest";
2-
import { buildNotificationUrl, buildProfileUrl } from "../src/lib/url-builder.js";
2+
import {
3+
buildNotificationUrl,
4+
buildProfileUrl,
5+
buildRepoNotificationsUrl,
6+
} from "../src/lib/url-builder.js";
37

48
const GITHUB_BASE = "https://github.com";
59

@@ -198,3 +202,17 @@ describe("buildProfileUrl", () => {
198202
expect(buildProfileUrl("")).toBeNull();
199203
});
200204
});
205+
206+
describe("buildRepoNotificationsUrl", () => {
207+
it("should build a repo-scoped notifications URL", () => {
208+
expect(buildRepoNotificationsUrl("owner/repo")).toBe(
209+
"https://github.com/notifications?query=repo%3Aowner%2Frepo",
210+
);
211+
});
212+
213+
it("should return null for falsy repo names", () => {
214+
expect(buildRepoNotificationsUrl(null)).toBeNull();
215+
expect(buildRepoNotificationsUrl(undefined)).toBeNull();
216+
expect(buildRepoNotificationsUrl("")).toBeNull();
217+
});
218+
});

0 commit comments

Comments
 (0)