Partially refresh notifications list (#35010)

This PR prevents full reloads for the notifications list when changing a
notifications status (read, unread, pinned).

---------

Co-authored-by: Anton Bracke <anton.bracke@fastleansmart.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

routers/web/user/notification.go

@@ -4,10 +4,8 @@
 package user
 
 import (
-	"errors"
 	"fmt"
 	"net/http"
-	"net/url"
 	"strings"
 
 	activities_model "code.gitea.io/gitea/models/activities"
@@ -34,58 +32,42 @@ const (
 	tplNotificationSubscriptions templates.TplName = "user/notification/notification_subscriptions"
 )
 
-// Notifications is the notifications page
+// Notifications is the notification list page
 func Notifications(ctx *context.Context) {
-	getNotifications(ctx)
+	prepareUserNotificationsData(ctx)
 	if ctx.Written() {
 		return
 	}
 	if ctx.FormBool("div-only") {
-		ctx.Data["SequenceNumber"] = ctx.FormString("sequence-number")
 		ctx.HTML(http.StatusOK, tplNotificationDiv)
 		return
 	}
 	ctx.HTML(http.StatusOK, tplNotification)
 }
 
-func getNotifications(ctx *context.Context) {
-	var (
-		keyword = ctx.FormTrim("q")
-		status  activities_model.NotificationStatus
-		page    = ctx.FormInt("page")
-		perPage = ctx.FormInt("perPage")
-	)
-	if page < 1 {
-		page = 1
-	}
-	if perPage < 1 {
-		perPage = 20
-	}
-
-	switch keyword {
-	case "read":
-		status = activities_model.NotificationStatusRead
-	default:
-		status = activities_model.NotificationStatusUnread
-	}
+func prepareUserNotificationsData(ctx *context.Context) {
+	pageType := ctx.FormString("type", ctx.FormString("q")) // "q" is the legacy query parameter for "page type"
+	page := max(1, ctx.FormInt("page"))
+	perPage := util.IfZero(ctx.FormInt("perPage"), 20) // this value is never used or exposed ....
+	queryStatus := util.Iif(pageType == "read", activities_model.NotificationStatusRead, activities_model.NotificationStatusUnread)
 
 	total, err := db.Count[activities_model.Notification](ctx, activities_model.FindNotificationOptions{
 		UserID: ctx.Doer.ID,
-		Status: []activities_model.NotificationStatus{status},
+		Status: []activities_model.NotificationStatus{queryStatus},
 	})
 	if err != nil {
 		ctx.ServerError("ErrGetNotificationCount", err)
 		return
 	}
 
-	// redirect to last page if request page is more than total pages
 	pager := context.NewPagination(int(total), perPage, page, 5)
 	if pager.Paginater.Current() < page {
-		ctx.Redirect(fmt.Sprintf("%s/notifications?q=%s&page=%d", setting.AppSubURL, url.QueryEscape(ctx.FormString("q")), pager.Paginater.Current()))
-		return
+		// use the last page if the requested page is more than total pages
+		page = pager.Paginater.Current()
+		pager = context.NewPagination(int(total), perPage, page, 5)
 	}
 
-	statuses := []activities_model.NotificationStatus{status, activities_model.NotificationStatusPinned}
+	statuses := []activities_model.NotificationStatus{queryStatus, activities_model.NotificationStatusPinned}
 	nls, err := db.Find[activities_model.Notification](ctx, activities_model.FindNotificationOptions{
 		ListOptions: db.ListOptions{
 			PageSize: perPage,
@@ -142,51 +124,37 @@ func getNotifications(ctx *context.Context) {
 	}
 
 	ctx.Data["Title"] = ctx.Tr("notifications")
-	ctx.Data["Keyword"] = keyword
-	ctx.Data["Status"] = status
+	ctx.Data["PageType"] = pageType
 	ctx.Data["Notifications"] = notifications
-
+	ctx.Data["Link"] = setting.AppSubURL + "/notifications"
+	ctx.Data["SequenceNumber"] = ctx.FormString("sequence-number")
 	pager.AddParamFromRequest(ctx.Req)
 	ctx.Data["Page"] = pager
 }
 
 // NotificationStatusPost is a route for changing the status of a notification
 func NotificationStatusPost(ctx *context.Context) {
-	var (
-		notificationID = ctx.FormInt64("notification_id")
-		statusStr      = ctx.FormString("status")
-		status         activities_model.NotificationStatus
-	)
-
-	switch statusStr {
-	case "read":
-		status = activities_model.NotificationStatusRead
-	case "unread":
-		status = activities_model.NotificationStatusUnread
-	case "pinned":
-		status = activities_model.NotificationStatusPinned
+	notificationID := ctx.FormInt64("notification_id")
+	var newStatus activities_model.NotificationStatus
+	switch ctx.FormString("notification_action") {
+	case "mark_as_read":
+		newStatus = activities_model.NotificationStatusRead
+	case "mark_as_unread":
+		newStatus = activities_model.NotificationStatusUnread
+	case "pin":
+		newStatus = activities_model.NotificationStatusPinned
 	default:
-		ctx.ServerError("InvalidNotificationStatus", errors.New("Invalid notification status"))
-		return
+		return // ignore user's invalid input
 	}
-
-	if _, err := activities_model.SetNotificationStatus(ctx, notificationID, ctx.Doer, status); err != nil {
+	if _, err := activities_model.SetNotificationStatus(ctx, notificationID, ctx.Doer, newStatus); err != nil {
 		ctx.ServerError("SetNotificationStatus", err)
 		return
 	}
 
-	if !ctx.FormBool("noredirect") {
-		url := fmt.Sprintf("%s/notifications?page=%s", setting.AppSubURL, url.QueryEscape(ctx.FormString("page")))
-		ctx.Redirect(url, http.StatusSeeOther)
-	}
-
-	getNotifications(ctx)
+	prepareUserNotificationsData(ctx)
 	if ctx.Written() {
 		return
 	}
-	ctx.Data["Link"] = setting.AppSubURL + "/notifications"
-	ctx.Data["SequenceNumber"] = ctx.Req.PostFormValue("sequence-number")
-
 	ctx.HTML(http.StatusOK, tplNotificationDiv)
 }
 

templates/user/notification/notification_div.tmpl

@@ -1,124 +1,96 @@
 <div role="main" aria-label="{{.Title}}" class="page-content user notification" id="notification_div" data-sequence-number="{{.SequenceNumber}}">
 	<div class="ui container">
+		{{$statusUnread := 1}}{{$statusRead := 2}}{{$statusPinned := 3}}
 		{{$notificationUnreadCount := call .PageGlobalData.GetNotificationUnreadCount}}
-		<div class="tw-flex tw-items-center tw-justify-between tw-mb-[--page-spacing]">
+		{{$pageTypeIsRead := eq $.PageType "read"}}
+		<div class="flex-text-block tw-justify-between tw-mb-[--page-spacing]">
 			<div class="small-menu-items ui compact tiny menu">
-				<a class="{{if eq .Status 1}}active {{end}}item" href="{{AppSubUrl}}/notifications?q=unread">
+				<a class="{{if not $pageTypeIsRead}}active{{end}} item" href="{{AppSubUrl}}/notifications?type=unread">
 					{{ctx.Locale.Tr "notification.unread"}}
 					<div class="notifications-unread-count ui label {{if not $notificationUnreadCount}}tw-hidden{{end}}">{{$notificationUnreadCount}}</div>
 				</a>
-				<a class="{{if eq .Status 2}}active {{end}}item" href="{{AppSubUrl}}/notifications?q=read">
+				<a class="{{if $pageTypeIsRead}}active{{end}} item" href="{{AppSubUrl}}/notifications?type=read">
 					{{ctx.Locale.Tr "notification.read"}}
 				</a>
 			</div>
-			{{if and (eq .Status 1)}}
+			{{if and (not $pageTypeIsRead) $notificationUnreadCount}}
 				<form action="{{AppSubUrl}}/notifications/purge" method="post">
 					{{$.CsrfTokenHtml}}
-					<div class="{{if not $notificationUnreadCount}}tw-hidden{{end}}">
 					<button class="ui mini button primary tw-mr-0" title="{{ctx.Locale.Tr "notification.mark_all_as_read"}}">
 						{{svg "octicon-checklist"}}
 					</button>
-					</div>
 				</form>
 			{{end}}
 		</div>
-		<div class="tw-p-0">
 		<div id="notification_table">
-				{{if not .Notifications}}
-					<div class="tw-flex tw-items-center tw-flex-col tw-p-4">
-						{{svg "octicon-inbox" 56 "tw-mb-4"}}
-						{{if eq .Status 1}}
-							{{ctx.Locale.Tr "notification.no_unread"}}
-						{{else}}
-							{{ctx.Locale.Tr "notification.no_read"}}
-						{{end}}
-					</div>
-				{{else}}
-					{{range $notification := .Notifications}}
-						<div class="notifications-item tw-flex tw-items-center tw-flex-wrap tw-gap-2 tw-p-2" id="notification_{{.ID}}" data-status="{{.Status}}">
-							<div class="notifications-icon tw-ml-2 tw-mr-1 tw-self-start tw-mt-1">
-								{{if .Issue}}
-									{{template "shared/issueicon" .Issue}}
+			{{range $one := .Notifications}}
+				<div class="notifications-item" id="notification_{{$one.ID}}" data-status="{{$one.Status}}">
+					<div class="tw-self-start tw-mt-[2px]">
+						{{if $one.Issue}}
+							{{template "shared/issueicon" $one.Issue}}
 						{{else}}
 							{{svg "octicon-repo" 16 "text grey"}}
 						{{end}}
 					</div>
-							<a class="notifications-link tw-flex tw-flex-1 tw-flex-col silenced" href="{{.Link ctx}}">
-								<div class="notifications-top-row tw-text-13 tw-break-anywhere">
-									{{.Repository.FullName}} {{if .Issue}}<span class="text light-3">#{{.Issue.Index}}</span>{{end}}
-									{{if eq .Status 3}}
-										{{svg "octicon-pin" 13 "text blue tw-mt-0.5 tw-ml-1"}}
+					<a class="notifications-link silenced tw-flex-1" href="{{$one.Link ctx}}">
+						<div class="flex-text-block tw-text-[0.95em]">
+							{{$one.Repository.FullName}} {{if $one.Issue}}<span class="text light-3">#{{$one.Issue.Index}}</span>{{end}}
+							{{if eq $one.Status $statusPinned}}
+								{{svg "octicon-pin" 13 "text blue"}}
 							{{end}}
 						</div>
-								<div class="notifications-bottom-row tw-text-16 tw-py-0.5">
-									<span class="issue-title tw-break-anywhere">
-										{{if .Issue}}
-											{{.Issue.Title | ctx.RenderUtils.RenderIssueSimpleTitle}}
+						<div class="tw-text-16 tw-py-0.5">
+							{{if $one.Issue}}
+								{{$one.Issue.Title | ctx.RenderUtils.RenderIssueSimpleTitle}}
 							{{else}}
-											{{.Repository.FullName}}
+								{{$one.Repository.FullName}}
 							{{end}}
-									</span>
 						</div>
 					</a>
-							<div class="notifications-updated tw-items-center tw-mr-2">
-								{{if .Issue}}
-									{{DateUtils.TimeSince .Issue.UpdatedUnix}}
+					<div class="notifications-updated flex-text-inline">
+						{{if $one.Issue}}
+							{{DateUtils.TimeSince $one.Issue.UpdatedUnix}}
 						{{else}}
-									{{DateUtils.TimeSince .UpdatedUnix}}
+							{{DateUtils.TimeSince $one.UpdatedUnix}}
 						{{end}}
 					</div>
-							<div class="notifications-buttons tw-items-center tw-justify-end tw-gap-1 tw-px-1">
-								{{if ne .Status 3}}
-									<form action="{{AppSubUrl}}/notifications/status" method="post">
+					<form class="notifications-buttons" action="{{AppSubUrl}}/notifications/status?type={{$.PageType}}&page={{$.Page.Paginater.Current}}&perPage={{$.Page.Paginater.PagingNum}}" method="post"
+								hx-boost="true" hx-target="#notification_div" hx-swap="outerHTML"
+					>
 						{{$.CsrfTokenHtml}}
-										<input type="hidden" name="notification_id" value="{{.ID}}">
-										<input type="hidden" name="status" value="pinned">
-										<button class="btn interact-bg tw-p-2" title="{{ctx.Locale.Tr "notification.pin"}}"
-											data-url="{{AppSubUrl}}/notifications/status"
-											data-status="pinned"
-											data-page="{{$.Page.Paginater.Current}}"
-											data-notification-id="{{.ID}}"
-											data-q="{{$.Keyword}}">
+						<input type="hidden" name="notification_id" value="{{$one.ID}}">
+						{{if ne $one.Status $statusPinned}}
+							<button class="btn interact-bg tw-p-2" data-tooltip-content="{{ctx.Locale.Tr "notification.pin"}}"
+											name="notification_action" value="pin"
+							>
 								{{svg "octicon-pin"}}
 							</button>
-									</form>
 						{{end}}
-								{{if or (eq .Status 1) (eq .Status 3)}}
-									<form action="{{AppSubUrl}}/notifications/status" method="post">
-										{{$.CsrfTokenHtml}}
-										<input type="hidden" name="notification_id" value="{{.ID}}">
-										<input type="hidden" name="status" value="read">
-										<input type="hidden" name="page" value="{{$.Page.Paginater.Current}}">
-										<button class="btn interact-bg tw-p-2" title="{{ctx.Locale.Tr "notification.mark_as_read"}}"
-											data-url="{{AppSubUrl}}/notifications/status"
-											data-status="read"
-											data-page="{{$.Page.Paginater.Current}}"
-											data-notification-id="{{.ID}}"
-											data-q="{{$.Keyword}}">
+						{{if or (eq $one.Status $statusUnread) (eq $one.Status $statusPinned)}}
+							<button class="btn interact-bg tw-p-2" data-tooltip-content="{{ctx.Locale.Tr "notification.mark_as_read"}}"
+											name="notification_action" value="mark_as_read"
+							>
 								{{svg "octicon-check"}}
 							</button>
-									</form>
-								{{else if eq .Status 2}}
-									<form action="{{AppSubUrl}}/notifications/status" method="post">
-										{{$.CsrfTokenHtml}}
-										<input type="hidden" name="notification_id" value="{{.ID}}">
-										<input type="hidden" name="status" value="unread">
-										<input type="hidden" name="page" value="{{$.Page.Paginater.Current}}">
-										<button class="btn interact-bg tw-p-2" title="{{ctx.Locale.Tr "notification.mark_as_unread"}}"
-											data-url="{{AppSubUrl}}/notifications/status"
-											data-status="unread"
-											data-page="{{$.Page.Paginater.Current}}"
-											data-notification-id="{{.ID}}"
-											data-q="{{$.Keyword}}">
+						{{else if eq $one.Status $statusRead}}
+							<button class="btn interact-bg tw-p-2" data-tooltip-content="{{ctx.Locale.Tr "notification.mark_as_unread"}}"
+											name="notification_action" value="mark_as_unread"
+							>
 								{{svg "octicon-bell"}}
 							</button>
-									</form>
 						{{end}}
+					</form>
 				</div>
-						</div>
-					{{end}}
+			{{else}}
+				<div class="empty-placeholder">
+					{{svg "octicon-inbox" 56 "tw-mb-4"}}
+					{{if $pageTypeIsRead}}
+						{{ctx.Locale.Tr "notification.no_read"}}
+					{{else}}
+						{{ctx.Locale.Tr "notification.no_unread"}}
 					{{end}}
 				</div>
+			{{end}}
 		</div>
 		{{template "base/paginate" .}}
 	</div>

web_src/css/user.css

@@ -114,6 +114,14 @@
   border-radius: var(--border-radius);
 }
 
+.notifications-item {
+  display: flex;
+  align-items: center;
+  flex-wrap: wrap;
+  gap: 0.5em;
+  padding: 0.5em 1em;
+}
+
 .notifications-item:hover {
   background: var(--color-hover);
 }
@@ -129,6 +137,9 @@
 
 .notifications-item:hover .notifications-buttons {
   display: flex;
+  align-items: center;
+  justify-content: end;
+  gap: 0.25em;
 }
 
 .notifications-item:hover .notifications-updated {

web_src/js/features/notification.ts

@@ -1,40 +1,13 @@
 import {GET} from '../modules/fetch.ts';
-import {toggleElem, type DOMEvent, createElementFromHTML} from '../utils/dom.ts';
+import {toggleElem, createElementFromHTML} from '../utils/dom.ts';
 import {logoutFromWorker} from '../modules/worker.ts';
 
 const {appSubUrl, notificationSettings, assetVersionEncoded} = window.config;
 let notificationSequenceNumber = 0;
 
-export function initNotificationsTable() {
-  const table = document.querySelector('#notification_table');
-  if (!table) return;
-
-  // when page restores from bfcache, delete previously clicked items
-  window.addEventListener('pageshow', (e) => {
-    if (e.persisted) { // page was restored from bfcache
-      const table = document.querySelector('#notification_table');
-      const unreadCountEl = document.querySelector<HTMLElement>('.notifications-unread-count');
-      let unreadCount = parseInt(unreadCountEl.textContent);
-      for (const item of table.querySelectorAll('.notifications-item[data-remove="true"]')) {
-        item.remove();
-        unreadCount -= 1;
-      }
-      unreadCountEl.textContent = String(unreadCount);
-    }
-  });
-
-  // mark clicked unread links for deletion on bfcache restore
-  for (const link of table.querySelectorAll<HTMLAnchorElement>('.notifications-item[data-status="1"] .notifications-link')) {
-    link.addEventListener('click', (e: DOMEvent<MouseEvent>) => {
-      e.target.closest('.notifications-item').setAttribute('data-remove', 'true');
-    });
-  }
-}
-
-async function receiveUpdateCount(event: MessageEvent) {
+async function receiveUpdateCount(event: MessageEvent<{type: string, data: string}>) {
   try {
-    const data = JSON.parse(event.data);
-
+    const data = JSON.parse(event.data.data);
     for (const count of document.querySelectorAll('.notification_count')) {
       count.classList.toggle('tw-hidden', data.Count === 0);
       count.textContent = `${data.Count}`;
@@ -71,7 +44,7 @@ export function initNotificationCount() {
       type: 'start',
       url: `${window.location.origin}${appSubUrl}/user/events`,
     });
-    worker.port.addEventListener('message', (event: MessageEvent) => {
+    worker.port.addEventListener('message', (event: MessageEvent<{type: string, data: string}>) => {
       if (!event.data || !event.data.type) {
         console.error('unknown worker message event', event);
         return;
@@ -144,11 +117,11 @@ async function updateNotificationCountWithCallback(callback: (timeout: number, n
 }
 
 async function updateNotificationTable() {
-  const notificationDiv = document.querySelector('#notification_div');
+  let notificationDiv = document.querySelector('#notification_div');
   if (notificationDiv) {
     try {
       const params = new URLSearchParams(window.location.search);
-      params.set('div-only', String(true));
+      params.set('div-only', 'true');
       params.set('sequence-number', String(++notificationSequenceNumber));
       const response = await GET(`${appSubUrl}/notifications?${params.toString()}`);
 
@@ -160,7 +133,8 @@ async function updateNotificationTable() {
       const el = createElementFromHTML(data);
       if (parseInt(el.getAttribute('data-sequence-number')) === notificationSequenceNumber) {
         notificationDiv.outerHTML = data;
-        initNotificationsTable();
+        notificationDiv = document.querySelector('#notification_div');
+        window.htmx.process(notificationDiv); // when using htmx, we must always remember to process the new content changed by us
       }
     } catch (error) {
       console.error(error);

web_src/js/index-domready.ts

@@ -15,7 +15,7 @@ import {initTableSort} from './features/tablesort.ts';
 import {initAdminUserListSearchForm} from './features/admin/users.ts';
 import {initAdminConfigs} from './features/admin/config.ts';
 import {initMarkupAnchors} from './markup/anchors.ts';
-import {initNotificationCount, initNotificationsTable} from './features/notification.ts';
+import {initNotificationCount} from './features/notification.ts';
 import {initRepoIssueContentHistory} from './features/repo-issue-content.ts';
 import {initStopwatch} from './features/stopwatch.ts';
 import {initFindFileInRepo} from './features/repo-findfile.ts';
@@ -117,7 +117,6 @@ const initPerformanceTracer = callInitFunctions([
   initDashboardRepoList,
 
   initNotificationCount,
-  initNotificationsTable,
 
   initOrgTeam,