Refactor head navbar icons (#34922)

Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: Giteabot <teabot@gitea.io>

routers/common/pagetmpl.go

@@ -6,6 +6,7 @@ package common
 import (
 	goctx "context"
 	"errors"
+	"sync"
 
 	activities_model "code.gitea.io/gitea/models/activities"
 	"code.gitea.io/gitea/models/db"
@@ -22,8 +23,7 @@ type StopwatchTmplInfo struct {
 	Seconds    int64
 }
 
-func getActiveStopwatch(goCtx goctx.Context) *StopwatchTmplInfo {
+func getActiveStopwatch(ctx *context.Context) *StopwatchTmplInfo {
-	ctx := context.GetWebContext(goCtx)
 	if ctx.Doer == nil {
 		return nil
 	}
@@ -48,8 +48,7 @@ func getActiveStopwatch(goCtx goctx.Context) *StopwatchTmplInfo {
 	}
 }
 
-func notificationUnreadCount(goCtx goctx.Context) int64 {
+func notificationUnreadCount(ctx *context.Context) int64 {
-	ctx := context.GetWebContext(goCtx)
 	if ctx.Doer == nil {
 		return 0
 	}
@@ -66,10 +65,19 @@ func notificationUnreadCount(goCtx goctx.Context) int64 {
 	return count
 }
 
-func PageTmplFunctions(ctx *context.Context) {
+type pageGlobalDataType struct {
-	if ctx.IsSigned {
+	IsSigned    bool
-		// defer the function call to the last moment when the tmpl renders
+	IsSiteAdmin bool
-		ctx.Data["NotificationUnreadCount"] = notificationUnreadCount
+
-		ctx.Data["GetActiveStopwatch"] = getActiveStopwatch
+	GetNotificationUnreadCount func() int64
-	}
+	GetActiveStopwatch         func() *StopwatchTmplInfo
+}
+
+func PageGlobalData(ctx *context.Context) {
+	var data pageGlobalDataType
+	data.IsSigned = ctx.Doer != nil
+	data.IsSiteAdmin = ctx.Doer != nil && ctx.Doer.IsAdmin
+	data.GetNotificationUnreadCount = sync.OnceValue(func() int64 { return notificationUnreadCount(ctx) })
+	data.GetActiveStopwatch = sync.OnceValue(func() *StopwatchTmplInfo { return getActiveStopwatch(ctx) })
+	ctx.Data["PageGlobalData"] = data
 }

routers/web/web.go

@@ -281,7 +281,7 @@ func Routes() *web.Router {
 	}
 
 	mid = append(mid, goGet)
-	mid = append(mid, common.PageTmplFunctions)
+	mid = append(mid, common.PageGlobalData)
 
 	webRoutes := web.NewRouter()
 	webRoutes.Use(mid...)

templates/base/head_navbar.tmpl

@@ -1,11 +1,3 @@
-{{$notificationUnreadCount := 0}}
-{{if and .IsSigned .NotificationUnreadCount}}
-	{{$notificationUnreadCount = call .NotificationUnreadCount ctx}}
-{{end}}
-{{$activeStopwatch := NIL}}
-{{if and .IsSigned EnableTimetracking .GetActiveStopwatch}}
-	{{$activeStopwatch = call .GetActiveStopwatch ctx}}
-{{end}}
 <nav id="navbar" aria-label="{{ctx.Locale.Tr "aria.navbar"}}">
 	<div class="navbar-left">
 		<!-- the logo -->
@@ -15,22 +7,7 @@
 
 		<!-- mobile right menu, it must be here because in mobile view, each item is a flex column, the first item is a full row column -->
 		<div class="ui secondary menu navbar-mobile-right only-mobile">
-			{{if $activeStopwatch}}
+			{{template "base/head_navbar_icons" dict "PageGlobalData" .PageGlobalData}}
-			<a id="mobile-stopwatch-icon" class="active-stopwatch item" href="{{$activeStopwatch.IssueLink}}" title="{{ctx.Locale.Tr "active_stopwatch"}}" data-seconds="{{$activeStopwatch.Seconds}}">
-				<div class="tw-relative">
-					{{svg "octicon-stopwatch"}}
-					<span class="header-stopwatch-dot"></span>
-				</div>
-			</a>
-			{{end}}
-			{{if .IsSigned}}
-			<a id="mobile-notifications-icon" class="item" href="{{AppSubUrl}}/notifications" data-tooltip-content="{{ctx.Locale.Tr "notifications"}}" aria-label="{{ctx.Locale.Tr "notifications"}}">
-				<div class="tw-relative">
-					{{svg "octicon-bell"}}
-					<span class="notification_count{{if not $notificationUnreadCount}} tw-hidden{{end}}">{{$notificationUnreadCount}}</span>
-				</div>
-			</a>
-			{{end}}
 			<button class="item ui icon mini button tw-m-0" id="navbar-expand-toggle" aria-label="{{ctx.Locale.Tr "home.nav_menu"}}">{{svg "octicon-three-bars"}}</button>
 		</div>
 
@@ -85,22 +62,7 @@
 				</div><!-- end content avatar menu -->
 			</div><!-- end dropdown avatar menu -->
 		{{else if .IsSigned}}
-			{{if $activeStopwatch}}
+			{{template "base/head_navbar_icons" dict "ItemExtraClass" "not-mobile" "PageGlobalData" .PageGlobalData}}
-			<a class="item not-mobile active-stopwatch" href="{{$activeStopwatch.IssueLink}}" title="{{ctx.Locale.Tr "active_stopwatch"}}" data-seconds="{{$activeStopwatch.Seconds}}">
-				<div class="tw-relative">
-					{{svg "octicon-stopwatch"}}
-					<span class="header-stopwatch-dot"></span>
-				</div>
-			</a>
-			{{end}}
-
-			<a class="item not-mobile" href="{{AppSubUrl}}/notifications" data-tooltip-content="{{ctx.Locale.Tr "notifications"}}" aria-label="{{ctx.Locale.Tr "notifications"}}">
-				<div class="tw-relative">
-					{{svg "octicon-bell"}}
-					<span class="notification_count{{if not $notificationUnreadCount}} tw-hidden{{end}}">{{$notificationUnreadCount}}</span>
-				</div>
-			</a>
-
 			<div class="ui dropdown jump item" data-tooltip-content="{{ctx.Locale.Tr "create_new"}}">
 				<span class="text">
 					{{svg "octicon-plus"}}
@@ -130,8 +92,6 @@
 					<span class="only-mobile">{{.SignedUser.Name}}</span>
 					<span class="not-mobile">{{svg "octicon-triangle-down"}}</span>
 				</span>
-				{{/* do not localize it, here it needs the fixed length (width) to make UI comfortable */}}
-				{{if .IsAdmin}}<span class="navbar-profile-admin">admin</span>{{end}}
 				<div class="menu user-menu">
 					<div class="header">
 						{{ctx.Locale.Tr "signed_in_as"}} <strong>{{.SignedUser.Name}}</strong>
@@ -160,14 +120,6 @@
 						{{svg "octicon-question"}}
 						{{ctx.Locale.Tr "help"}}
 					</a>
-					{{if .IsAdmin}}
-						<div class="divider"></div>
-						<a class="{{if .PageIsAdmin}}active {{end}}item" href="{{AppSubUrl}}/-/admin">
-							{{svg "octicon-server"}}
-							{{ctx.Locale.Tr "admin_panel"}}
-						</a>
-					{{end}}
-
 					<div class="divider"></div>
 					<a class="item link-action" href data-url="{{AppSubUrl}}/user/logout">
 						{{svg "octicon-sign-out"}}
@@ -189,6 +141,7 @@
 		{{end}}
 	</div><!-- end full right menu -->
 
+	{{$activeStopwatch := and .PageGlobalData (call .PageGlobalData.GetActiveStopwatch)}}
 	{{if $activeStopwatch}}
 		<div class="active-stopwatch-popup tippy-target">
 			<div class="tw-flex tw-items-center tw-gap-2 tw-p-3">

templates/base/head_navbar_icons.tmpl

@@ -0,0 +1,25 @@
+{{- $itemExtraClass := .ItemExtraClass -}}
+{{- $data := .PageGlobalData -}}
+{{if and $data $data.IsSigned}}{{/* data may not exist, for example: rendering 503 page before the PageGlobalData middleware */}}
+	{{- $activeStopwatch := call $data.GetActiveStopwatch -}}
+	{{- $notificationUnreadCount := call $data.GetNotificationUnreadCount -}}
+	{{if $activeStopwatch}}
+	<a class="item active-stopwatch {{$itemExtraClass}}" href="{{$activeStopwatch.IssueLink}}" title="{{ctx.Locale.Tr "active_stopwatch"}}" data-seconds="{{$activeStopwatch.Seconds}}">
+		<div class="tw-relative">
+			{{svg "octicon-stopwatch"}}
+			<span class="header-stopwatch-dot"></span>
+		</div>
+	</a>
+	{{end}}
+	<a class="item {{$itemExtraClass}}" href="{{AppSubUrl}}/notifications" data-tooltip-content="{{ctx.Locale.Tr "notifications"}}">
+		<div class="tw-relative">
+			{{svg "octicon-bell"}}
+			<span class="notification_count{{if not $notificationUnreadCount}} tw-hidden{{end}}">{{$notificationUnreadCount}}</span>
+		</div>
+	</a>
+	{{if $data.IsSiteAdmin}}
+	<a class="item {{$itemExtraClass}}" href="{{AppSubUrl}}/-/admin" data-tooltip-content="{{ctx.Locale.Tr "admin_panel"}}">
+		{{svg "octicon-server"}}
+	</a>
+	{{end}}
+{{end}}

templates/swagger/ui.tmpl

@@ -7,6 +7,7 @@
 	<body>
 		<a class="swagger-back-link" href="{{AppSubUrl}}/">{{svg "octicon-reply"}}{{ctx.Locale.Tr "return_to_gitea"}}</a>
 		<div id="swagger-ui" data-source="{{AppSubUrl}}/swagger.{{.APIJSONVersion}}.json"></div>
+		<footer class="page-footer"></footer>
 		<script src="{{AssetUrlPrefix}}/js/swagger.js?v={{AssetVersion}}"></script>
 	</body>
 </html>

templates/user/notification/notification_div.tmpl

@@ -1,6 +1,6 @@
 <div role="main" aria-label="{{.Title}}" class="page-content user notification" id="notification_div" data-sequence-number="{{.SequenceNumber}}">
 	<div class="ui container">
-		{{$notificationUnreadCount := call .NotificationUnreadCount ctx}}
+		{{$notificationUnreadCount := call .PageGlobalData.GetNotificationUnreadCount}}
 		<div class="tw-flex tw-items-center 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">

tests/integration/integration_test.go

@@ -148,6 +148,9 @@ func (s *TestSession) GetCookieFlashMessage() *middleware.Flash {
 
 func (s *TestSession) MakeRequest(t testing.TB, rw *RequestWrapper, expectedStatus int) *httptest.ResponseRecorder {
 	t.Helper()
+	if s == nil {
+		return MakeRequest(t, rw, expectedStatus)
+	}
 	req := rw.Request
 	baseURL, err := url.Parse(setting.AppURL)
 	assert.NoError(t, err)

tests/integration/links_test.go

@@ -17,38 +17,48 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestLinksNoLogin(t *testing.T) {
+func assertLinkPageComplete(t *testing.T, session *TestSession, link string) {
+	req := NewRequest(t, "GET", link)
+	resp := session.MakeRequest(t, req, http.StatusOK)
+	assert.True(t, test.IsNormalPageCompleted(resp.Body.String()), "Page did not complete: "+link)
+}
+
+func TestLinks(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 
+	t.Run("NoLogin", testLinksNoLogin)
+	t.Run("RedirectsNoLogin", testLinksRedirectsNoLogin)
+	t.Run("NoLoginNotExist", testLinksNoLoginNotExist)
+	t.Run("AsUser", testLinksAsUser)
+	t.Run("RepoCommon", testLinksRepoCommon)
+}
+
+func testLinksNoLogin(t *testing.T) {
 	links := []string{
+		"/",
 		"/explore/repos",
 		"/explore/repos?q=test",
 		"/explore/users",
 		"/explore/users?q=test",
 		"/explore/organizations",
 		"/explore/organizations?q=test",
-		"/",
 		"/user/sign_up",
 		"/user/login",
 		"/user/forgot_password",
-		"/api/swagger",
 		"/user2/repo1",
 		"/user2/repo1/",
 		"/user2/repo1/projects",
 		"/user2/repo1/projects/1",
 		"/user2/repo1/releases/tag/delete-tag", // It's the only one existing record on release.yml which has is_tag: true
-		"/.well-known/security.txt",
+		"/api/swagger",
 	}
-
 	for _, link := range links {
-		req := NewRequest(t, "GET", link)
+		assertLinkPageComplete(t, nil, link)
-		MakeRequest(t, req, http.StatusOK)
 	}
+	MakeRequest(t, NewRequest(t, "GET", "/.well-known/security.txt"), http.StatusOK)
 }
 
-func TestRedirectsNoLogin(t *testing.T) {
+func testLinksRedirectsNoLogin(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
-
 	redirects := []struct{ from, to string }{
 		{"/user2/repo1/commits/master", "/user2/repo1/commits/branch/master"},
 		{"/user2/repo1/src/master", "/user2/repo1/src/branch/master"},
@@ -68,9 +78,7 @@ func TestRedirectsNoLogin(t *testing.T) {
 	}
 }
 
-func TestNoLoginNotExist(t *testing.T) {
+func testLinksNoLoginNotExist(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
-
 	links := []string{
 		"/user5/repo4/projects",
 		"/user5/repo4/projects/3",
@@ -82,7 +90,8 @@ func TestNoLoginNotExist(t *testing.T) {
 	}
 }
 
-func testLinksAsUser(userName string, t *testing.T) {
+func testLinksAsUser(t *testing.T) {
+	session := loginUser(t, "user2")
 	links := []string{
 		"/explore/repos",
 		"/explore/repos?q=test",
@@ -130,18 +139,14 @@ func testLinksAsUser(userName string, t *testing.T) {
 		"/user/settings/repos",
 	}
 
-	session := loginUser(t, userName)
 	for _, link := range links {
-		req := NewRequest(t, "GET", link)
+		assertLinkPageComplete(t, session, link)
-		session.MakeRequest(t, req, http.StatusOK)
 	}
 
-	reqAPI := NewRequestf(t, "GET", "/api/v1/users/%s/repos", userName)
+	reqAPI := NewRequestf(t, "GET", "/api/v1/users/user2/repos")
 	respAPI := MakeRequest(t, reqAPI, http.StatusOK)
-
 	var apiRepos []*api.Repository
 	DecodeJSON(t, respAPI, &apiRepos)
-
 	repoLinks := []string{
 		"",
 		"/issues",
@@ -164,24 +169,15 @@ func testLinksAsUser(userName string, t *testing.T) {
 		"/wiki/?action=_new",
 		"/activity",
 	}
-
 	for _, repo := range apiRepos {
 		for _, link := range repoLinks {
-			req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s%s", userName, repo.Name, link))
+			link = fmt.Sprintf("/user2/%s%s", repo.Name, link)
-			session.MakeRequest(t, req, http.StatusOK)
+			assertLinkPageComplete(t, session, link)
 		}
 	}
 }
 
-func TestLinksLogin(t *testing.T) {
+func testLinksRepoCommon(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
-
-	testLinksAsUser("user2", t)
-}
-
-func TestRepoLinks(t *testing.T) {
-	defer tests.PrepareTestEnv(t)()
-
 	// repo1 has enabled almost features, so we can test most links
 	repoLink := "/user2/repo1"
 	links := []string{
@@ -192,21 +188,18 @@ func TestRepoLinks(t *testing.T) {
 
 	// anonymous user
 	for _, link := range links {
-		req := NewRequest(t, "GET", repoLink+link)
+		assertLinkPageComplete(t, nil, repoLink+link)
-		MakeRequest(t, req, http.StatusOK)
 	}
 
 	// admin/owner user
 	session := loginUser(t, "user1")
 	for _, link := range links {
-		req := NewRequest(t, "GET", repoLink+link)
+		assertLinkPageComplete(t, session, repoLink+link)
-		session.MakeRequest(t, req, http.StatusOK)
 	}
 
 	// non-admin non-owner user
 	session = loginUser(t, "user2")
 	for _, link := range links {
-		req := NewRequest(t, "GET", repoLink+link)
+		assertLinkPageComplete(t, session, repoLink+link)
-		session.MakeRequest(t, req, http.StatusOK)
 	}
 }

web_src/css/modules/navbar.css

@@ -101,19 +101,6 @@
   }
 }
 
-#navbar .ui.dropdown .navbar-profile-admin {
-  display: block;
-  position: absolute;
-  font-size: 9px;
-  font-weight: var(--font-weight-bold);
-  color: var(--color-nav-bg);
-  background: var(--color-primary);
-  padding: 2px 3px;
-  border-radius: 10px;
-  top: -1px;
-  left: 18px;
-}
-
 #navbar a.item:hover .notification_count,
 #navbar a.item:hover .header-stopwatch-dot {
   border-color: var(--color-nav-hover-bg);