Skip to content

Commit 615b810

Browse files
committed
refactor(popup): extract settings module
Move the settings panel — theme picker, popup width controls, desktop notification toggle (with browser-permission flow), user info footer, and the sign-out action — out of popup.js into src/popup/settings.js. popup.js drops from ~1126 to ~887 lines and now owns auth/login, the main notification list, sync/filter/settings module wiring, and view-stack helpers (showView, toggleOverlayView). createSettings(deps) takes the cross-view helpers popup.js still owns (toggleOverlayView, setCachedTheme/setCachedPopupWidth localStorage boot-time writers, updateScrollbarCompensation, getAuthMethodLabels, updateProfileLinks, onLogout, sync) and binds its own DOM events. The system-theme media listener moves into settings.init(), called once from popup.js's init(). The notification-permission helpers (hasExtensionNotifications, checkNotificationPermission, requestNotificationPermission) move with the toggle handler since they were only consumed by it. Logout flow: settings.handleLogout calls onLogout() first and only hide()s the panel after it resolves. On the success path showView ('login') tears down main view (and the settings overlay with it), so the trailing hide() is a no-op. On the failure path (MV3 background reconnecting, sendMessage rejection) the panel stays open so the user sees the error in context with an obvious retry path, matching the pre-extraction behavior. logout() in popup.js correspondingly drops its hideSettings() call. Pure relocation otherwise: theme apply timing, width clamping/ clamping-on-save, permission-prompt fallback chain, and event wiring order are all unchanged. The settings.css cascade order in popup.html is unaffected. This finishes the popup module split (filter.js / sync.js / settings.js) mirroring the existing CSS split (filter.css / sync.css / settings.css).
1 parent 9768482 commit 615b810

2 files changed

Lines changed: 308 additions & 263 deletions

File tree

src/popup/popup.js

Lines changed: 27 additions & 263 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import {
1111
MIN_POPUP_WIDTH,
1212
MAX_POPUP_WIDTH,
1313
DEFAULT_POPUP_WIDTH,
14-
POPUP_WIDTH_STEP,
1514
TIMING_THRESHOLDS,
1615
} from "../lib/constants.js";
1716
import { applyTheme } from "../lib/theme.js";
@@ -21,6 +20,7 @@ import { parseSVG } from "../lib/icons.js";
2120
import { initRenderer, clearNotificationCache } from "./notification-renderer.js";
2221
import { createFilter } from "./filter.js";
2322
import { createSync } from "./sync.js";
23+
import { createSettings } from "./settings.js";
2424

2525
/**
2626
* Get auth method labels
@@ -163,7 +163,6 @@ const patLoginBtn = document.getElementById("pat-login-btn");
163163
const loginErrorEl = document.getElementById("login-error");
164164

165165
// Main view elements
166-
const settingsIconBtn = document.getElementById("settings-icon-btn");
167166
const refreshBtn = document.getElementById("refresh-btn");
168167
const markAllBtn = document.getElementById("mark-all-btn");
169168
const usernameEl = document.getElementById("username");
@@ -172,30 +171,18 @@ const userProfileLink = document.getElementById("user-profile-link");
172171
const notificationsList = document.getElementById("notifications-list");
173172
const emptyState = document.getElementById("empty-state");
174173

175-
// Settings view elements
176-
const settingsView = document.getElementById("settings-view");
177-
const settingsBackBtn = document.getElementById("settings-back-btn");
178-
const themeRadios = document.querySelectorAll('input[name="theme"]');
179-
const settingsLogoutBtn = document.getElementById("settings-logout-btn");
180-
const settingsUsernameEl = document.getElementById("settings-username");
181-
const settingsAvatarEl = document.getElementById("settings-avatar");
174+
// Settings page is owned by createSettings (./settings.js); the only
175+
// settings-view DOM popup.js still touches is settingsProfileLink, kept
176+
// here because updateProfileLinks (header + settings) is the cross-view
177+
// helper called from login / init / settings.show().
182178
const settingsProfileLink = document.getElementById("settings-profile-link");
183-
const settingsAuthMethodEl = document.getElementById("settings-auth-method");
179+
184180
const notificationsContainer = document.getElementById("notifications-container");
185181
const refreshCountdownEl = document.getElementById("refresh-countdown");
186182

187183
// Filter view is owned by createFilter (./filter.js).
188184
// Gist sync UI is owned by createSync (./sync.js).
189185

190-
// Popup size controls
191-
const popupWidthInput = document.getElementById("popup-width-input");
192-
const widthDecreaseBtn = document.getElementById("width-decrease");
193-
const widthIncreaseBtn = document.getElementById("width-increase");
194-
195-
// Desktop notification settings
196-
const desktopNotificationsToggle = document.getElementById("desktop-notifications-toggle");
197-
const desktopNotificationsHint = document.getElementById("desktop-notifications-hint");
198-
199186
let scrollbarCompensationRaf = null;
200187

201188
function showLoginError(message) {
@@ -259,48 +246,6 @@ if (notificationsList && typeof MutationObserver !== "undefined") {
259246
mutationObserver.observe(notificationsList, { childList: true });
260247
}
261248

262-
/**
263-
* Check if browser notification permission is granted
264-
*/
265-
function hasExtensionNotifications() {
266-
return (
267-
(typeof chrome !== "undefined" && !!chrome.notifications) ||
268-
(typeof browser !== "undefined" && !!browser.notifications)
269-
);
270-
}
271-
272-
function checkNotificationPermission() {
273-
if (hasExtensionNotifications()) {
274-
return "granted";
275-
}
276-
if (typeof Notification === "undefined") {
277-
console.warn("Notification API not available");
278-
return "unsupported";
279-
}
280-
return Notification.permission;
281-
}
282-
283-
/**
284-
* Request browser notification permission
285-
*/
286-
async function requestNotificationPermission() {
287-
if (hasExtensionNotifications()) {
288-
return "granted";
289-
}
290-
if (typeof Notification === "undefined") {
291-
console.warn("Notification API not available");
292-
return "unsupported";
293-
}
294-
295-
try {
296-
const permission = await Notification.requestPermission();
297-
return permission;
298-
} catch (error) {
299-
console.error("Failed to request notification permission:", error);
300-
return "denied";
301-
}
302-
}
303-
304249
/**
305250
* Update countdown timer
306251
*/
@@ -401,148 +346,17 @@ const sync = createSync({
401346
onPulledFilter: filter.applyPulledFilter,
402347
});
403348

404-
/**
405-
* Show settings view
406-
*/
407-
async function showSettings() {
408-
// Load current theme
409-
const theme = (await storage.getTheme()) || "system";
410-
themeRadios.forEach((radio) => {
411-
radio.checked = radio.value === theme;
412-
});
413-
414-
// Load and display username
415-
const username = await storage.getUsername();
416-
const authMethod = await storage.getAuthMethod();
417-
const { shortLabel, fullLabel } = getAuthMethodLabels(authMethod);
418-
if (settingsUsernameEl && username) {
419-
settingsUsernameEl.textContent = username;
420-
}
421-
if (settingsAuthMethodEl) {
422-
settingsAuthMethodEl.textContent = shortLabel || "";
423-
if (fullLabel) {
424-
settingsAuthMethodEl.title = fullLabel;
425-
} else {
426-
settingsAuthMethodEl.removeAttribute("title");
427-
}
428-
}
429-
430-
// Load and display user avatar
431-
const userInfo = await storage.getUserInfo();
432-
updateProfileLinks(username, userInfo);
433-
if (settingsAvatarEl && userInfo?.avatar_url) {
434-
settingsAvatarEl.src = userInfo.avatar_url;
435-
settingsAvatarEl.alt = userInfo.login || "User";
436-
settingsAvatarEl.hidden = false;
437-
} else if (settingsAvatarEl) {
438-
settingsAvatarEl.hidden = true;
439-
}
440-
441-
// Load popup width setting
442-
const width = await storage.getPopupWidth();
443-
popupWidthInput.value = width;
444-
updateWidthButtons(width);
445-
446-
// Load desktop notification settings
447-
const enableDesktopNotifications = await storage.getEnableDesktopNotifications();
448-
desktopNotificationsToggle.checked = enableDesktopNotifications;
449-
// Check browser notification permission status
450-
const permission = checkNotificationPermission();
451-
452-
// Update toggle state based on permission
453-
if (permission === "denied") {
454-
desktopNotificationsToggle.disabled = true;
455-
desktopNotificationsToggle.parentElement.title =
456-
"Browser notification permission denied. Please enable it in browser settings.";
457-
if (desktopNotificationsHint) {
458-
desktopNotificationsHint.textContent =
459-
"Permission denied. Please enable notifications in browser settings.";
460-
desktopNotificationsHint.hidden = false;
461-
}
462-
} else if (permission === "unsupported") {
463-
desktopNotificationsToggle.disabled = true;
464-
desktopNotificationsToggle.parentElement.title = "Browser notifications not supported.";
465-
if (desktopNotificationsHint) {
466-
desktopNotificationsHint.textContent = "Browser notifications are not supported.";
467-
desktopNotificationsHint.hidden = false;
468-
}
469-
}
470-
sync.init();
471-
toggleOverlayView(true);
472-
settingsView.hidden = false;
473-
}
474-
475-
/**
476-
* Hide settings view
477-
*/
478-
function hideSettings() {
479-
toggleOverlayView(false);
480-
settingsView.hidden = true;
481-
}
482-
483-
/**
484-
* Handle theme change
485-
*/
486-
async function handleThemeChange() {
487-
const selectedTheme = document.querySelector('input[name="theme"]:checked');
488-
const theme = selectedTheme ? selectedTheme.value : "system";
489-
490-
// Save to storage and cache for instant apply on next open
491-
try {
492-
await storage.setTheme(theme);
493-
} catch (error) {
494-
console.error("Failed to save theme:", error);
495-
}
496-
setCachedTheme(theme);
497-
498-
// Apply theme immediately
499-
applyTheme(theme);
500-
}
501-
502-
/**
503-
* Handle popup width change
504-
*/
505-
async function handleWidthChange() {
506-
const parsed = parseInt(popupWidthInput.value, 10);
507-
const width = clampPopupWidth(isNaN(parsed) ? MIN_POPUP_WIDTH : parsed);
508-
509-
popupWidthInput.value = width;
510-
document.body.style.width = `${width}px`;
511-
updateScrollbarCompensation();
512-
setCachedPopupWidth(width);
513-
updateWidthButtons(width);
514-
515-
// Save to storage
516-
try {
517-
await storage.setPopupWidth(width);
518-
} catch (error) {
519-
console.error("Failed to save popup width:", error);
520-
}
521-
}
522-
523-
/**
524-
* Decrease width
525-
*/
526-
async function decreaseWidth() {
527-
const currentWidth = parseInt(popupWidthInput.value, 10);
528-
popupWidthInput.value = clampPopupWidth(currentWidth - POPUP_WIDTH_STEP);
529-
await handleWidthChange();
530-
}
531-
532-
/**
533-
* Increase width
534-
*/
535-
async function increaseWidth() {
536-
const currentWidth = parseInt(popupWidthInput.value, 10);
537-
popupWidthInput.value = clampPopupWidth(currentWidth + POPUP_WIDTH_STEP);
538-
await handleWidthChange();
539-
}
540-
541-
function updateWidthButtons(width) {
542-
if (!widthDecreaseBtn || !widthIncreaseBtn) return;
543-
widthDecreaseBtn.disabled = width <= MIN_POPUP_WIDTH;
544-
widthIncreaseBtn.disabled = width >= MAX_POPUP_WIDTH;
545-
}
349+
const settings = createSettings({
350+
storage,
351+
toggleOverlayView,
352+
setCachedTheme,
353+
setCachedPopupWidth,
354+
updateScrollbarCompensation,
355+
getAuthMethodLabels,
356+
updateProfileLinks,
357+
onLogout: logout,
358+
sync,
359+
});
546360

547361
/**
548362
* Send message to background script.
@@ -803,11 +617,16 @@ async function handleOAuthLogin() {
803617

804618
/**
805619
* Logout
620+
*
621+
* Called by settings.handleLogout BEFORE the panel hides, so a failure
622+
* here (MV3 background reconnecting, sendMessage rejection) leaves the
623+
* settings view open and the user sees the error in context. On success
624+
* showView('login') tears down the entire main view and the panel along
625+
* with it; settings then calls hide() as a no-op cleanup.
806626
*/
807627
async function logout() {
808628
stopCountdown();
809629
await sendMessage(MESSAGE_TYPES.LOGOUT);
810-
hideSettings();
811630
await showView("login");
812631
}
813632

@@ -963,13 +782,8 @@ async function init() {
963782
onMarkAsReadSuccess: filter.refreshFilteredBadge,
964783
});
965784

966-
// Listen for system theme changes
967-
window.matchMedia("(prefers-color-scheme: dark)").addEventListener("change", async () => {
968-
const currentTheme = await storage.getTheme();
969-
if (currentTheme === "system") {
970-
applyTheme("system");
971-
}
972-
});
785+
// Settings module owns the system-theme media listener.
786+
settings.init();
973787

974788
const state = await sendMessage(MESSAGE_TYPES.GET_STATE);
975789

@@ -1018,61 +832,11 @@ patInput.addEventListener("input", () => {
1018832
}
1019833
});
1020834

1021-
// Settings
1022-
settingsIconBtn.addEventListener("click", showSettings);
1023-
settingsBackBtn.addEventListener("click", hideSettings);
1024-
themeRadios.forEach((radio) => {
1025-
radio.addEventListener("change", handleThemeChange);
1026-
});
1027-
popupWidthInput.addEventListener("change", handleWidthChange);
1028-
popupWidthInput.addEventListener("blur", handleWidthChange);
1029-
widthDecreaseBtn.addEventListener("click", decreaseWidth);
1030-
widthIncreaseBtn.addEventListener("click", increaseWidth);
1031-
835+
// Settings page (theme / width / desktop notifications / sign-out) is wired
836+
// up inside createSettings (./settings.js).
1032837
// Filter page is wired up inside createFilter (./filter.js).
1033838
// Gist sync is wired up inside createSync (./sync.js).
1034839

1035-
// Desktop notification settings
1036-
desktopNotificationsToggle.addEventListener("change", async () => {
1037-
const enabled = desktopNotificationsToggle.checked;
1038-
1039-
if (desktopNotificationsHint) desktopNotificationsHint.hidden = true;
1040-
1041-
if (enabled) {
1042-
// Check current permission
1043-
let permission = checkNotificationPermission();
1044-
1045-
// Request permission if not granted
1046-
if (permission === "default" || permission === "prompt") {
1047-
permission = await requestNotificationPermission();
1048-
}
1049-
1050-
// Only enable if permission granted
1051-
if (permission === "granted") {
1052-
await storage.setEnableDesktopNotifications(true);
1053-
} else {
1054-
// Permission denied or unavailable
1055-
desktopNotificationsToggle.checked = false;
1056-
await storage.setEnableDesktopNotifications(false);
1057-
1058-
if (desktopNotificationsHint) {
1059-
if (permission === "denied") {
1060-
desktopNotificationsHint.textContent =
1061-
"Permission denied. Please enable notifications in browser settings.";
1062-
} else if (permission === "unsupported") {
1063-
desktopNotificationsHint.textContent = "Browser notifications are not supported.";
1064-
}
1065-
desktopNotificationsHint.hidden = false;
1066-
}
1067-
}
1068-
} else {
1069-
// User disabled the toggle
1070-
await storage.setEnableDesktopNotifications(false);
1071-
}
1072-
});
1073-
1074-
// User menu
1075-
settingsLogoutBtn.addEventListener("click", logout);
1076840
refreshBtn.addEventListener("click", refresh);
1077841
markAllBtn.addEventListener("click", markAllAsRead);
1078842

0 commit comments

Comments
 (0)