Skip to content

Commit c1b9fb1

Browse files
committed
feat(popup): rework mark-as-read overlay animations
- Use ::after overlay + border-color fade for single/repo/all flows - Defer DOM removal until API success with requestAnimationFrame guard - Track and cancel stagger timeouts on rollback - Fix clearNotificationCache() to clear both cached data and hash - Remove slide-up/rollback dead code and SLIDE_UP constant
1 parent 740d31c commit c1b9fb1

5 files changed

Lines changed: 150 additions & 157 deletions

File tree

src/lib/constants.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ export const MAX_POLL_INTERVAL_SECONDS = 600; // 10 minutes maximum (prevent exc
1616
// UI Animation Timings (milliseconds)
1717
export const ANIMATION_DURATION = {
1818
FADE_OUT: 200,
19-
SLIDE_UP: 200,
2019
STAGGER_DELAY: 50,
2120
MIN_SPINNER_TIME: 500,
2221
STATUS_MESSAGE: 3000,

src/popup/notification-renderer.js

Lines changed: 29 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ export function getCachedNotifications() {
9191
* Clear notification cache to force re-render
9292
*/
9393
export function clearNotificationCache() {
94+
cachedNotifications = null;
9495
cachedNotificationsHash = null;
9596
}
9697

@@ -346,7 +347,7 @@ function createNotificationItem(notif, repoHeader, repoFullName, notifications)
346347
window.close();
347348
});
348349

349-
// Mark as read button with optimistic update
350+
// Mark as read button with immediate visual feedback
350351
const markReadBtn = li.querySelector('.btn-mark-read');
351352
markReadBtn.addEventListener('click', async (e) => {
352353
e.stopPropagation();
@@ -360,70 +361,20 @@ function createNotificationItem(notif, repoHeader, repoFullName, notifications)
360361
markReadBtn.textContent = '✓';
361362

362363
// Track user action
363-
const animationDuration = ANIMATION_DURATION.FADE_OUT + ANIMATION_DURATION.SLIDE_UP;
364+
const animationDuration = ANIMATION_DURATION.FADE_OUT;
364365
onUserAction(animationDuration);
365366

367+
// Force reflow so ::after pseudo-element starts at opacity 0
368+
// eslint-disable-next-line no-unused-expressions
369+
li.offsetHeight;
366370
li.classList.add('fade-out');
367371

368-
const originalParent = li.parentElement;
369-
const originalNextSibling = li.nextSibling;
370-
371-
const slideUpTimeout = setTimeout(() => {
372-
li.classList.add('slide-up');
373-
374-
const removeTimeout = setTimeout(() => {
375-
li.remove();
376-
377-
// Check if any notifications from this group remain
378-
const groupItems = notificationsList.querySelectorAll('.notification-item[data-id]');
379-
let hasNotificationsInGroup = false;
380-
381-
for (const item of groupItems) {
382-
const itemId = item.dataset.id;
383-
const itemNotif = notifications.find((n) => n.id === itemId);
384-
if (itemNotif && itemNotif.repository.full_name === repoFullName) {
385-
hasNotificationsInGroup = true;
386-
break;
387-
}
388-
}
389-
390-
if (!hasNotificationsInGroup && repoHeader) {
391-
repoHeader.remove();
392-
}
393-
394-
const remaining = notificationsList.querySelectorAll('.notification-item').length;
395-
if (remaining === 0) {
396-
emptyState.hidden = false;
397-
markAllBtn.disabled = true;
398-
}
399-
}, ANIMATION_DURATION.SLIDE_UP);
400-
401-
li.dataset.removeTimeout = removeTimeout;
402-
}, ANIMATION_DURATION.FADE_OUT);
403-
404-
li.dataset.slideUpTimeout = slideUpTimeout;
405-
406372
// Restore the notification item to its original state on failure
407373
function restoreItem() {
408-
clearTimeout(slideUpTimeout);
409-
if (li.dataset.removeTimeout) {
410-
clearTimeout(parseInt(li.dataset.removeTimeout));
411-
}
412-
413-
li.classList.remove('marking-read', 'fade-out', 'slide-up');
374+
li.classList.remove('marking-read', 'fade-out');
414375
markReadBtn.disabled = false;
415376
markReadBtn.textContent = '✓';
416377

417-
if (!li.parentElement) {
418-
if (originalNextSibling && originalNextSibling.parentElement) {
419-
originalParent.insertBefore(li, originalNextSibling);
420-
} else {
421-
originalParent.appendChild(li);
422-
}
423-
emptyState.hidden = true;
424-
markAllBtn.disabled = false;
425-
}
426-
427378
li.style.backgroundColor = 'rgba(239, 68, 68, 0.1)';
428379
setTimeout(() => {
429380
li.style.backgroundColor = '';
@@ -433,7 +384,28 @@ function createNotificationItem(notif, repoHeader, repoFullName, notifications)
433384
try {
434385
const result = await sendMessage(MESSAGE_TYPES.MARK_AS_READ, { notificationId: notif.id });
435386

436-
if (!result.success) {
387+
if (result.success) {
388+
// Ensure at least one frame of the overlay animation is painted
389+
await new Promise(requestAnimationFrame);
390+
391+
// Success: remove notification from DOM
392+
li.remove();
393+
394+
// Check if any notifications from this group remain
395+
const escapedRepo = CSS.escape(repoFullName);
396+
const remainingInGroup = notificationsList.querySelectorAll(`.notification-item[data-repo="${escapedRepo}"]`);
397+
398+
if (remainingInGroup.length === 0 && repoHeader) {
399+
repoHeader.remove();
400+
}
401+
402+
const remaining = notificationsList.querySelectorAll('.notification-item').length;
403+
if (remaining === 0) {
404+
emptyState.hidden = false;
405+
markAllBtn.disabled = true;
406+
}
407+
} else {
408+
// Failure: restore item
437409
restoreItem();
438410
}
439411
} catch (error) {

src/popup/popup.css

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -532,36 +532,41 @@ body.dark-theme .pat-input.input-error {
532532
}
533533

534534
/* Fade out animation for marking as read */
535-
.notification-item.marking-read {
535+
.notification-item.marking-read,
536+
.repo-group-header.marking-read {
536537
pointer-events: none;
538+
transition: border-color 0.2s ease-out;
537539
}
538540

539-
.notification-item.fade-out {
541+
.notification-item.fade-out,
542+
.repo-group-header.fade-out {
543+
border-color: transparent;
544+
}
545+
546+
/* Use overlay instead of opacity to avoid compositing artifacts in scroll containers */
547+
.notification-item.marking-read::after {
548+
content: '';
549+
position: absolute;
550+
inset: 0;
551+
background: var(--bg-primary);
540552
opacity: 0;
541-
transform: translateX(10px);
542-
transition:
543-
opacity 0.2s ease-out,
544-
transform 0.2s ease-out;
553+
pointer-events: none;
554+
transition: opacity 0.2s ease-out;
545555
}
546556

547-
.notification-item.slide-up {
548-
max-height: 0;
549-
padding-top: 0;
550-
padding-bottom: 0;
551-
margin-top: 0;
552-
margin-bottom: 0;
553-
border-top-width: 0;
554-
overflow: hidden;
555-
transition:
556-
max-height 0.2s ease-out,
557-
padding 0.2s ease-out,
558-
margin 0.2s ease-out,
559-
border-top-width 0.2s ease-out;
557+
.repo-group-header.marking-read::after {
558+
content: '';
559+
position: absolute;
560+
inset: 0;
561+
background: var(--bg-secondary);
562+
opacity: 0;
563+
pointer-events: none;
564+
transition: opacity 0.2s ease-out;
560565
}
561566

562-
/* Smooth transitions for background color (error feedback) */
563-
.notification-item.rollback {
564-
transition: background-color 0.3s ease-out;
567+
.notification-item.fade-out::after,
568+
.repo-group-header.fade-out::after {
569+
opacity: 1;
565570
}
566571

567572
.notification-item:hover {
@@ -599,10 +604,6 @@ body.dark-theme .pat-input.input-error {
599604
pointer-events: auto; /* Enable mouse interaction when visible */
600605
}
601606

602-
.notification-item {
603-
position: relative; /* Enable absolute positioning for child */
604-
}
605-
606607
body.dark-theme .notification-hover-card {
607608
box-shadow:
608609
0 10px 15px -3px rgba(0, 0, 0, 0.3),

0 commit comments

Comments
 (0)