From 516a4883fa323eb2130becd8944046cc4e2f116e Mon Sep 17 00:00:00 2001 From: silverwind Date: Sat, 3 Oct 2026 18:35:54 +0200 Subject: [PATCH] enhance(ui): cleanup navbar template and styling (#39554) Clean up the navbar template and styling, and fix the navbar stopwatch, which navigated to the issue instead of opening its popup since https://github.com/go-gitea/gitea/pull/36965. 1. Add hover background to the create and user menus 2. Simplify navbar HTML and CSS and remove Fomantic styles 3. Render the notification and stopwatch icons once instead of separate mobile and desktop copies 4. Make the stopwatch a keyboard accessible button whose popup updates on push and closes when the stopwatch stops Co-authored-by: wxiaoguang Co-authored-by: bircni --- services/context/context_template.go | 1 - templates/base/head_navbar.tmpl | 49 ++------ templates/base/head_navbar_icons.tmpl | 51 +++++++-- tests/e2e/events.test.ts | 22 ++-- tests/e2e/utils.ts | 6 - web_src/css/modules/navbar.css | 103 +++++++---------- web_src/js/features/stopwatch.ts | 156 +++++++++++--------------- web_src/js/globals.d.ts | 1 - web_src/js/vitest.setup.ts | 1 - 9 files changed, 165 insertions(+), 225 deletions(-) diff --git a/services/context/context_template.go b/services/context/context_template.go index 2d990b44add..3109559b7e7 100644 --- a/services/context/context_template.go +++ b/services/context/context_template.go @@ -160,7 +160,6 @@ func (c TemplateContext) WindowConfig() map[string]any { "runModeIsProd": setting.IsProd, "customEmojis": setting.UI.CustomEmojisMap, "pageData": c.parentContext().GetData()["PageData"], - "enableTimeTracking": setting.Service.EnableTimetracking, "mermaidMaxSourceCharacters": setting.MermaidMaxSourceCharacters, "sharedWorkerUri": public.AssetURI("web_src/js/user-events.sharedworker.ts"), "notificationSettings": map[string]any{ diff --git a/templates/base/head_navbar.tmpl b/templates/base/head_navbar.tmpl index 65816badcca..9cdfb5efaef 100644 --- a/templates/base/head_navbar.tmpl +++ b/templates/base/head_navbar.tmpl @@ -1,17 +1,11 @@ {{template "base/head_impersonate_banner"}} diff --git a/templates/base/head_navbar_icons.tmpl b/templates/base/head_navbar_icons.tmpl index b6a2ef1020d..a2f610bf432 100644 --- a/templates/base/head_navbar_icons.tmpl +++ b/templates/base/head_navbar_icons.tmpl @@ -1,16 +1,43 @@ -{{- $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 -}} - {{/* always rendered so a real-time push can reveal it without a reload */}} - -
- {{svg "octicon-stopwatch"}} - +{{- $data := ctx.RootData.PageGlobalData}}{{/* data may not exist, for example: rendering 503 page before the PageGlobalData middleware */}} +{{if and $data $data.IsSigned (not ctx.RootData.MustChangePassword) -}} + {{if EnableTimetracking}} + {{$activeStopwatch := call $data.GetActiveStopwatch -}} + {{/* always rendered so a real-time push can reveal it without a reload */}} + + {{if not $activeStopwatch}}{{$activeStopwatch = dict}}{{end}}{{/* mock an empty stopwatch */}} +
+
+ + {{svg "octicon-issue-opened" 16}} + {{$activeStopwatch.RepoSlug}}#{{$activeStopwatch.IssueIndex}} + +
+
+ +
+
+ +
+
+
- - + {{end}} + + {{$notificationUnreadCount := call $data.GetNotificationUnreadCount -}} +
{{svg "octicon-bell"}} {{$notificationUnreadCount}} diff --git a/tests/e2e/events.test.ts b/tests/e2e/events.test.ts index 2df6ec9f8df..d48bcccc3bc 100644 --- a/tests/e2e/events.test.ts +++ b/tests/e2e/events.test.ts @@ -1,5 +1,5 @@ import {test, expect} from '@playwright/test'; -import {loginUser, baseUrl, apiUserHeaders, apiCreateUser, apiCreateRepo, apiCreateIssue, apiStartStopwatch, apiCancelStopwatch, apiCloseIssue, randomString} from './utils.ts'; +import {loginUser, baseUrl, apiUserHeaders, apiCreateUser, apiCreateRepo, apiCreateIssue, apiStartStopwatch, apiCloseIssue, randomString} from './utils.ts'; // The /-/ws WebSocket pipeline is push-only: every event is fired by the server // immediately on the DB write. These tests exercise that each event type @@ -17,7 +17,7 @@ test.describe('events', () => { loginUser(page, owner), ]); await page.goto('/'); - const badge = page.locator('a.not-mobile .notification_count'); + const badge = page.locator('#navbar .notification_count'); await expect(badge).toBeHidden(); await expect(page.locator('html[data-user-events-connected]')).toBeAttached(); @@ -26,7 +26,7 @@ test.describe('events', () => { await expect(badge).toBeVisible(); }); - test('stopwatch appears and hides via real-time push', async ({page, request}) => { + test('stopwatch appears via real-time push and stops from its popup', async ({page, request}) => { const name = `ev-sw-push-${randomString(8)}`; const headers = apiUserHeaders(name); @@ -40,23 +40,23 @@ test.describe('events', () => { ]); // Page loads before the stopwatch starts — the icon is hidden in the rendered HTML await page.goto('/'); - const stopwatch = page.locator('.active-stopwatch.not-mobile'); + const stopwatch = page.getByTitle('Active Time Tracker'); // Element must exist in the DOM (just hidden); otherwise the push has nothing to reveal. await expect(stopwatch).toHaveCount(1); await expect(stopwatch).toBeHidden(); await expect(page.locator('html[data-user-events-connected]')).toBeAttached(); - // Drive both directions from outside this tab; each push must reach it await apiStartStopwatch(request, name, name, 1, {headers}); await expect(stopwatch).toBeVisible(); - await apiCancelStopwatch(request, name, name, 1, {headers}); + await stopwatch.click(); + await page.getByRole('button', {name: 'Stop Timer'}).click(); await expect(stopwatch).toBeHidden(); }); // Closing an issue stops the timer away from any stopwatch route handler. - test('stopwatch renders when already active and hides when the issue is closed', async ({page, request}) => { + test('stopwatch renders when already active and hides with its popup when the issue is closed', async ({page, request}) => { const name = `ev-sw-close-${randomString(8)}`; const headers = apiUserHeaders(name); @@ -70,12 +70,16 @@ test.describe('events', () => { })(), ]); await page.goto('/'); - const stopwatch = page.locator('.active-stopwatch.not-mobile'); + const stopwatch = page.getByTitle('Active Time Tracker'); await expect(stopwatch).toBeVisible(); await expect(page.locator('html[data-user-events-connected]')).toBeAttached(); + await stopwatch.click(); + const stopButton = page.getByRole('button', {name: 'Stop Timer'}); + await expect(stopButton).toBeVisible(); await apiCloseIssue(request, name, name, 1, {headers}); await expect(stopwatch).toBeHidden(); + await expect(stopButton).toBeHidden(); }); // Repro for https://github.com/go-gitea/gitea/pull/36965#issuecomment-4321282667: @@ -103,7 +107,7 @@ test.describe('events', () => { // neither of these would be present. await expect(page.getByRole('button', {name: 'Stop timer'})).toBeVisible(); await expect(page.getByRole('button', {name: 'Discard timer'})).toBeVisible(); - await expect(page.locator('.active-stopwatch.not-mobile')).toBeVisible(); + await expect(page.getByTitle('Active Time Tracker')).toBeVisible(); }); test('logout propagation', async ({browser, request}) => { diff --git a/tests/e2e/utils.ts b/tests/e2e/utils.ts index 16079f66df6..b742a181624 100644 --- a/tests/e2e/utils.ts +++ b/tests/e2e/utils.ts @@ -84,12 +84,6 @@ export async function apiCreateFiles(requestContext: APIRequestContext, owner: s }), 'apiCreateFiles'); } -export async function apiCancelStopwatch(requestContext: APIRequestContext, owner: string, repo: string, issueIndex: number, {headers}: {headers?: Record} = {}) { - await apiRetry(() => requestContext.delete(`${baseUrl()}/api/v1/repos/${owner}/${repo}/issues/${issueIndex}/stopwatch/delete`, { - headers: headers || apiHeaders(), - }), 'apiCancelStopwatch'); -} - export async function apiCloseIssue(requestContext: APIRequestContext, owner: string, repo: string, issueIndex: number, {headers}: {headers?: Record} = {}) { await apiRetry(() => requestContext.patch(`${baseUrl()}/api/v1/repos/${owner}/${repo}/issues/${issueIndex}`, { headers: headers || apiHeaders(), diff --git a/web_src/css/modules/navbar.css b/web_src/css/modules/navbar.css index eacbdedd3b3..6777743ba32 100644 --- a/web_src/css/modules/navbar.css +++ b/web_src/css/modules/navbar.css @@ -1,28 +1,22 @@ #navbar { - display: flex; - align-items: center; - justify-content: space-between; background: var(--color-nav-bg); border-bottom: 1px solid var(--color-secondary); padding: 0 10px; } -#navbar .navbar-left, -#navbar .navbar-right { +#navbar, +.navbar-list { display: flex; align-items: center; gap: 5px; - min-height: 49px; /* +1px border-bottom */ } -.navbar-left > .item, -.navbar-right > .item, -.navbar-mobile-right > .item { +#navbar > .item, +#navbar > .navbar-list > .item { flex: 0 0 auto; display: flex; align-items: center; color: var(--color-nav-text); - position: relative; text-decoration: none; min-height: 36px; min-width: 36px; @@ -30,81 +24,67 @@ border-radius: 4px; } -#navbar .item.active { +#navbar-logo { + margin: 6.5px 0; /* center the 36px items in the 49px row */ +} + +#navbar > .item.active, +#navbar > .navbar-list > .item.active { background: var(--color-active); } -#navbar :is(a, button).item:not(.dropdown *):hover { +#navbar > .item:hover, +#navbar > .navbar-list > .item:hover { background: var(--color-nav-hover-bg); } -#navbar .item.ui.dropdown { +#navbar > .navbar-list > .item.ui.dropdown { padding-right: 5px; } +#navbar > .navbar-left { + margin-right: auto; +} + @media (max-width: 767.98px) { #navbar { + flex-wrap: wrap; + row-gap: 0; + } + #navbar-logo { + margin-right: auto; + } + #navbar-expand-toggle { + order: 1; + } + .navbar-list { + display: none; + order: 2; + width: 100%; + flex-direction: column; align-items: stretch; } - /* hide all items */ - #navbar .navbar-left > .item, - #navbar .navbar-right > .item { - display: none; + .navbar-left { + margin-top: 5px; } - #navbar #navbar-logo { + .navbar-menu-open .navbar-list { display: flex; } - /* show the first navbar item (logo and its mobile right items) */ - #navbar .navbar-left { - flex: 1; - display: flex; - justify-content: space-between; - } - #navbar .navbar-mobile-right { - display: flex; - align-items: center; - margin: 0 0 0 auto; - width: auto; - } - #navbar .navbar-mobile-right > .item { - display: flex; - width: auto; - } - /* show items if the navbar is open */ #navbar.navbar-menu-open { padding-bottom: 8px; } - #navbar.navbar-menu-open, - #navbar.navbar-menu-open .navbar-right { - flex-direction: column; - } - #navbar.navbar-menu-open .navbar-left { - flex-wrap: wrap; - } - #navbar.navbar-menu-open .navbar-left > .item, - #navbar.navbar-menu-open .navbar-right > .item { - display: flex; - width: 100%; - } - #navbar.navbar-menu-open .navbar-left #navbar-logo { - justify-content: flex-start; - width: auto; - } - #navbar.navbar-menu-open .navbar-left .navbar-mobile-right { - justify-content: flex-end; - min-height: 49px; - } } -#navbar a.item:hover .notification_count, -#navbar a.item:hover .header-stopwatch-dot, +#navbar .item:hover .notification_count, +#navbar .item:hover .header-stopwatch-dot, +#navbar .item:hover .navbar-admin-badge, #navbar .item.active .navbar-admin-badge { border-color: var(--color-nav-hover-bg); } -#navbar a.item .notification_count, -#navbar a.item .header-stopwatch-dot, -#navbar .item .navbar-admin-badge { +#navbar .notification_count, +#navbar .header-stopwatch-dot, +#navbar .navbar-admin-badge { color: var(--color-nav-bg); padding: 0 3.75px; font-size: 12px; @@ -132,8 +112,7 @@ display: inline-flex; } -#navbar .item .navbar-admin-badge { - position: absolute; +#navbar .navbar-admin-badge { left: auto; right: -7px; bottom: -5px; diff --git a/web_src/js/features/stopwatch.ts b/web_src/js/features/stopwatch.ts index 63dabbd3335..2651856420d 100644 --- a/web_src/js/features/stopwatch.ts +++ b/web_src/js/features/stopwatch.ts @@ -1,51 +1,79 @@ import {createTippy} from '../modules/tippy.ts'; import {GET} from '../modules/fetch.ts'; -import {hideElem, queryElems, showElem} from '../utils/dom.ts'; +import {hideElem, showElem} from '../utils/dom.ts'; import {onUserEvent} from '../modules/worker.ts'; import type {StopwatchData} from '../types.ts'; +import {registerGlobalInitFunc} from '../modules/observer.ts'; -const {appSubUrl, notificationSettings, enableTimeTracking} = window.config; - -export function initStopwatch() { - if (!enableTimeTracking) { - return; - } - - const stopwatchEls = document.querySelectorAll('.active-stopwatch'); - const stopwatchPopup = document.querySelector('.active-stopwatch-popup'); - - if (!stopwatchEls.length || !stopwatchPopup) { - return; - } +const {appSubUrl, notificationSettings} = window.config; +export const initStopwatch = () => registerGlobalInitFunc('initActiveStopwatchNotification', (btn: HTMLElement) => { // Init the icon + popup even when no stopwatch is active so a real-time push has a target to toggle. - const seconds = stopwatchEls[0]?.getAttribute('data-seconds'); - if (seconds) { - updateStopwatchTime(parseInt(seconds)); - } + const popup = btn.nextElementSibling!; + const tippy = createTippy(btn, { + content: popup, + placement: 'bottom-end', + trigger: 'click', + maxWidth: 'none', + interactive: true, + hideOnClick: true, + theme: 'default', + }); - for (const stopwatchEl of stopwatchEls) { - createTippy(stopwatchEl, { - content: stopwatchPopup.cloneNode(true) as Element, - placement: 'bottom-end', - trigger: 'click', - maxWidth: 'none', - interactive: true, - hideOnClick: true, - theme: 'default', - onShow(instance) { - // Re-clone on every open so the popup reflects the latest stopwatch state, - // including the case where the icon became visible via a real-time push. - instance.setContent(stopwatchPopup.cloneNode(true) as Element); - }, - }); - } + // TODO: This flickers on page load, we could avoid this by making a custom element to render time periods. + const updateStopwatchTime = (seconds: number) => { + const hours = seconds / 3600 || 0; + const minutes = seconds / 60 || 0; + btn.querySelector('.header-stopwatch-dot')!.textContent = hours >= 1 ? `${Math.round(hours)}h` : `${Math.round(minutes)}m`; + }; + + const updateStopwatchData = (data: Array) => { + const watch = data[0]; + if (!watch) { + tippy.hide(); + hideElem(btn); + return false; + } + const {repo_owner_name, repo_name, issue_index, seconds} = watch; + const issueUrl = `${appSubUrl}/${repo_owner_name}/${repo_name}/issues/${issue_index}`; + popup.querySelector('.stopwatch-link')!.setAttribute('href', issueUrl); + popup.querySelector('.stopwatch-commit')!.setAttribute('action', `${issueUrl}/times/stopwatch/stop`); + popup.querySelector('.stopwatch-cancel')!.setAttribute('action', `${issueUrl}/times/stopwatch/cancel`); + popup.querySelector('.stopwatch-issue')!.textContent = `${repo_owner_name}/${repo_name}#${issue_index}`; + updateStopwatchTime(seconds); + showElem(btn); + return true; + }; + + const updateStopwatch = async () => { + try { + const response = await GET(`${appSubUrl}/user/stopwatches`); + if (!response.ok) { + console.error('Failed to fetch stopwatch data'); + return false; + } + return updateStopwatchData(await response.json()); + } catch (error) { + console.error(error); + return false; + } + }; const startPeriodicPoller = (timeout: number) => { if (timeout <= 0 || !Number.isFinite(timeout)) return; - setTimeout(() => updateStopwatchWithCallback(startPeriodicPoller, timeout), timeout); + setTimeout(async () => { + if (!await updateStopwatch()) { + timeout = notificationSettings.MinTimeout; + } else if (timeout < notificationSettings.MaxTimeout) { + timeout += notificationSettings.TimeoutStep; + } + startPeriodicPoller(timeout); + }, timeout); }; + const seconds = btn.getAttribute('data-seconds'); + if (seconds) updateStopwatchTime(parseInt(seconds)); + let pollerStarted = false; onUserEvent('stopwatches', (msg) => updateStopwatchData(msg.eventData)); // On each (re)connect, reconcile stopwatch state from the server to recover any push dropped during the connect gap. @@ -55,60 +83,4 @@ export function initStopwatch() { pollerStarted = true; startPeriodicPoller(notificationSettings.MinTimeout); }); -} - -async function updateStopwatchWithCallback(callback: (timeout: number) => void, timeout: number) { - const isSet = await updateStopwatch(); - - if (!isSet) { - timeout = notificationSettings.MinTimeout; - } else if (timeout < notificationSettings.MaxTimeout) { - timeout += notificationSettings.TimeoutStep; - } - - callback(timeout); -} - -async function updateStopwatch() { - try { - const response = await GET(`${appSubUrl}/user/stopwatches`); - if (!response.ok) { - console.error('Failed to fetch stopwatch data'); - return false; - } - const data = await response.json(); - return updateStopwatchData(data); - } catch (error) { - console.error(error); - return false; - } -} - -function updateStopwatchData(data: Array) { - if (!data) return; - const watch = data[0]; - const btnEls = document.querySelectorAll('.active-stopwatch'); - if (!watch) { - hideElem(btnEls); - } else { - const {repo_owner_name, repo_name, issue_index, seconds} = watch; - const issueUrl = `${appSubUrl}/${repo_owner_name}/${repo_name}/issues/${issue_index}`; - for (const btnEl of btnEls) btnEl.setAttribute('href', issueUrl); - document.querySelector('.stopwatch-link')?.setAttribute('href', issueUrl); - document.querySelector('.stopwatch-commit')?.setAttribute('action', `${issueUrl}/times/stopwatch/stop`); - document.querySelector('.stopwatch-cancel')?.setAttribute('action', `${issueUrl}/times/stopwatch/cancel`); - const stopwatchIssue = document.querySelector('.stopwatch-issue'); - if (stopwatchIssue) stopwatchIssue.textContent = `${repo_owner_name}/${repo_name}#${issue_index}`; - updateStopwatchTime(seconds); - showElem(btnEls); - } - return Boolean(data.length); -} - -// TODO: This flickers on page load, we could avoid this by making a custom element to render time periods. -function updateStopwatchTime(seconds: number) { - const hours = seconds / 3600 || 0; - const minutes = seconds / 60 || 0; - const timeText = hours >= 1 ? `${Math.round(hours)}h` : `${Math.round(minutes)}m`; - queryElems(document, '.header-stopwatch-dot', (el) => el.textContent = timeText); -} +}); diff --git a/web_src/js/globals.d.ts b/web_src/js/globals.d.ts index 618aeb90d59..504c75e6feb 100644 --- a/web_src/js/globals.d.ts +++ b/web_src/js/globals.d.ts @@ -53,7 +53,6 @@ interface Window { TimeoutStep: number, MaxTimeout: number, }, - enableTimeTracking: boolean, mermaidMaxSourceCharacters: number, i18n: Record, frontendInited: boolean, diff --git a/web_src/js/vitest.setup.ts b/web_src/js/vitest.setup.ts index 91e819f7a5c..116d5c5e775 100644 --- a/web_src/js/vitest.setup.ts +++ b/web_src/js/vitest.setup.ts @@ -9,7 +9,6 @@ window.config = { customEmojis: {}, pageData: {}, notificationSettings: {MinTimeout: 0, TimeoutStep: 0, MaxTimeout: 0}, - enableTimeTracking: true, mermaidMaxSourceCharacters: 5000, i18n: {}, frontendInited: false,