From 564102ce89f53d6bd2fdbaa33416e4287d6fe9a8 Mon Sep 17 00:00:00 2001 From: silverwind Date: Tue, 30 Apr 2024 16:52:46 +0200 Subject: [PATCH] Rework and fix stopwatch (#30732) Fixes https://github.com/go-gitea/gitea/issues/30721 and overhauls the stopwatch. Time is now shown inside the "dot" icon and on both mobile and desktop. All rendering is now done by ``, the `pretty-ms` dependency is dropped. Desktop: Screenshot 2024-04-29 at 22 33 27 Mobile: Screenshot 2024-04-29 at 22 34 19 Note for tippy: Previously, tippy instances defaulted to "menu" theme, but that theme is really only meant for `.ui.menu`, so it was not optimal for the stopwatch popover. This introduces a unopinionated `default` theme that has no padding and should be suitable for all content. I reviewed all existing uses and explicitely set the desired `theme` on all of them. --- package-lock.json | 26 ------- package.json | 1 - templates/base/head_navbar.tmpl | 69 ++++++++++--------- web_src/css/modules/navbar.css | 16 ++--- web_src/css/modules/tippy.css | 7 +- web_src/js/features/contextpopup.js | 2 + web_src/js/features/repo-code.js | 1 + web_src/js/features/repo-issue.js | 1 + web_src/js/features/stopwatch.js | 82 +++++++++++------------ web_src/js/modules/tippy.js | 6 +- web_src/js/webcomponents/overflow-menu.js | 1 + 11 files changed, 99 insertions(+), 113 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8e4eeb7fb..917ff1029 100644 --- a/package-lock.json +++ b/package-lock.json @@ -42,7 +42,6 @@ "postcss": "8.4.38", "postcss-loader": "8.1.1", "postcss-nesting": "12.1.2", - "pretty-ms": "9.0.0", "sortablejs": "1.15.2", "swagger-ui-dist": "5.17.2", "tailwindcss": "3.4.3", @@ -9170,17 +9169,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/parse-ms": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/parse-ms/-/parse-ms-4.0.0.tgz", - "integrity": "sha512-TXfryirbmq34y8QBwgqCVLi+8oA3oWx2eAnSn62ITyEhEYaWRlVZ2DvMM9eZbMs/RfxPu/PK/aBLyGj4IrqMHw==", - "engines": { - "node": ">=18" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, "node_modules/path-exists": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/path-exists/-/path-exists-4.0.0.tgz", @@ -9772,20 +9760,6 @@ "url": "https://github.com/chalk/ansi-styles?sponsor=1" } }, - "node_modules/pretty-ms": { - "version": "9.0.0", - "resolved": "https://registry.npmjs.org/pretty-ms/-/pretty-ms-9.0.0.tgz", - "integrity": "sha512-E9e9HJ9R9NasGOgPaPE8VMeiPKAyWR5jcFpNnwIejslIhWqdqOrb2wShBsncMPUb+BcCd2OPYfh7p2W6oemTng==", - "dependencies": { - "parse-ms": "^4.0.0" - }, - "engines": { - "node": ">=18" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, "node_modules/printable-characters": { "version": "1.0.42", "resolved": "https://registry.npmjs.org/printable-characters/-/printable-characters-1.0.42.tgz", diff --git a/package.json b/package.json index 142b9bb3e..5f9b81032 100644 --- a/package.json +++ b/package.json @@ -41,7 +41,6 @@ "postcss": "8.4.38", "postcss-loader": "8.1.1", "postcss-nesting": "12.1.2", - "pretty-ms": "9.0.0", "sortablejs": "1.15.2", "swagger-ui-dist": "5.17.2", "tailwindcss": "3.4.3", diff --git a/templates/base/head_navbar.tmpl b/templates/base/head_navbar.tmpl index addff22c4..7a3e663c4 100644 --- a/templates/base/head_navbar.tmpl +++ b/templates/base/head_navbar.tmpl @@ -12,6 +12,14 @@
+ {{if and .IsSigned EnableTimetracking .ActiveStopwatch}} + +
+ {{svg "octicon-stopwatch"}} + +
+ + {{end}} {{if .IsSigned}}
@@ -74,41 +82,13 @@
{{else if .IsSigned}} - {{if EnableTimetracking}} - + {{if and EnableTimetracking .ActiveStopwatch}} +
{{svg "octicon-stopwatch"}}
- {{ctx.Locale.Tr "active_stopwatch"}} -
-
- - {{svg "octicon-issue-opened" 16 "tw-mr-2"}} - {{.ActiveStopwatch.RepoSlug}}#{{.ActiveStopwatch.IssueIndex}} - - {{if .ActiveStopwatch}}{{Sec2Time .ActiveStopwatch.Seconds}}{{end}} - - -
- {{.CsrfTokenHtml}} - -
-
- {{.CsrfTokenHtml}} - -
-
-
{{end}} @@ -202,4 +182,33 @@ {{end}} + + {{if and .IsSigned EnableTimetracking .ActiveStopwatch}} +
+
+ + {{svg "octicon-issue-opened" 16}} + {{.ActiveStopwatch.RepoSlug}}#{{.ActiveStopwatch.IssueIndex}} + +
+
+ {{.CsrfTokenHtml}} + +
+
+ {{.CsrfTokenHtml}} + +
+
+
+
+ {{end}} diff --git a/web_src/css/modules/navbar.css b/web_src/css/modules/navbar.css index d7aa197e0..848f9331d 100644 --- a/web_src/css/modules/navbar.css +++ b/web_src/css/modules/navbar.css @@ -103,19 +103,12 @@ width: 50%; min-height: 48px; } + #navbar #mobile-stopwatch-icon, #navbar #mobile-notifications-icon { margin-right: 6px !important; } } -#navbar a.item .notification_count { - color: var(--color-nav-bg); - padding: 0 3.75px; - font-size: 12px; - line-height: 12px; - font-weight: var(--font-weight-bold); -} - #navbar a.item:hover .notification_count, #navbar a.item:hover .header-stopwatch-dot { border-color: var(--color-nav-hover-bg); @@ -123,6 +116,11 @@ #navbar a.item .notification_count, #navbar a.item .header-stopwatch-dot { + color: var(--color-nav-bg); + padding: 0 3.75px; + font-size: 12px; + line-height: 12px; + font-weight: var(--font-weight-bold); background: var(--color-primary); border: 2px solid var(--color-nav-bg); position: absolute; @@ -135,6 +133,8 @@ align-items: center; justify-content: center; z-index: 1; /* prevent menu button background from overlaying icon */ + user-select: none; + white-space: nowrap; } .secondary-nav { diff --git a/web_src/css/modules/tippy.css b/web_src/css/modules/tippy.css index 6ac7c37d9..53c3d5aae 100644 --- a/web_src/css/modules/tippy.css +++ b/web_src/css/modules/tippy.css @@ -16,8 +16,8 @@ .tippy-box { position: relative; - background-color: var(--color-body); - color: var(--color-secondary-dark-6); + background-color: var(--color-menu); + color: var(--color-text); border: 1px solid var(--color-secondary); border-radius: var(--border-radius); font-size: 1rem; @@ -25,7 +25,6 @@ .tippy-content { position: relative; - padding: 1rem; /* if you need different padding, use different data-theme */ z-index: 1; } @@ -166,5 +165,5 @@ } .tippy-svg-arrow-inner { - fill: var(--color-body); + fill: var(--color-menu); } diff --git a/web_src/js/features/contextpopup.js b/web_src/js/features/contextpopup.js index ce90f3e50..6a9325ed1 100644 --- a/web_src/js/features/contextpopup.js +++ b/web_src/js/features/contextpopup.js @@ -18,6 +18,7 @@ export function attachRefIssueContextPopup(refIssues) { if (!owner) return; const el = document.createElement('div'); + el.classList.add('tw-p-3'); refIssue.parentNode.insertBefore(el, refIssue.nextSibling); const view = createApp(ContextPopup); @@ -30,6 +31,7 @@ export function attachRefIssueContextPopup(refIssues) { } createTippy(refIssue, { + theme: 'default', content: el, placement: 'top-start', interactive: true, diff --git a/web_src/js/features/repo-code.js b/web_src/js/features/repo-code.js index 63da5f203..7c74c253a 100644 --- a/web_src/js/features/repo-code.js +++ b/web_src/js/features/repo-code.js @@ -113,6 +113,7 @@ function showLineButton() { btn.closest('.code-view').append(menu.cloneNode(true)); createTippy(btn, { + theme: 'menu', trigger: 'click', hideOnClick: true, content: menu, diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index 2b2eed58b..c4e14c62c 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -502,6 +502,7 @@ export function initRepoPullRequestReview() { if ($reviewBtn.length && $panel.length) { const tippy = createTippy($reviewBtn[0], { content: $panel[0], + theme: 'default', placement: 'bottom', trigger: 'click', maxWidth: 'none', diff --git a/web_src/js/features/stopwatch.js b/web_src/js/features/stopwatch.js index 2ec74344f..bcea26bd6 100644 --- a/web_src/js/features/stopwatch.js +++ b/web_src/js/features/stopwatch.js @@ -1,4 +1,3 @@ -import prettyMilliseconds from 'pretty-ms'; import {createTippy} from '../modules/tippy.js'; import {GET} from '../modules/fetch.js'; import {hideElem, showElem} from '../utils/dom.js'; @@ -10,28 +9,31 @@ export function initStopwatch() { return; } - const stopwatchEl = document.querySelector('.active-stopwatch-trigger'); + const stopwatchEls = document.querySelectorAll('.active-stopwatch'); const stopwatchPopup = document.querySelector('.active-stopwatch-popup'); - if (!stopwatchEl || !stopwatchPopup) { + if (!stopwatchEls.length || !stopwatchPopup) { return; } - stopwatchEl.removeAttribute('href'); // intended for noscript mode only - - createTippy(stopwatchEl, { - content: stopwatchPopup, - placement: 'bottom-end', - trigger: 'click', - maxWidth: 'none', - interactive: true, - hideOnClick: true, - }); - // global stop watch (in the head_navbar), it should always work in any case either the EventSource or the PeriodicPoller is used. - const currSeconds = document.querySelector('.stopwatch-time')?.getAttribute('data-seconds'); - if (currSeconds) { - updateStopwatchTime(currSeconds); + const seconds = stopwatchEls[0]?.getAttribute('data-seconds'); + if (seconds) { + updateStopwatchTime(parseInt(seconds)); + } + + for (const stopwatchEl of stopwatchEls) { + stopwatchEl.removeAttribute('href'); // intended for noscript mode only + + createTippy(stopwatchEl, { + content: stopwatchPopup.cloneNode(true), + placement: 'bottom-end', + trigger: 'click', + maxWidth: 'none', + interactive: true, + hideOnClick: true, + theme: 'default', + }); } let usingPeriodicPoller = false; @@ -124,10 +126,9 @@ async function updateStopwatch() { function updateStopwatchData(data) { const watch = data[0]; - const btnEl = document.querySelector('.active-stopwatch-trigger'); + const btnEls = document.querySelectorAll('.active-stopwatch'); if (!watch) { - clearStopwatchTimer(); - hideElem(btnEl); + hideElem(btnEls); } else { const {repo_owner_name, repo_name, issue_index, seconds} = watch; const issueUrl = `${appSubUrl}/${repo_owner_name}/${repo_name}/issues/${issue_index}`; @@ -137,31 +138,28 @@ function updateStopwatchData(data) { const stopwatchIssue = document.querySelector('.stopwatch-issue'); if (stopwatchIssue) stopwatchIssue.textContent = `${repo_owner_name}/${repo_name}#${issue_index}`; updateStopwatchTime(seconds); - showElem(btnEl); + showElem(btnEls); } return Boolean(data.length); } -let updateTimeIntervalId = null; // holds setInterval id when active -function clearStopwatchTimer() { - if (updateTimeIntervalId !== null) { - clearInterval(updateTimeIntervalId); - updateTimeIntervalId = null; +// TODO: This flickers on page load, we could avoid this by making a custom +// element to render time periods. Feeding a datetime in backend does not work +// when time zone between server and client differs. +function updateStopwatchTime(seconds) { + if (!Number.isFinite(seconds)) return; + const datetime = (new Date(Date.now() - seconds * 1000)).toISOString(); + for (const parent of document.querySelectorAll('.header-stopwatch-dot')) { + const existing = parent.querySelector(':scope > relative-time'); + if (existing) { + existing.setAttribute('datetime', datetime); + } else { + const el = document.createElement('relative-time'); + el.setAttribute('format', 'micro'); + el.setAttribute('datetime', datetime); + el.setAttribute('lang', 'en-US'); + el.setAttribute('title', ''); // make show no title and therefor no tooltip + parent.append(el); + } } } -function updateStopwatchTime(seconds) { - const secs = parseInt(seconds); - if (!Number.isFinite(secs)) return; - - clearStopwatchTimer(); - const stopwatch = document.querySelector('.stopwatch-time'); - // TODO: replace with similar to how system status up time is shown - const start = Date.now(); - const updateUi = () => { - const delta = Date.now() - start; - const dur = prettyMilliseconds(secs * 1000 + delta, {compact: true}); - if (stopwatch) stopwatch.textContent = dur; - }; - updateUi(); - updateTimeIntervalId = setInterval(updateUi, 1000); -} diff --git a/web_src/js/modules/tippy.js b/web_src/js/modules/tippy.js index 83b28e574..a18c94caf 100644 --- a/web_src/js/modules/tippy.js +++ b/web_src/js/modules/tippy.js @@ -37,8 +37,10 @@ export function createTippy(target, opts = {}) { return onShow?.(instance); }, arrow: arrow || (theme === 'bare' ? false : arrowSvg), - role: role || 'menu', // HTML role attribute - theme: theme || role || 'menu', // CSS theme, either "tooltip", "menu", "box-with-header" or "bare" + // HTML role attribute, ideally the default role would be "popover" but it does not exist + role: role || 'menu', + // CSS theme, either "default", "tooltip", "menu", "box-with-header" or "bare" + theme: theme || role || 'default', plugins: [followCursor], ...other, }); diff --git a/web_src/js/webcomponents/overflow-menu.js b/web_src/js/webcomponents/overflow-menu.js index 0778c5990..80dd1a545 100644 --- a/web_src/js/webcomponents/overflow-menu.js +++ b/web_src/js/webcomponents/overflow-menu.js @@ -131,6 +131,7 @@ window.customElements.define('overflow-menu', class extends HTMLElement { interactive: true, placement: 'bottom-end', role: 'menu', + theme: 'menu', content: this.tippyContent, onShow: () => { // FIXME: onShown doesn't work (never be called) setTimeout(() => {