Skip to content

Commit 0f7a8d3

Browse files
committed
perf(popup): use server-returned notifications after mark-repo-as-read
Have markRepoAsRead return the updated notifications list instead of requiring a separate GET_STATE round-trip. Eliminates redundant manual DOM removal + full re-render cycle. Fall back to GET_STATE if the response payload is invalid.
1 parent abb7d3c commit 0f7a8d3

3 files changed

Lines changed: 18 additions & 15 deletions

File tree

src/background/service-worker.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -750,7 +750,7 @@ async function markRepoAsRead(owner, repo) {
750750
await storage.setNotifications(updated);
751751
await updateBadge(updated.length, hasMoreNotifications);
752752

753-
return { success: true };
753+
return { success: true, notifications: updated };
754754
} catch (error) {
755755
console.error('Failed to mark repo as read:', error);
756756
return { success: false, error: error.message };

src/popup/popup.js

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -858,21 +858,18 @@ async function handleMarkRepoAsRead(repoFullName) {
858858
if (response.success) {
859859
// Wait for stagger animation to finish before removing DOM
860860
await anim.waitForCompletion();
861-
allElements.forEach((el) => el.remove());
862861

863-
const remainingItems = notificationsList.querySelectorAll('.notification-item');
864-
if (remainingItems.length === 0) {
865-
emptyState.hidden = false;
866-
markAllBtn.disabled = true;
867-
}
868-
869-
// Reload to get updated state
870-
try {
862+
// Re-render with updated notifications returned by the background.
863+
// Defensive fallback: if payload shape is unexpected, reload full state.
864+
let nextNotifications = response.notifications;
865+
if (!Array.isArray(nextNotifications)) {
866+
console.warn('MARK_REPO_AS_READ returned invalid notifications payload, reloading state');
871867
const state = await sendMessage(MESSAGE_TYPES.GET_STATE);
872-
renderNotifications(state.notifications, true);
873-
} catch (error) {
874-
console.error('Failed to reload notifications:', error);
868+
nextNotifications = Array.isArray(state.notifications) ? state.notifications : [];
875869
}
870+
871+
clearNotificationCache();
872+
renderNotifications(nextNotifications, false);
876873
} else {
877874
anim.rollback();
878875
console.error('Failed to mark repo as read:', response.error);

tests/service-worker.test.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,10 @@ describe('service-worker', () => {
512512
{ id: '456', repository: { full_name: 'other/repo' }, title: 'Test 2' },
513513
]);
514514
expect(mockAction.setBadgeText).toHaveBeenCalledWith({ text: '1' });
515-
expect(sendResponse).toHaveBeenCalledWith({ success: true });
515+
expect(sendResponse).toHaveBeenCalledWith({
516+
success: true,
517+
notifications: [{ id: '456', repository: { full_name: 'other/repo' }, title: 'Test 2' }],
518+
});
516519
});
517520

518521
it('should return error on API failure', async () => {
@@ -552,7 +555,10 @@ describe('service-worker', () => {
552555
await new Promise((resolve) => setTimeout(resolve, 50));
553556

554557
expect(mockAction.setBadgeText).toHaveBeenCalledWith({ text: '1+' });
555-
expect(sendResponse).toHaveBeenCalledWith({ success: true });
558+
expect(sendResponse).toHaveBeenCalledWith({
559+
success: true,
560+
notifications: [{ id: '456', repository: { full_name: 'other/repo' }, title: 'Test 2' }],
561+
});
556562
});
557563
});
558564

0 commit comments

Comments
 (0)