Skip to content

Commit 2c38b1b

Browse files
committed
fix: replace alert() with inline hints for notification permission feedback
- Replace blocking alert() dialogs with inline hint text below the Desktop Notifications toggle in settings - Show hint on settings page init when permission is denied/unsupported - Add clarifying comment for retryOn: [500] meaning all 5xx errors
1 parent f74d0e7 commit 2c38b1b

File tree

4 files changed

+24
-7
lines changed

4 files changed

+24
-7
lines changed

src/lib/github-api.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ async function retryWithStrategy(fetchFn, options = {}) {
8787
return response;
8888
}
8989

90-
// Check if we should retry this status code
90+
// Check if we should retry this status code.
91+
// Note: including 500 in retryOn acts as a wildcard for all 5xx errors.
9192
const shouldRetry =
9293
retryOn.includes(response.status) || (response.status >= 500 && retryOn.includes(500));
9394

src/popup/popup.css

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1454,6 +1454,7 @@ body.dark-theme .settings-view {
14541454

14551455
.setting-hint {
14561456
margin-top: 4px;
1457+
padding-left: 40px;
14571458
font-size: 11px;
14581459
color: var(--text-secondary);
14591460
line-height: 1.4;

src/popup/popup.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,7 @@ <h3 class="settings-title">Settings</h3>
322322
<span class="toggle-slider" aria-hidden="true"></span>
323323
</label>
324324
</div>
325+
<div id="desktop-notifications-hint" class="setting-hint" hidden aria-live="polite"></div>
325326

326327
<div class="setting-row">
327328
<span class="setting-icon" aria-hidden="true">

src/popup/popup.js

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ const hoverCardsToggle = document.getElementById("hover-cards-toggle");
197197

198198
// Desktop notification settings
199199
const desktopNotificationsToggle = document.getElementById("desktop-notifications-toggle");
200+
const desktopNotificationsHint = document.getElementById("desktop-notifications-hint");
200201

201202
let scrollbarCompensationRaf = null;
202203

@@ -440,9 +441,18 @@ async function showSettings() {
440441
desktopNotificationsToggle.disabled = true;
441442
desktopNotificationsToggle.parentElement.title =
442443
"Browser notification permission denied. Please enable it in browser settings.";
444+
if (desktopNotificationsHint) {
445+
desktopNotificationsHint.textContent =
446+
"Permission denied. Please enable notifications in browser settings.";
447+
desktopNotificationsHint.hidden = false;
448+
}
443449
} else if (permission === "unsupported") {
444450
desktopNotificationsToggle.disabled = true;
445451
desktopNotificationsToggle.parentElement.title = "Browser notifications not supported.";
452+
if (desktopNotificationsHint) {
453+
desktopNotificationsHint.textContent = "Browser notifications are not supported.";
454+
desktopNotificationsHint.hidden = false;
455+
}
446456
}
447457
// Hide header and footer
448458
document.querySelector(".header").hidden = true;
@@ -1011,6 +1021,8 @@ hoverCardsToggle.addEventListener("change", async () => {
10111021
desktopNotificationsToggle.addEventListener("change", async () => {
10121022
const enabled = desktopNotificationsToggle.checked;
10131023

1024+
if (desktopNotificationsHint) desktopNotificationsHint.hidden = true;
1025+
10141026
if (enabled) {
10151027
// Check current permission
10161028
let permission = checkNotificationPermission();
@@ -1028,12 +1040,14 @@ desktopNotificationsToggle.addEventListener("change", async () => {
10281040
desktopNotificationsToggle.checked = false;
10291041
await storage.setEnableDesktopNotifications(false);
10301042

1031-
if (permission === "denied") {
1032-
alert(
1033-
"Browser notification permission was denied. Please enable it in your browser settings to use desktop notifications.",
1034-
);
1035-
} else if (permission === "unsupported") {
1036-
alert("Browser notifications are not supported in this browser.");
1043+
if (desktopNotificationsHint) {
1044+
if (permission === "denied") {
1045+
desktopNotificationsHint.textContent =
1046+
"Permission denied. Please enable notifications in browser settings.";
1047+
} else if (permission === "unsupported") {
1048+
desktopNotificationsHint.textContent = "Browser notifications are not supported.";
1049+
}
1050+
desktopNotificationsHint.hidden = false;
10371051
}
10381052
}
10391053
} else {

0 commit comments

Comments
 (0)