From eadb2259a045eb02f0a7229b306fc96465469877 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Thu, 3 Sep 2026 18:42:56 +0100 Subject: [PATCH 1/6] Add @primer/behaviors as a direct dependency --- frontend/package-lock.json | 7 ++++--- frontend/package.json | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/frontend/package-lock.json b/frontend/package-lock.json index d67eac2aa35a..0fd6152d95ae 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -64,6 +64,7 @@ "@openproject/primer-view-components": "^0.91.1", "@openproject/reactivestates": "^3.0.1", "@openproject/stimulus-elements": "^0.2.0", + "@primer/behaviors": "^1.10.3", "@primer/css": "^22.1.0", "@primer/live-region-element": "^0.8.0", "@primer/primitives": "^11.5.1", @@ -5533,9 +5534,9 @@ "license": "MIT" }, "node_modules/@primer/behaviors": { - "version": "1.10.2", - "resolved": "https://registry.npmjs.org/@primer/behaviors/-/behaviors-1.10.2.tgz", - "integrity": "sha512-93juWZbWg2DRhC11+7RT7hMpY1VD3lBosLmccqEZ65yrCHqkBCjI8Uj8wxs3y0U+wWE07LAoLHAPylyWbifg5A==", + "version": "1.10.3", + "resolved": "https://registry.npmjs.org/@primer/behaviors/-/behaviors-1.10.3.tgz", + "integrity": "sha512-cxxWDeR5IE7vdL0Ca0RAAAuPVrU+Bo8CSP1WfawGo4rEP1sZTi7hwl5i7TT0jCTCR0C9IfnQBfSPAkz0oznxUw==", "license": "MIT" }, "node_modules/@primer/css": { diff --git a/frontend/package.json b/frontend/package.json index a04eef1f2d68..d5859f65f919 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -110,6 +110,7 @@ "@openproject/primer-view-components": "^0.91.1", "@openproject/reactivestates": "^3.0.1", "@openproject/stimulus-elements": "^0.2.0", + "@primer/behaviors": "^1.10.3", "@primer/css": "^22.1.0", "@primer/live-region-element": "^0.8.0", "@primer/primitives": "^11.5.1", From 718d77315c90a0dea91f2e2ff03eaaee96456cf2 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Thu, 3 Sep 2026 18:42:57 +0100 Subject: [PATCH 2/6] [DREAM-813] Share the anchored popover caret helpers The time-entry calendar popover and the budget chart tooltips hand-build a Primer Popover-message with a fixed caret that points the wrong way whenever anchored-position flips. Moves the timeline tooltip's caret geometry into a shared module and adds the lit partial and the placement helper the next two commits adopt. The helper runs Primer's own getAnchoredPosition and applies the result at once, since the element only does so in a later frame and never reports which side it chose; the caret is then read off the placed box. https://community.openproject.org/wp/DREAM-813 --- .../anchored-popover/anchored-popover.sass | 20 ++++ .../caret-placement.spec.ts} | 2 +- .../caret-placement.ts} | 0 .../anchored-popover/popover-message.spec.ts | 76 +++++++++++++ .../anchored-popover/popover-message.ts | 49 ++++++++ .../popover-placement.spec.ts | 105 ++++++++++++++++++ .../anchored-popover/popover-placement.ts | 46 ++++++++ .../project-timeline-graph.component.sass | 17 --- .../project-timeline-graph.component.spec.ts | 38 ++++--- .../project-timeline-tooltip.builder.ts | 18 +-- .../project-timeline-tooltip.popover.ts | 16 +-- .../common/openproject-common.module.sass | 1 + 12 files changed, 326 insertions(+), 62 deletions(-) create mode 100644 frontend/src/app/shared/components/anchored-popover/anchored-popover.sass rename frontend/src/app/shared/components/{project-timeline-graph/project-timeline-tooltip-caret.spec.ts => anchored-popover/caret-placement.spec.ts} (97%) rename frontend/src/app/shared/components/{project-timeline-graph/project-timeline-tooltip-caret.ts => anchored-popover/caret-placement.ts} (100%) create mode 100644 frontend/src/app/shared/components/anchored-popover/popover-message.spec.ts create mode 100644 frontend/src/app/shared/components/anchored-popover/popover-message.ts create mode 100644 frontend/src/app/shared/components/anchored-popover/popover-placement.spec.ts create mode 100644 frontend/src/app/shared/components/anchored-popover/popover-placement.ts diff --git a/frontend/src/app/shared/components/anchored-popover/anchored-popover.sass b/frontend/src/app/shared/components/anchored-popover/anchored-popover.sass new file mode 100644 index 000000000000..7424b4aaf83d --- /dev/null +++ b/frontend/src/app/shared/components/anchored-popover/anchored-popover.sass @@ -0,0 +1,20 @@ +anchored-position.op-anchored-popover--host + margin: 0 + padding: 0 + border: 0 + background: none + text-align: start + +.op-anchored-popover + color: var(--fgColor-default) + + &:not(.Popover-message--left):not(.Popover-message--right) + &::before, + &::after + left: var(--op-anchored-popover-caret-offset, 50%) + + &.Popover-message--left, + &.Popover-message--right + &::before, + &::after + top: var(--op-anchored-popover-caret-offset, 50%) diff --git a/frontend/src/app/shared/components/project-timeline-graph/project-timeline-tooltip-caret.spec.ts b/frontend/src/app/shared/components/anchored-popover/caret-placement.spec.ts similarity index 97% rename from frontend/src/app/shared/components/project-timeline-graph/project-timeline-tooltip-caret.spec.ts rename to frontend/src/app/shared/components/anchored-popover/caret-placement.spec.ts index a7a767cc9c18..51df66cf02df 100644 --- a/frontend/src/app/shared/components/project-timeline-graph/project-timeline-tooltip-caret.spec.ts +++ b/frontend/src/app/shared/components/anchored-popover/caret-placement.spec.ts @@ -26,7 +26,7 @@ // See COPYRIGHT and LICENSE files for more details. //++ -import { caretPlacement } from './project-timeline-tooltip-caret'; +import { caretPlacement } from './caret-placement'; describe('caretPlacement', () => { const rect = (left:number, top:number, width:number, height:number) => new DOMRect(left, top, width, height); diff --git a/frontend/src/app/shared/components/project-timeline-graph/project-timeline-tooltip-caret.ts b/frontend/src/app/shared/components/anchored-popover/caret-placement.ts similarity index 100% rename from frontend/src/app/shared/components/project-timeline-graph/project-timeline-tooltip-caret.ts rename to frontend/src/app/shared/components/anchored-popover/caret-placement.ts diff --git a/frontend/src/app/shared/components/anchored-popover/popover-message.spec.ts b/frontend/src/app/shared/components/anchored-popover/popover-message.spec.ts new file mode 100644 index 000000000000..e105712000c0 --- /dev/null +++ b/frontend/src/app/shared/components/anchored-popover/popover-message.spec.ts @@ -0,0 +1,76 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { render } from 'lit-html'; +import { within } from '@testing-library/dom'; +import { popoverMessage } from './popover-message'; + +describe('popoverMessage', () => { + let host:HTMLElement; + + beforeEach(() => { + host = document.createElement('div'); + }); + + const message = () => host.querySelector('.Popover-message')!; + + it('renders the content inside a Primer popover message', () => { + render(popoverMessage('Launch'), host); + expect(within(host).getByText('Launch')).toHaveClass('Popover-message', 'op-anchored-popover'); + }); + + it('renders an element as content', () => { + const el = document.createElement('strong'); + el.textContent = 'Milestone'; + render(popoverMessage(el), host); + expect(within(host).getByText('Milestone')).toBe(el); + expect(el.parentElement).toBe(message()); + }); + + it('uses the default (top) caret when no placement is known', () => { + render(popoverMessage('x'), host); + expect(message()).toHaveClass('Popover-message op-anchored-popover', { exact: true }); + expect(message().style.getPropertyValue('--op-anchored-popover-caret-offset')).toBe(''); + }); + + it.each([ + ['bottom', 'Popover-message--bottom'], + ['left', 'Popover-message--left'], + ['right', 'Popover-message--right'], + ] as const)('sets the %s caret class and offset', (side, className) => { + render(popoverMessage('x', { side, offset: 42 }), host); + expect(message()).toHaveClass(className); + expect(message().style.getPropertyValue('--op-anchored-popover-caret-offset')).toBe('42px'); + }); + + it('drops the modifier again when re-rendered with a top caret', () => { + render(popoverMessage('x', { side: 'bottom', offset: 10 }), host); + render(popoverMessage('x', { side: 'top', offset: 10 }), host); + expect(message()).not.toHaveClass('Popover-message--bottom'); + }); +}); diff --git a/frontend/src/app/shared/components/anchored-popover/popover-message.ts b/frontend/src/app/shared/components/anchored-popover/popover-message.ts new file mode 100644 index 000000000000..6c23c4336a6d --- /dev/null +++ b/frontend/src/app/shared/components/anchored-popover/popover-message.ts @@ -0,0 +1,49 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { html } from 'lit-html'; +import type { TemplateResult } from 'lit-html'; +import { classMap } from 'lit-html/directives/class-map.js'; +import { styleMap } from 'lit-html/directives/style-map.js'; +import type { CaretPlacement } from './caret-placement'; + +export function popoverMessage(content:unknown, caret?:CaretPlacement|null):TemplateResult { + return html` +
+ ${content} +
+ `; +} diff --git a/frontend/src/app/shared/components/anchored-popover/popover-placement.spec.ts b/frontend/src/app/shared/components/anchored-popover/popover-placement.spec.ts new file mode 100644 index 000000000000..7a6650ecc642 --- /dev/null +++ b/frontend/src/app/shared/components/anchored-popover/popover-placement.spec.ts @@ -0,0 +1,105 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import '@openproject/primer-view-components/app/components/primer/anchored_position'; +import type AnchoredPositionElement from '@openproject/primer-view-components/app/components/primer/anchored_position'; +import { placePopover } from './popover-placement'; + +describe('placePopover', () => { + let popover:AnchoredPositionElement; + let anchor:HTMLElement; + + beforeEach(() => { + anchor = document.createElement('div'); + anchor.style.cssText = 'position: fixed; top: 200px; left: 200px; width: 10px; height: 10px;'; + popover = document.createElement('anchored-position') as AnchoredPositionElement; + popover.setAttribute('popover', 'manual'); + popover.setAttribute('side', 'outside-top'); + popover.setAttribute('align', 'center'); + popover.setAttribute('anchor-offset', 'spacious'); + popover.style.cssText = 'margin: 0; padding: 0; border: 0;'; + popover.innerHTML = '
'; + document.body.append(anchor, popover); + popover.anchorElement = anchor; + popover.togglePopover(true); + }); + + afterEach(() => { + popover.remove(); + anchor.remove(); + document.body.style.minHeight = ''; + window.scrollTo(0, 0); + }); + + const centred = () => popover.getBoundingClientRect().width / 2; + + it('places the caret on the edge facing the anchor as soon as the popover is open', () => { + expect(placePopover(popover, anchor)).toEqual({ side: 'bottom', offset: centred() }); + }); + + it('moves the popover into place without waiting for a frame', () => { + anchor.style.left = '300px'; + placePopover(popover, anchor); + + const box = popover.getBoundingClientRect(); + expect(box.left + box.width / 2).toBeCloseTo(305, 0); + expect(box.bottom).toBeLessThanOrEqual(200); + }); + + it('follows the flip when the anchor is too close to the viewport edge', () => { + anchor.style.top = '0px'; + expect(placePopover(popover, anchor).side).toBe('top'); + }); + + it('accepts a rectangle as the anchor', () => { + expect(placePopover(popover, new DOMRect(200, 200, 10, 10))).toEqual({ side: 'bottom', offset: centred() }); + }); + + it('agrees with where anchored-position puts the popover', async () => { + const caret = placePopover(popover, anchor); + await new Promise(requestAnimationFrame); + + const box = popover.getBoundingClientRect(); + expect(box.bottom).toBeLessThanOrEqual(200); + expect(caret.offset).toBeCloseTo(205 - box.left, 0); + }); + + it('faces the anchor on a scrolled page', async () => { + document.body.style.minHeight = '3000px'; + window.scrollTo(0, 1000); + anchor.style.cssText = 'position: absolute; top: 1200px; left: 200px; width: 10px; height: 10px;'; + + const caret = placePopover(popover, anchor); + expect(caret.side).toBe('bottom'); + + await new Promise(requestAnimationFrame); + const box = popover.getBoundingClientRect(); + expect(box.bottom).toBeLessThanOrEqual(anchor.getBoundingClientRect().top); + expect(caret.offset).toBeCloseTo(205 - box.left, 0); + }); +}); diff --git a/frontend/src/app/shared/components/anchored-popover/popover-placement.ts b/frontend/src/app/shared/components/anchored-popover/popover-placement.ts new file mode 100644 index 000000000000..cd8ff0470880 --- /dev/null +++ b/frontend/src/app/shared/components/anchored-popover/popover-placement.ts @@ -0,0 +1,46 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { getAnchoredPosition } from '@primer/behaviors'; +import type AnchoredPositionElement from '@openproject/primer-view-components/app/components/primer/anchored_position'; +import { caretPlacement } from './caret-placement'; +import type { CaretPlacement } from './caret-placement'; + +// `anchored-position` positions its popover in a later animation frame and +// does not expose which side it settled on. Applying Primer's result right +// away pre-empts that frame and yields the box the caret is read from. +export function placePopover(popover:AnchoredPositionElement, anchor:Element|DOMRect):CaretPlacement { + const { left, top } = getAnchoredPosition(popover, anchor, popover); + popover.style.top = `${top}px`; + popover.style.left = `${left}px`; + popover.style.bottom = 'auto'; + popover.style.right = 'auto'; + + const anchorRect = anchor instanceof DOMRect ? anchor : anchor.getBoundingClientRect(); + return caretPlacement(popover.getBoundingClientRect(), anchorRect); +} diff --git a/frontend/src/app/shared/components/project-timeline-graph/project-timeline-graph.component.sass b/frontend/src/app/shared/components/project-timeline-graph/project-timeline-graph.component.sass index 4f89253d7f3a..753e35a89d97 100644 --- a/frontend/src/app/shared/components/project-timeline-graph/project-timeline-graph.component.sass +++ b/frontend/src/app/shared/components/project-timeline-graph/project-timeline-graph.component.sass @@ -31,30 +31,13 @@ // Tooltip .op-project-timeline-graph--tooltip pointer-events: none - margin: 0 - padding: 0 - border: 0 - background: none - text-align: start .Popover-message width: auto max-width: 350px padding: var(--base-size-12) - color: var(--fgColor-default) overflow-wrap: anywhere - &:not(.Popover-message--left):not(.Popover-message--right) - &::before, - &::after - left: var(--op-timeline-tooltip-caret-offset, 50%) - - &.Popover-message--left, - &.Popover-message--right - &::before, - &::after - top: var(--op-timeline-tooltip-caret-offset, 50%) - .op-timeline-tooltip--meta-row display: inline-flex align-items: center diff --git a/frontend/src/app/shared/components/project-timeline-graph/project-timeline-graph.component.spec.ts b/frontend/src/app/shared/components/project-timeline-graph/project-timeline-graph.component.spec.ts index 9078dd914c19..e067c023eaa6 100644 --- a/frontend/src/app/shared/components/project-timeline-graph/project-timeline-graph.component.spec.ts +++ b/frontend/src/app/shared/components/project-timeline-graph/project-timeline-graph.component.spec.ts @@ -550,8 +550,8 @@ describe('ProjectTimelineGraphComponent', () => { it('renders no caret side until the placement is known', () => { const { message } = renderView({ content: 'Launch' }); - expect(Array.from(message.classList)).toEqual(['Popover-message']); - expect(message.style.getPropertyValue('--op-timeline-tooltip-caret-offset')).toBe(''); + expect(Array.from(message.classList)).toEqual(['Popover-message', 'op-anchored-popover']); + expect(message.style.getPropertyValue('--op-anchored-popover-caret-offset')).toBe(''); }); it('turns the caret to face the anchor at the given offset', () => { @@ -559,7 +559,7 @@ describe('ProjectTimelineGraphComponent', () => { expect(message.classList.contains('Popover-message--left')).toBe(true); expect(message.classList.contains('Popover-message--bottom')).toBe(false); - expect(message.style.getPropertyValue('--op-timeline-tooltip-caret-offset')).toBe('30px'); + expect(message.style.getPropertyValue('--op-anchored-popover-caret-offset')).toBe('30px'); }); it('drops the sideways caret when the popover moves back above the anchor', () => { @@ -662,8 +662,8 @@ describe('ProjectTimelineGraphComponent', () => { let element:HTMLElement; - // Only the hover delay is faked; the caret waits for a real animation frame. const fakeHoverDelay = () => vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + const nextFrame = () => new Promise(requestAnimationFrame); const popover = () => element.querySelector('.op-project-timeline-graph--tooltip')!; const message = () => popover().querySelector('.Popover-message')!; @@ -680,17 +680,19 @@ describe('ProjectTimelineGraphComponent', () => { fixture.detectChanges(); return element.querySelector(selector) !== null; }); + await nextFrame(); return element.querySelector(selector)!; }; - const caretOffsetValue = () => message().style.getPropertyValue('--op-timeline-tooltip-caret-offset'); + const caretOffsetValue = () => message().style.getPropertyValue('--op-anchored-popover-caret-offset'); - const openTooltip = async (item:Element) => { - fakeHoverDelay(); + // anchored-position places the popover in the frame after it opens. + const openTooltip = (item:Element) => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout', 'requestAnimationFrame', 'cancelAnimationFrame'] }); hover('mouseover', item); vi.advanceTimersByTime(500); + vi.advanceTimersToNextFrame(); vi.useRealTimers(); - await vi.waitUntil(() => caretOffsetValue() !== ''); }; const caretOffset = () => parseFloat(caretOffsetValue()); @@ -759,8 +761,8 @@ describe('ProjectTimelineGraphComponent', () => { expect(popover()).toBe(before); }); - it('points the caret at the diamond from the side facing it', async () => { - await openTooltip(milestoneItem); + it('points the caret at the diamond from the side facing it', () => { + openTooltip(milestoneItem); const diamond = milestoneItem.querySelector('.vis-dot')!.getBoundingClientRect(); const box = popover().getBoundingClientRect(); @@ -772,16 +774,16 @@ describe('ProjectTimelineGraphComponent', () => { expect(message().classList.contains('Popover-message--bottom')).toBe(popoverIsAbove); }); - it('closes the tooltip when the page scrolls', async () => { - await openTooltip(milestoneItem); + it('closes the tooltip when the page scrolls', () => { + openTooltip(milestoneItem); expect(isOpen()).toBe(true); document.dispatchEvent(new Event('scroll')); expect(isOpen()).toBe(false); }); - it('closes the tooltip when the window is resized', async () => { - await openTooltip(milestoneItem); + it('closes the tooltip when the window is resized', () => { + openTooltip(milestoneItem); expect(isOpen()).toBe(true); window.dispatchEvent(new Event('resize')); @@ -813,7 +815,7 @@ describe('ProjectTimelineGraphComponent', () => { it('keeps a long milestone name inside the viewport', async () => { const longName = 'really long milestone '.repeat(12).trim(); const item = await renderItems('.vis-item.vis-point', { milestonesData: [{ ...milestone, subject: longName }] }); - await openTooltip(item); + openTooltip(item); const box = popover().getBoundingClientRect(); expect(box.left).toBeGreaterThanOrEqual(0); @@ -823,7 +825,7 @@ describe('ProjectTimelineGraphComponent', () => { it('anchors a phase bar on the bar itself', async () => { const bar = await renderItems('.vis-item.vis-range', { phasesData: [phaseWithDates] }); - await openTooltip(bar); + openTooltip(bar); expect(caretOffset()).toBeCloseTo(expectedCaretOffset(bar.getBoundingClientRect()), 0); expect(popover().textContent).toContain('Design'); @@ -831,7 +833,7 @@ describe('ProjectTimelineGraphComponent', () => { it('anchors a gate on its visible icon rather than the hidden dot', async () => { const gate = await renderItems('.vis-item.vis-point.op-timeline-gate', { phasesData: [phaseWithGates] }); - await openTooltip(gate); + openTooltip(gate); const icon = gate.getBoundingClientRect(); expect(icon.width).toBeGreaterThan(0); @@ -842,7 +844,7 @@ describe('ProjectTimelineGraphComponent', () => { it('shows every gate of a cluster', async () => { const secondPhase = { ...phaseWithGates, id: 3, name: 'Test', startGateName: 'Test Start', finishGate: false }; const cluster = await renderItems('.vis-item.vis-cluster', { phasesData: [phaseWithGates, secondPhase] }); - await openTooltip(cluster); + openTooltip(cluster); expect(popover().textContent).toContain('Build Start'); expect(popover().textContent).toContain('Test Start'); diff --git a/frontend/src/app/shared/components/project-timeline-graph/project-timeline-tooltip.builder.ts b/frontend/src/app/shared/components/project-timeline-graph/project-timeline-tooltip.builder.ts index d758a6a9b440..321a1c783cae 100644 --- a/frontend/src/app/shared/components/project-timeline-graph/project-timeline-tooltip.builder.ts +++ b/frontend/src/app/shared/components/project-timeline-graph/project-timeline-tooltip.builder.ts @@ -33,10 +33,9 @@ import { TimezoneService } from 'core-app/core/datetime/timezone.service'; import { octiconElement } from 'core-app/shared/helpers/op-icon-builder'; import { html, nothing } from 'lit-html'; import type { TemplateResult } from 'lit-html'; -import { classMap } from 'lit-html/directives/class-map.js'; -import { styleMap } from 'lit-html/directives/style-map.js'; +import { popoverMessage } from 'core-app/shared/components/anchored-popover/popover-message'; +import type { CaretPlacement } from 'core-app/shared/components/anchored-popover/caret-placement'; import type { ProjectTimelineItem } from './project-timeline-item.builder'; -import type { CaretPlacement } from './project-timeline-tooltip-caret'; export interface TooltipView { anchor:HTMLElement | null; @@ -52,22 +51,13 @@ export class ProjectTimelineTooltipBuilder { popoverTemplate({ anchor, content, caret }:TooltipView):TemplateResult { return html` -
- ${content ?? nothing} -
+ ${popoverMessage(content ?? nothing, caret)} `; } diff --git a/frontend/src/app/shared/components/project-timeline-graph/project-timeline-tooltip.popover.ts b/frontend/src/app/shared/components/project-timeline-graph/project-timeline-tooltip.popover.ts index 887cec366b50..c01c3cb3d145 100644 --- a/frontend/src/app/shared/components/project-timeline-graph/project-timeline-tooltip.popover.ts +++ b/frontend/src/app/shared/components/project-timeline-graph/project-timeline-tooltip.popover.ts @@ -29,8 +29,8 @@ import type AnchoredPositionElement from '@openproject/primer-view-components/app/components/primer/anchored_position'; import { render } from 'lit-html'; import type { Timeline } from 'vis-timeline/standalone'; +import { placePopover } from 'core-app/shared/components/anchored-popover/popover-placement'; import type { ProjectTimelineTooltipBuilder, TooltipView } from './project-timeline-tooltip.builder'; -import { caretPlacement } from './project-timeline-tooltip-caret'; const TOOLTIP_DELAY_IN_MS = 500; @@ -113,22 +113,14 @@ export class ProjectTimelineTooltipPopover { }, TOOLTIP_DELAY_IN_MS); } - // `anchored-position` positions the popover in a frame it requests on - // `beforetoggle` and does not report which side it settled on, so the caret - // is derived from the resulting geometry one frame after opening. private open(anchor:HTMLElement):void { if (!anchor.isConnected) return; - this.popover?.togglePopover(true); - requestAnimationFrame(() => this.alignCaret()); - } - - private alignCaret():void { const popover = this.popover; - const { anchor } = this.view; - if (!popover || !anchor || !popover.matches(':popover-open')) return; + if (!popover) return; - this.view = { ...this.view, caret: caretPlacement(popover.getBoundingClientRect(), anchor.getBoundingClientRect()) }; + popover.togglePopover(true); + this.view = { ...this.view, caret: placePopover(popover, anchor) }; this.render(); } diff --git a/frontend/src/global_styles/common/openproject-common.module.sass b/frontend/src/global_styles/common/openproject-common.module.sass index adc7bbbed6fb..6a918b35adc1 100644 --- a/frontend/src/global_styles/common/openproject-common.module.sass +++ b/frontend/src/global_styles/common/openproject-common.module.sass @@ -2,6 +2,7 @@ @import 'menu/menu' @import 'input/input' @import 'bubble/bubble' +@import '../../app/shared/components/anchored-popover/anchored-popover' @import '../../app/shared/components/forms' @import '../../app/shared/components/option-list/option-list' @import '../../app/shared/components/table/table' From 510dc0180615a3f1ed6365bfc7e9fa9d5e174633 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Thu, 3 Sep 2026 12:55:28 +0100 Subject: [PATCH 3/6] Point the calendar popover caret at its entry The time-entry popover carried a fixed left-top caret, so it pointed away from the entry whenever anchored-position placed the popover below or left of it. Derives the caret from the shared placement once the popover has opened and moves the markup into a stateless template. The anchor offset grows from 4px to 8px, the room the caret tip needs, replacing the margin the message used to carry for the same purpose. anchored-position moves an open popover when the window is resized, which would leave the caret stale, so the popover closes on resize as the timeline tooltip does. --- .../te-calendar/te-calendar-popover.spec.ts | 69 +++++++++ .../te-calendar/te-calendar-popover.ts | 69 +++++++++ .../te-calendar/te-calendar.component.sass | 4 + .../te-calendar/te-calendar.component.spec.ts | 135 ++++++++++++++++++ .../te-calendar/te-calendar.component.ts | 83 ++++------- 5 files changed, 306 insertions(+), 54 deletions(-) create mode 100644 frontend/src/app/features/calendar/te-calendar/te-calendar-popover.spec.ts create mode 100644 frontend/src/app/features/calendar/te-calendar/te-calendar-popover.ts create mode 100644 frontend/src/app/features/calendar/te-calendar/te-calendar.component.spec.ts diff --git a/frontend/src/app/features/calendar/te-calendar/te-calendar-popover.spec.ts b/frontend/src/app/features/calendar/te-calendar/te-calendar-popover.spec.ts new file mode 100644 index 000000000000..814081962e18 --- /dev/null +++ b/frontend/src/app/features/calendar/te-calendar/te-calendar-popover.spec.ts @@ -0,0 +1,69 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { render } from 'lit-html'; +import { within } from '@testing-library/dom'; +import { timeEntryPopoverHtml } from './te-calendar-popover'; + +describe('timeEntryPopoverHtml', () => { + let host:HTMLElement; + + beforeEach(() => { + host = document.createElement('div'); + }); + + const rows = [{ label: 'Project', value: 'Demo' }, { label: 'Hours', value: '2h' }]; + + it('renders a hint popover anchored to the entry', () => { + render(timeEntryPopoverHtml('pop-1', 'anchor-1', rows, null), host); + const popover = host.querySelector('anchored-position')!; + + expect(popover).toHaveAttribute('id', 'pop-1'); + expect(popover).toHaveAttribute('anchor', 'anchor-1'); + expect(popover).toHaveAttribute('popover', 'hint'); + expect(popover).toHaveAttribute('role', 'dialog'); + expect(popover).toHaveClass('op-anchored-popover--host'); + }); + + it('lists every row as label and value', () => { + render(timeEntryPopoverHtml('pop-1', 'anchor-1', rows, null), host); + const entries = within(host).getAllByRole('listitem'); + + expect(entries).toHaveLength(2); + expect(entries[0]).toHaveTextContent('Project: Demo'); + expect(entries[1]).toHaveTextContent('Hours: 2h'); + }); + + it('renders the caret from the placement', () => { + render(timeEntryPopoverHtml('pop-1', 'anchor-1', rows, { side: 'right', offset: 20 }), host); + const message = host.querySelector('.Popover-message')!; + + expect(message).toHaveClass('Popover-message--right'); + expect(message.style.getPropertyValue('--op-anchored-popover-caret-offset')).toBe('20px'); + }); +}); diff --git a/frontend/src/app/features/calendar/te-calendar/te-calendar-popover.ts b/frontend/src/app/features/calendar/te-calendar/te-calendar-popover.ts new file mode 100644 index 000000000000..6533c044cb12 --- /dev/null +++ b/frontend/src/app/features/calendar/te-calendar/te-calendar-popover.ts @@ -0,0 +1,69 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { html } from 'lit-html'; +import type { TemplateResult } from 'lit-html'; +import { popoverMessage } from 'core-app/shared/components/anchored-popover/popover-message'; +import type { CaretPlacement } from 'core-app/shared/components/anchored-popover/caret-placement'; + +export interface PopoverRow { + label:string; + value:string; +} + +export function timeEntryPopoverHtml( + popoverId:string, + anchorId:string, + rows:PopoverRow[], + caret:CaretPlacement|null, +):TemplateResult { + const list = html` +
    + ${rows.map((row) => html` +
  • + ${row.label}: + ${row.value} +
    • + `)} +
+ `; + + return html` + + ${popoverMessage(list, caret)} + + `; +} diff --git a/frontend/src/app/features/calendar/te-calendar/te-calendar.component.sass b/frontend/src/app/features/calendar/te-calendar/te-calendar.component.sass index 457b0bb67409..7748a1253469 100644 --- a/frontend/src/app/features/calendar/te-calendar/te-calendar.component.sass +++ b/frontend/src/app/features/calendar/te-calendar/te-calendar.component.sass @@ -21,6 +21,10 @@ op-time-entries-calendar flex-basis: 165px flex-shrink: 0 + .te-calendar--popover .Popover-message + padding: var(--base-size-8) + font-size: var(--text-body-size-small) + .te-calendar--popover-entry @include text-shortener(false) diff --git a/frontend/src/app/features/calendar/te-calendar/te-calendar.component.spec.ts b/frontend/src/app/features/calendar/te-calendar/te-calendar.component.spec.ts new file mode 100644 index 000000000000..64e2a24fbc11 --- /dev/null +++ b/frontend/src/app/features/calendar/te-calendar/te-calendar.component.spec.ts @@ -0,0 +1,135 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { NO_ERRORS_SCHEMA } from '@angular/core'; +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { FullCalendarModule } from '@fullcalendar/angular'; +import { StateService } from '@uirouter/core'; +import moment from 'moment'; +import { of } from 'rxjs'; +import { ApiV3Service } from 'core-app/core/apiv3/api-v3.service'; +import { BrowserDetector } from 'core-app/core/browser/browser-detector.service'; +import { ConfigurationService } from 'core-app/core/config/configuration.service'; +import { TimezoneService } from 'core-app/core/datetime/timezone.service'; +import { WeekdayService } from 'core-app/core/days/weekday.service'; +import { I18nService } from 'core-app/core/i18n/i18n.service'; +import { PathHelperService } from 'core-app/core/path-helper/path-helper.service'; +import { SchemaCacheService } from 'core-app/core/schemas/schema-cache.service'; +import { DayResourceService } from 'core-app/core/state/days/day.service'; +import { States } from 'core-app/core/states/states.service'; +import { TurboRequestsService } from 'core-app/core/turbo/turbo-requests.service'; +import { HalResourceNotificationService } from 'core-app/features/hal/services/hal-resource-notification.service'; +import { OpCalendarService } from 'core-app/features/calendar/op-calendar.service'; +import { ColorsService } from 'core-app/shared/components/colors/colors.service'; +import { HalResourceEditingService } from 'core-app/shared/components/fields/edit/services/hal-resource-editing.service'; +import { TimeEntryCalendarComponent } from './te-calendar.component'; + +describe('TimeEntryCalendarComponent', () => { + let fixture:ComponentFixture; + let element:HTMLElement; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + declarations: [TimeEntryCalendarComponent], + imports: [FullCalendarModule], + schemas: [NO_ERRORS_SCHEMA], + providers: [ + { provide: I18nService, useValue: { locale: 'en', t: (key:string) => key, toNumber: (value:number) => `${value}` } }, + { provide: ConfigurationService, useValue: { startOfWeek: () => 1, isTimezoneSet: () => false, timezone: () => 'UTC', dateFormatPresent: () => false } }, + { provide: WeekdayService, useValue: { loadWeekdays: () => of([]), isNonWorkingDay: () => false } }, + { provide: DayResourceService, useValue: { requireNonWorkingYears$: () => of([]) } }, + { provide: ApiV3Service, useValue: { time_entries: { list: () => of({ elements: [], createTimeEntry: undefined }) } } }, + { + provide: TimezoneService, + useValue: { + toHours: () => 1, + formattedDuration: () => '1h', + formattedISODate: (date:Date) => moment(date).format('YYYY-MM-DD'), + }, + }, + { provide: BrowserDetector, useValue: { isMobile: false } }, + { provide: States, useValue: {} }, + { provide: StateService, useValue: {} }, + { provide: HalResourceNotificationService, useValue: {} }, + { provide: SchemaCacheService, useValue: {} }, + { provide: ColorsService, useValue: {} }, + ], + }) + .overrideComponent(TimeEntryCalendarComponent, { + set: { + providers: [ + OpCalendarService, + { provide: HalResourceEditingService, useValue: {} }, + { provide: TurboRequestsService, useValue: {} }, + { provide: PathHelperService, useValue: {} }, + ], + }, + }) + .compileComponents(); + + fixture = TestBed.createComponent(TimeEntryCalendarComponent); + fixture.componentRef.setInput('projectIdentifier', 'demo'); + fixture.detectChanges(); + element = fixture.nativeElement as HTMLElement; + }); + + const renderWeek = async (displayedDays:boolean[]) => { + fixture.componentRef.setInput('displayedDays', displayedDays); + await vi.waitUntil(() => { + fixture.detectChanges(); + return element.querySelector('.fc-col-header-cell') !== null; + }); + }; + + it('renders the calendar container', () => { + expect(element.querySelector('.te-calendar--container')).not.toBeNull(); + expect(element.querySelector('full-calendar')).toBeNull(); + }); + + it('renders a week grid once the displayed days are known', async () => { + await renderWeek([true, true, true, true, true, true, true]); + expect(element.querySelectorAll('.fc-col-header-cell.fc-day')).toHaveLength(7); + }); + + it('hides the days that are not displayed', async () => { + await renderWeek([true, true, true, true, true, false, false]); + expect(element.querySelectorAll('.fc-col-header-cell.fc-day')).toHaveLength(5); + }); + + it('closes an open entry popover when the window is resized', () => { + const popover = document.createElement('div'); + popover.className = 'te-calendar--popover'; + popover.setAttribute('popover', 'manual'); + element.append(popover); + popover.showPopover(); + + window.dispatchEvent(new Event('resize')); + + expect(popover.matches(':popover-open')).toBe(false); + }); +}); diff --git a/frontend/src/app/features/calendar/te-calendar/te-calendar.component.ts b/frontend/src/app/features/calendar/te-calendar/te-calendar.component.ts index 0cc28aa8d5e9..dcca81f60f08 100644 --- a/frontend/src/app/features/calendar/te-calendar/te-calendar.component.ts +++ b/frontend/src/app/features/calendar/te-calendar/te-calendar.component.ts @@ -71,7 +71,12 @@ import { TurboRequestsService } from 'core-app/core/turbo/turbo-requests.service import { PathHelperService } from 'core-app/core/path-helper/path-helper.service'; import { ensureId, generateId } from 'core-app/shared/helpers/dom-helpers'; import { target } from 'core-app/shared/helpers/event-helpers'; -import { html, render } from 'lit-html'; +import type AnchoredPositionElement from '@openproject/primer-view-components/app/components/primer/anchored_position'; +import { placePopover } from 'core-app/shared/components/anchored-popover/popover-placement'; +import type { CaretPlacement } from 'core-app/shared/components/anchored-popover/caret-placement'; +import { timeEntryPopoverHtml } from './te-calendar-popover'; +import type { PopoverRow } from './te-calendar-popover'; +import { render } from 'lit-html'; import { DialogCloseDetail } from 'core-turbo/dialog-stream-action'; interface TimeEntrySchema extends SchemaResource { @@ -185,6 +190,10 @@ export class TimeEntryCalendarComponent implements AfterViewInit, OnDestroy { private closeDialogHandler:EventListener = this.handleDialogClose.bind(this); + private closeOpenPopover = () => { + this.element.nativeElement.querySelector('.te-calendar--popover:popover-open')?.hidePopover(); + }; + public additionalOptions:CalendarOptionsWithDayGrid = { editable: false, locales: allLocales, @@ -238,10 +247,12 @@ export class TimeEntryCalendarComponent implements AfterViewInit, OnDestroy { ngAfterViewInit():void { document.addEventListener('dialog:close', this.closeDialogHandler); + window.addEventListener('resize', this.closeOpenPopover); } ngOnDestroy():void { document.removeEventListener('dialog:close', this.closeDialogHandler); + window.removeEventListener('resize', this.closeOpenPopover); } async requireNonWorkingDays(start:Date | string, end:Date | string) { @@ -556,15 +567,19 @@ export class TimeEntryCalendarComponent implements AfterViewInit, OnDestroy { anchorEl.role = 'button'; const popoverId = generateId('popover'); - const popoverHtml = this.popoverHtml(popoverId, anchorId, event.event.extendedProps.entry as TimeEntryResource, schema); - - render(popoverHtml, anchorEl); + const rows = this.popoverRows(entry as TimeEntryResource, schema); + const draw = (caret:CaretPlacement|null) => render(timeEntryPopoverHtml(popoverId, anchorId, rows, caret), anchorEl); + draw(null); anchorEl.setAttribute('aria-haspopup', 'true'); anchorEl.setAttribute('popovertarget', popoverId); - const popoverEl = document.getElementById(popoverId)!; - const showPopover = () => { popoverEl.showPopover(); }; + const popoverEl = document.getElementById(popoverId) as AnchoredPositionElement; + const showPopover = () => { + if (popoverEl.matches(':popover-open')) return; + popoverEl.showPopover(); + draw(placePopover(popoverEl, anchorEl)); + }; const hidePopover = () => { popoverEl.hidePopover(); }; target(anchorEl).on('mouseenter.anchor', showPopover); @@ -650,54 +665,14 @@ export class TimeEntryCalendarComponent implements AfterViewInit, OnDestroy { return formatTimeEntryEntityName(entry.entity); } - private popoverHtml( - popoverId:string, - anchorId:string, - entry:TimeEntryResource, - schema:TimeEntrySchema) { - return html` - - ${this.popoverContentHtml(entry, schema)} - - `; - } - - private popoverContentHtml(entry:TimeEntryResource, schema:TimeEntrySchema) { - return html` -
-
-
    -
  • - ${schema.project.name}: - ${this.sanitizedValue(entry.project.name)} -
    • -
  • - ${schema.entity.name}: - ${entry.entity ? this.sanitizedValue(this.entityName(entry)) : this.i18n.t('js.placeholders.default')} -
    • -
  • - ${schema.activity.name}: - ${this.sanitizedValue(entry.activity?.name ?? '')} -
    • -
  • - ${schema.hours.name}: - ${this.timezone.formattedDuration(entry.hours as string)} -
    • -
  • - ${schema.comment.name}: - ${this.sanitizedValue(entry.comment.raw ?? this.i18n.t('js.placeholders.default'))} -
    • -
-
-
- `; + private popoverRows(entry:TimeEntryResource, schema:TimeEntrySchema):PopoverRow[] { + return [ + { label: schema.project.name, value: this.sanitizedValue(entry.project.name) }, + { label: schema.entity.name, value: entry.entity ? this.sanitizedValue(this.entityName(entry)) : this.i18n.t('js.placeholders.default') }, + { label: schema.activity.name, value: this.sanitizedValue(entry.activity?.name ?? '') }, + { label: schema.hours.name, value: this.timezone.formattedDuration(entry.hours as string) }, + { label: schema.comment.name, value: this.sanitizedValue(entry.comment.raw ?? this.i18n.t('js.placeholders.default')) }, + ]; } private sanitizedValue(value:string):string { From 2688ed876dcd331c3d02546ea88e691e4e78927f Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Thu, 3 Sep 2026 12:55:28 +0100 Subject: [PATCH 4/6] Anchor budget chart tooltips on anchored-position The chart tooltips rendered their lit root straight into document.body and positioned themselves by hand with a fixed left-top caret. Renders them into a host owned by the chart component, lets anchored-position place them from the Chart.js caret point and derives the caret once the popover has opened; the tooltip closes on scroll and resize. --- .../budget-graphs/budget-graphs.sass | 6 + .../budget-graphs/chart.config.spec.ts | 149 ++++++++++++++++++ .../components/budget-graphs/chart.config.ts | 145 ++++++++++------- .../overview/actual-costs.component.html | 1 + .../overview/actual-costs.component.spec.ts | 95 +++++++++++ .../overview/actual-costs.component.ts | 26 ++- .../budget-by-cost-type.component.html | 1 + .../budget-by-cost-type.component.spec.ts | 95 +++++++++++ .../overview/budget-by-cost-type.component.ts | 26 ++- .../common/openproject-common.module.sass | 1 + 10 files changed, 483 insertions(+), 62 deletions(-) create mode 100644 frontend/src/app/shared/components/budget-graphs/budget-graphs.sass create mode 100644 frontend/src/app/shared/components/budget-graphs/chart.config.spec.ts create mode 100644 frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.spec.ts create mode 100644 frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.spec.ts diff --git a/frontend/src/app/shared/components/budget-graphs/budget-graphs.sass b/frontend/src/app/shared/components/budget-graphs/budget-graphs.sass new file mode 100644 index 000000000000..21643abddbcb --- /dev/null +++ b/frontend/src/app/shared/components/budget-graphs/budget-graphs.sass @@ -0,0 +1,6 @@ +.op-chart-tooltip + pointer-events: none + + .Popover-message + padding: var(--base-size-8) + font-size: var(--text-body-size-small) diff --git a/frontend/src/app/shared/components/budget-graphs/chart.config.spec.ts b/frontend/src/app/shared/components/budget-graphs/chart.config.spec.ts new file mode 100644 index 000000000000..7f199a5087f2 --- /dev/null +++ b/frontend/src/app/shared/components/budget-graphs/chart.config.spec.ts @@ -0,0 +1,149 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import '@openproject/primer-view-components/app/components/primer/anchored_position'; +import { within } from '@testing-library/dom'; +import { createPieTooltipRenderer } from './chart.config'; + +describe('chart tooltip renderer', () => { + let wrapper:HTMLElement; + let host:HTMLElement; + let canvas:HTMLCanvasElement; + let renderers:{ destroy():void }[]; + + const nextFrame = () => new Promise(requestAnimationFrame); + const createRenderer = (formatCurrency:(v:number) => string) => { + const renderer = createPieTooltipRenderer(host, formatCurrency); + renderers.push(renderer); + return renderer; + }; + + beforeEach(() => { + renderers = []; + wrapper = document.createElement('div'); + wrapper.style.cssText = 'position: relative; top: 40px; left: 30px;'; + host = document.createElement('div'); + wrapper.append(host); + canvas = document.createElement('canvas'); + canvas.style.cssText = 'position: fixed; top: 100px; left: 100px; width: 300px; height: 200px;'; + document.body.append(canvas, wrapper); + }); + + afterEach(() => { + renderers.forEach((renderer) => renderer.destroy()); + wrapper.remove(); + canvas.remove(); + }); + + const context = (opacity:number, caretX = 50) => ({ + chart: { canvas }, + tooltip: { + opacity, + caretX, + caretY: 40, + dataPoints: [{ label: 'Labour', parsed: 1234 }], + labelColors: [{ backgroundColor: '#123456' }], + }, + }) as unknown as Parameters>[0]; + + const popover = () => host.querySelector('anchored-position')!; + const message = () => host.querySelector('.Popover-message')!; + + it('opens a popover beside the caret point with the formatted value', async () => { + const renderTooltip = createRenderer((v) => `€${v}`); + renderTooltip(context(1)); + + expect(popover().matches(':popover-open')).toBe(true); + expect(within(host).getByText('Labour')).toBeInTheDocument(); + expect(within(host).getByText('€1234')).toBeInTheDocument(); + + await nextFrame(); + expect(popover().getBoundingClientRect().left).toBeGreaterThanOrEqual(158); + }); + + it('points the caret at the point as soon as it opens', () => { + createRenderer((v) => `${v}`)(context(1)); + + expect(message()).toHaveClass('Popover-message--left'); + expect(message().style.getPropertyValue('--op-anchored-popover-caret-offset')).not.toBe(''); + }); + + // Chart.js drives the tooltip from its own animation frame, so the frame + // anchored-position schedules on reopen only runs after the next paint. + it('reopens beside the new point before anchored-position gets its frame', async () => { + const renderTooltip = createRenderer((v) => `${v}`); + renderTooltip(context(1)); + await nextFrame(); + renderTooltip(context(0)); + await nextFrame(); + + const right = await new Promise((resolve) => { + requestAnimationFrame(() => { + renderTooltip(context(1, 250)); + resolve(popover().getBoundingClientRect().right); + }); + }); + expect(right).toBeCloseTo(100 + 250 - 8, 0); + }); + + it('closes the popover when the tooltip fades out', () => { + const renderTooltip = createRenderer((v) => `${v}`); + renderTooltip(context(1)); + renderTooltip(context(0)); + + expect(popover().matches(':popover-open')).toBe(false); + }); + + const viewportChanges = [ + ['the page scrolls', () => document.dispatchEvent(new Event('scroll'))], + ['the window is resized', () => window.dispatchEvent(new Event('resize'))], + ] as const; + + it.each(viewportChanges)('closes the popover when %s', (_, change) => { + createRenderer((v) => `${v}`)(context(1)); + change(); + + expect(popover().matches(':popover-open')).toBe(false); + }); + + it('never renders into document.body', () => { + createRenderer((v) => `${v}`)(context(1)); + expect(document.body.querySelector(':scope > anchored-position')).toBeNull(); + }); + + it.each(viewportChanges)('stops listening once destroyed when %s', (_, change) => { + const renderTooltip = createRenderer((v) => `${v}`); + renderTooltip(context(1)); + renderTooltip.destroy(); + expect(popover().matches(':popover-open')).toBe(false); + + renderTooltip(context(1)); + change(); + expect(popover().matches(':popover-open')).toBe(true); + }); +}); diff --git a/frontend/src/app/shared/components/budget-graphs/chart.config.ts b/frontend/src/app/shared/components/budget-graphs/chart.config.ts index 8b2438f7238a..011b7bc06dd8 100644 --- a/frontend/src/app/shared/components/budget-graphs/chart.config.ts +++ b/frontend/src/app/shared/components/budget-graphs/chart.config.ts @@ -28,6 +28,11 @@ import { ChartOptions, TooltipModel } from 'chart.js'; import { html, render } from 'lit-html'; +import type { TemplateResult } from 'lit-html'; +import type AnchoredPositionElement from '@openproject/primer-view-components/app/components/primer/anchored_position'; +import { popoverMessage } from 'core-app/shared/components/anchored-popover/popover-message'; +import { placePopover } from 'core-app/shared/components/anchored-popover/popover-placement'; +import type { CaretPlacement } from 'core-app/shared/components/anchored-popover/caret-placement'; export const chartFont:ChartOptions['font'] = { family: @@ -54,49 +59,82 @@ interface TooltipContext { tooltip:TooltipModel; } -function applyTooltipPosition( - context:TooltipContext, - popoverHtml:ReturnType, - tooltipId:string, -) { - render(popoverHtml, document.body); +export type BarTooltipContext = TooltipContext<'bar'>; +export type PieTooltipContext = TooltipContext<'pie'>; - const tooltipEl = document.getElementById(tooltipId)!; +class ChartTooltip { + private anchor:DOMRect|null = null; + private caret:CaretPlacement|null = null; + private items:TemplateResult[] = []; + private readonly close = () => { + const popover = this.popover; + if (popover?.isConnected) popover.togglePopover(false); + }; + + constructor(private readonly host:HTMLElement) { + this.draw(); + document.addEventListener('scroll', this.close, { capture: true, passive: true }); + window.addEventListener('resize', this.close); + } + + destroy():void { + document.removeEventListener('scroll', this.close, { capture: true }); + window.removeEventListener('resize', this.close); + this.close(); + } - if (context.tooltip.opacity === 0) { - tooltipEl.style.opacity = '0'; - return; + update(context:TooltipContext, items:TemplateResult[]):void { + if (context.tooltip.opacity === 0) { + this.close(); + return; + } + + const { left, top } = context.chart.canvas.getBoundingClientRect(); + const anchor = new DOMRect(Math.round(left + context.tooltip.caretX), Math.round(top + context.tooltip.caretY), 0, 0); + const moved = anchor.x !== this.anchor?.x || anchor.y !== this.anchor?.y || !this.popover?.matches(':popover-open'); + + this.anchor = anchor; + this.items = items; + this.draw(); + + const popover = this.popover; + if (!moved || !popover?.isConnected) return; + + // Primer's getAnchoredPosition accepts `Element|DOMRect`; the element's + // setter only narrows the type. + popover.anchorElement = anchor as unknown as HTMLElement; + popover.togglePopover(true); + popover.update(); + this.caret = placePopover(popover, anchor); + this.draw(); } - const { left, top } = context.chart.canvas.getBoundingClientRect(); - const x = Math.round(left + context.tooltip.caretX); - const y = Math.round(top + context.tooltip.caretY); - - const wasHidden = !tooltipEl.style.opacity || tooltipEl.style.opacity === '0'; - if (wasHidden) { - // Snap to position before fading in (avoids sliding from initial 0,0) - tooltipEl.style.transition = 'none'; - tooltipEl.style.transform = `translate(${x}px, ${y}px)`; - void tooltipEl.offsetHeight; // force reflow so transform is committed - tooltipEl.style.transition = 'transform 0.1s ease, opacity 0.15s ease'; - } else { - tooltipEl.style.transition = 'transform 0.1s ease, opacity 0.15s ease'; - tooltipEl.style.transform = `translate(${x}px, ${y}px)`; + private get popover():AnchoredPositionElement|null { + return this.host.querySelector('anchored-position'); } - tooltipEl.style.opacity = '1'; + private draw():void { + render( + html` + + ${popoverMessage(html`
    ${this.items}
`, this.caret)} + + `, + this.host, + ); + } } function renderColorDot(color:string) { return html``; } -function renderTooltipItem( - color:string, - label:string, - formattedValue:string, - dateStr?:string, -):ReturnType { +function renderTooltipItem(color:string, label:string, formattedValue:string, dateStr?:string):TemplateResult { const header = dateStr ? html`
${dateStr}${renderColorDot(color)}${label}
` : html`
${renderColorDot(color)}${label}
`; @@ -107,43 +145,30 @@ function renderTooltipItem( `; } -function renderTooltipPopover(tooltipId:string, items:ReturnType[]):ReturnType { - return html` -
-
-
    - ${items} -
-
-
`; -} - -export function createBarTooltipRenderer(formatCurrency:FormatCurrency) { - return function(context:TooltipContext<'bar'>) { - const { tooltip } = context; - const items = tooltip.dataPoints.map((dp, i) => { +export function createBarTooltipRenderer(host:HTMLElement, formatCurrency:FormatCurrency) { + const tooltip = new ChartTooltip(host); + const renderer = (context:TooltipContext<'bar'>) => { + const items = context.tooltip.dataPoints.map((dp, i) => { const timestamp = dp.parsed.x; const dateStr = timestamp != null ? new Date(timestamp).toLocaleDateString(undefined, { month: 'short', year: 'numeric' }) : undefined; - const label = dp.dataset.label ?? ''; - const value = dp.parsed.y ?? 0; - const color = tooltip.labelColors[i]?.backgroundColor as string; - return renderTooltipItem(color, label, formatCurrency(value), dateStr); + const color = context.tooltip.labelColors[i]?.backgroundColor as string; + return renderTooltipItem(color, dp.dataset.label ?? '', formatCurrency(dp.parsed.y ?? 0), dateStr); }); - applyTooltipPosition(context, renderTooltipPopover('chartjs-tooltip-bar', items), 'chartjs-tooltip-bar'); + tooltip.update(context, items); }; + return Object.assign(renderer, { destroy: () => tooltip.destroy() }); } -export function createPieTooltipRenderer(formatCurrency:FormatCurrency) { - return function(context:TooltipContext<'pie'>) { - const { tooltip } = context; - const items = tooltip.dataPoints.map((dp, i) => { - const color = tooltip.labelColors[i]?.backgroundColor as string; - const label = dp.label ?? ''; - const value = dp.parsed; - return renderTooltipItem(color, label, formatCurrency(value)); +export function createPieTooltipRenderer(host:HTMLElement, formatCurrency:FormatCurrency) { + const tooltip = new ChartTooltip(host); + const renderer = (context:TooltipContext<'pie'>) => { + const items = context.tooltip.dataPoints.map((dp, i) => { + const color = context.tooltip.labelColors[i]?.backgroundColor as string; + return renderTooltipItem(color, dp.label ?? '', formatCurrency(dp.parsed)); }); - applyTooltipPosition(context, renderTooltipPopover('chartjs-tooltip-pie', items), 'chartjs-tooltip-pie'); + tooltip.update(context, items); }; + return Object.assign(renderer, { destroy: () => tooltip.destroy() }); } diff --git a/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.html b/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.html index a0b5df90e82b..29db7f9f41f4 100644 --- a/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.html +++ b/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.html @@ -1,5 +1,6 @@ @if (hasChartData()) {
+
} diff --git a/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.spec.ts b/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.spec.ts new file mode 100644 index 000000000000..3a6f8300b438 --- /dev/null +++ b/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.spec.ts @@ -0,0 +1,95 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import '@openproject/primer-view-components/app/components/primer/anchored_position'; +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { provideCharts, withDefaultRegisterables } from 'ng2-charts'; +import { within } from '@testing-library/dom'; +import { I18nService } from 'core-app/core/i18n/i18n.service'; +import PrimerColorsPlugin from 'core-app/shared/components/work-package-graphs/plugin.primer-colors'; +import { ActualCostsComponent } from './actual-costs.component'; +import type { BarTooltipContext } from '../chart.config'; + +describe('ActualCostsComponent', () => { + let fixture:ComponentFixture; + let element:HTMLElement; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [ActualCostsComponent], + providers: [ + { provide: I18nService, useValue: {} }, + provideCharts(withDefaultRegisterables(PrimerColorsPlugin)), + ], + }).compileComponents(); + + fixture = TestBed.createComponent(ActualCostsComponent); + element = fixture.nativeElement as HTMLElement; + }); + + const renderWith = (datasets:unknown[]) => { + fixture.componentRef.setInput('chartData', JSON.stringify({ labels: ['Labour'], datasets })); + fixture.detectChanges(); + }; + + const tooltipContext = (opacity:number):BarTooltipContext => ({ + chart: { canvas: element.querySelector('canvas')! }, + tooltip: { + opacity, + caretX: 50, + caretY: 40, + dataPoints: [{ parsed: { x: 0, y: 10 }, dataset: { label: 'Labour' } }], + labelColors: [{ backgroundColor: '#123456' }], + }, + } as unknown as BarTooltipContext); + + it('renders the chart with a tooltip host beside the canvas', () => { + renderWith([{ label: 'Labour', data: [10] }]); + + const canvas = element.querySelector('canvas')!; + expect(canvas).not.toBeNull(); + expect(canvas.nextElementSibling?.tagName).toBe('DIV'); + }); + + it('renders nothing without data', () => { + renderWith([]); + expect(element.querySelector('canvas')).toBeNull(); + }); + + it('opens the tooltip inside its host from the Chart.js callback', () => { + renderWith([{ label: 'Labour', data: [10] }]); + const external = fixture.componentInstance.barChartOptions()!.plugins!.tooltip!.external as (context:BarTooltipContext) => void; + + external(tooltipContext(1)); + + const popover = element.querySelector('anchored-position')!; + expect(popover.matches(':popover-open')).toBe(true); + expect(within(popover).getByText('Labour')).toBeInTheDocument(); + expect(document.body.querySelector(':scope > anchored-position')).toBeNull(); + }); +}); diff --git a/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.ts b/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.ts index f6129e60ebff..4960baaa9971 100644 --- a/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.ts +++ b/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.ts @@ -29,15 +29,19 @@ import { ChangeDetectionStrategy, Component, + DestroyRef, + ElementRef, Signal, computed, inject, input, + viewChild, } from '@angular/core'; import { ChartConfiguration, ChartData } from 'chart.js'; import 'chartjs-adapter-luxon'; import { I18nService } from 'core-app/core/i18n/i18n.service'; import { chartFont, chartLegend, createBarTooltipRenderer } from 'core-app/shared/components/budget-graphs/chart.config'; +import type { BarTooltipContext } from 'core-app/shared/components/budget-graphs/chart.config'; import PrimerColorsPlugin from 'core-app/shared/components/work-package-graphs/plugin.primer-colors'; import { BaseChartDirective, provideCharts, withDefaultRegisterables } from 'ng2-charts'; @@ -50,6 +54,14 @@ import { BaseChartDirective, provideCharts, withDefaultRegisterables } from 'ng2 }) export class ActualCostsComponent { private readonly i18n = inject(I18nService); + private readonly tooltipHost = viewChild>('tooltipHost'); + + private renderer:ReturnType|null = null; + private renderedHost:HTMLElement|null = null; + + constructor() { + inject(DestroyRef).onDestroy(() => this.renderer?.destroy()); + } readonly chartData = input.required(); readonly currency = input('€'); @@ -80,11 +92,23 @@ export class ActualCostsComponent { 'primer-colors': { datasetLabelBased: true }, tooltip: { enabled: false, - external: createBarTooltipRenderer(this.formatCurrency.bind(this)), + external: this.tooltipRenderer, }, }, })); + private readonly tooltipRenderer = (context:BarTooltipContext) => { + const host = this.tooltipHost()?.nativeElement; + if (!host) return; + + if (host !== this.renderedHost) { + this.renderer?.destroy(); + this.renderer = createBarTooltipRenderer(host, this.formatCurrency.bind(this)); + this.renderedHost = host; + } + this.renderer?.(context); + }; + private formatCurrencyCompact(value:number):string { const currency = this.currency(); diff --git a/frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.html b/frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.html index 2fa897c2fdb6..228700ad80c0 100644 --- a/frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.html +++ b/frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.html @@ -1,5 +1,6 @@ @if (hasChartData()) {
+
} diff --git a/frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.spec.ts b/frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.spec.ts new file mode 100644 index 000000000000..9026a6571c42 --- /dev/null +++ b/frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.spec.ts @@ -0,0 +1,95 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import '@openproject/primer-view-components/app/components/primer/anchored_position'; +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { provideCharts, withDefaultRegisterables } from 'ng2-charts'; +import { within } from '@testing-library/dom'; +import { I18nService } from 'core-app/core/i18n/i18n.service'; +import PrimerColorsPlugin from 'core-app/shared/components/work-package-graphs/plugin.primer-colors'; +import { BudgetByCostTypeComponent } from './budget-by-cost-type.component'; +import type { PieTooltipContext } from '../chart.config'; + +describe('BudgetByCostTypeComponent', () => { + let fixture:ComponentFixture; + let element:HTMLElement; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [BudgetByCostTypeComponent], + providers: [ + { provide: I18nService, useValue: {} }, + provideCharts(withDefaultRegisterables(PrimerColorsPlugin)), + ], + }).compileComponents(); + + fixture = TestBed.createComponent(BudgetByCostTypeComponent); + element = fixture.nativeElement as HTMLElement; + }); + + const renderWith = (datasets:unknown[]) => { + fixture.componentRef.setInput('chartData', JSON.stringify({ labels: ['Labour'], datasets })); + fixture.detectChanges(); + }; + + const tooltipContext = (opacity:number):PieTooltipContext => ({ + chart: { canvas: element.querySelector('canvas')! }, + tooltip: { + opacity, + caretX: 50, + caretY: 40, + dataPoints: [{ label: 'Labour', parsed: 10 }], + labelColors: [{ backgroundColor: '#123456' }], + }, + } as unknown as PieTooltipContext); + + it('renders the chart with a tooltip host beside the canvas', () => { + renderWith([{ data: [10] }]); + + const canvas = element.querySelector('canvas')!; + expect(canvas).not.toBeNull(); + expect(canvas.nextElementSibling?.tagName).toBe('DIV'); + }); + + it('renders nothing without data', () => { + renderWith([{ data: [] }]); + expect(element.querySelector('canvas')).toBeNull(); + }); + + it('opens the tooltip inside its host from the Chart.js callback', () => { + renderWith([{ data: [10] }]); + const external = fixture.componentInstance.pieChartOptions()!.plugins!.tooltip!.external as (context:PieTooltipContext) => void; + + external(tooltipContext(1)); + + const popover = element.querySelector('anchored-position')!; + expect(popover.matches(':popover-open')).toBe(true); + expect(within(popover).getByText('Labour')).toBeInTheDocument(); + expect(document.body.querySelector(':scope > anchored-position')).toBeNull(); + }); +}); diff --git a/frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.ts b/frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.ts index 538358eed7cb..61305cf8b720 100644 --- a/frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.ts +++ b/frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.ts @@ -29,14 +29,18 @@ import { ChangeDetectionStrategy, Component, + DestroyRef, + ElementRef, Signal, computed, inject, input, + viewChild, } from '@angular/core'; import { ChartConfiguration, ChartData } from 'chart.js'; import { I18nService } from 'core-app/core/i18n/i18n.service'; import { chartFont, chartLegend, createPieTooltipRenderer } from 'core-app/shared/components/budget-graphs/chart.config'; +import type { PieTooltipContext } from 'core-app/shared/components/budget-graphs/chart.config'; import PrimerColorsPlugin from 'core-app/shared/components/work-package-graphs/plugin.primer-colors'; import { BaseChartDirective, provideCharts, withDefaultRegisterables } from 'ng2-charts'; @@ -49,6 +53,14 @@ import { BaseChartDirective, provideCharts, withDefaultRegisterables } from 'ng2 }) export class BudgetByCostTypeComponent { private readonly i18n = inject(I18nService); + private readonly tooltipHost = viewChild>('tooltipHost'); + + private renderer:ReturnType|null = null; + private renderedHost:HTMLElement|null = null; + + constructor() { + inject(DestroyRef).onDestroy(() => this.renderer?.destroy()); + } readonly chartData = input.required(); readonly currency = input('€'); @@ -63,11 +75,23 @@ export class BudgetByCostTypeComponent { 'primer-colors': { labelBased: true }, tooltip: { enabled: false, - external: createPieTooltipRenderer(this.formatCurrency.bind(this)), + external: this.tooltipRenderer, }, }, })); + private readonly tooltipRenderer = (context:PieTooltipContext) => { + const host = this.tooltipHost()?.nativeElement; + if (!host) return; + + if (host !== this.renderedHost) { + this.renderer?.destroy(); + this.renderer = createPieTooltipRenderer(host, this.formatCurrency.bind(this)); + this.renderedHost = host; + } + this.renderer?.(context); + }; + private formatCurrency(value:number):string { const currency = this.currency(); try { diff --git a/frontend/src/global_styles/common/openproject-common.module.sass b/frontend/src/global_styles/common/openproject-common.module.sass index 6a918b35adc1..ac437c45b0d9 100644 --- a/frontend/src/global_styles/common/openproject-common.module.sass +++ b/frontend/src/global_styles/common/openproject-common.module.sass @@ -3,6 +3,7 @@ @import 'input/input' @import 'bubble/bubble' @import '../../app/shared/components/anchored-popover/anchored-popover' +@import '../../app/shared/components/budget-graphs/budget-graphs' @import '../../app/shared/components/forms' @import '../../app/shared/components/option-list/option-list' @import '../../app/shared/components/table/table' From ee693f796bfcedc54f99aeb7103dfcb7c31d7fd5 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Thu, 3 Sep 2026 19:36:12 +0100 Subject: [PATCH 5/6] Centre bar chart tooltips on the hovered segment Chart.js anchors a bar tooltip on the top edge of the hovered segment, so the popover sat beside the segment's top corner. Anchors it on the segment's box instead, which centres the popover on the segment and points the caret at its middle. --- .../budget-graphs/chart.config.spec.ts | 28 +++++++++++++++++- .../components/budget-graphs/chart.config.ts | 29 +++++++++++++++---- .../overview/actual-costs.component.spec.ts | 2 +- 3 files changed, 52 insertions(+), 7 deletions(-) diff --git a/frontend/src/app/shared/components/budget-graphs/chart.config.spec.ts b/frontend/src/app/shared/components/budget-graphs/chart.config.spec.ts index 7f199a5087f2..8b152c811edf 100644 --- a/frontend/src/app/shared/components/budget-graphs/chart.config.spec.ts +++ b/frontend/src/app/shared/components/budget-graphs/chart.config.spec.ts @@ -28,7 +28,8 @@ import '@openproject/primer-view-components/app/components/primer/anchored_position'; import { within } from '@testing-library/dom'; -import { createPieTooltipRenderer } from './chart.config'; +import { createBarTooltipRenderer, createPieTooltipRenderer } from './chart.config'; +import type { BarTooltipContext } from './chart.config'; describe('chart tooltip renderer', () => { let wrapper:HTMLElement; @@ -131,6 +132,31 @@ describe('chart tooltip renderer', () => { expect(popover().matches(':popover-open')).toBe(false); }); + it('centres a bar tooltip on the hovered segment', () => { + const renderTooltip = createBarTooltipRenderer(host, (v) => `${v}`); + renderers.push(renderTooltip); + renderTooltip({ + chart: { canvas }, + tooltip: { + opacity: 1, + caretX: 50, + caretY: 20, + dataPoints: [{ + parsed: { x: 0, y: 10 }, + dataset: { label: 'Labour' }, + element: { getProps: () => ({ x: 50, y: 20, base: 120, width: 30 }) }, + }], + labelColors: [{ backgroundColor: '#123456' }], + }, + } as unknown as BarTooltipContext); + + const box = popover().getBoundingClientRect(); + expect(box.left).toBeCloseTo(100 + 65 + 8, 0); + expect(box.top + box.height / 2).toBeCloseTo(100 + 70, 0); + expect(message()).toHaveClass('Popover-message--left'); + expect(message().style.getPropertyValue('--op-anchored-popover-caret-offset')).toBe(`${box.height / 2}px`); + }); + it('never renders into document.body', () => { createRenderer((v) => `${v}`)(context(1)); expect(document.body.querySelector(':scope > anchored-position')).toBeNull(); diff --git a/frontend/src/app/shared/components/budget-graphs/chart.config.ts b/frontend/src/app/shared/components/budget-graphs/chart.config.ts index 011b7bc06dd8..e5302d586ecf 100644 --- a/frontend/src/app/shared/components/budget-graphs/chart.config.ts +++ b/frontend/src/app/shared/components/budget-graphs/chart.config.ts @@ -26,7 +26,7 @@ // See COPYRIGHT and LICENSE files for more details. //++ -import { ChartOptions, TooltipModel } from 'chart.js'; +import { BarElement, ChartOptions, TooltipModel } from 'chart.js'; import { html, render } from 'lit-html'; import type { TemplateResult } from 'lit-html'; import type AnchoredPositionElement from '@openproject/primer-view-components/app/components/primer/anchored_position'; @@ -83,14 +83,12 @@ class ChartTooltip { this.close(); } - update(context:TooltipContext, items:TemplateResult[]):void { + update(context:TooltipContext, items:TemplateResult[], anchor = caretPoint(context)):void { if (context.tooltip.opacity === 0) { this.close(); return; } - const { left, top } = context.chart.canvas.getBoundingClientRect(); - const anchor = new DOMRect(Math.round(left + context.tooltip.caretX), Math.round(top + context.tooltip.caretY), 0, 0); const moved = anchor.x !== this.anchor?.x || anchor.y !== this.anchor?.y || !this.popover?.matches(':popover-open'); this.anchor = anchor; @@ -130,6 +128,27 @@ class ChartTooltip { } } +function caretPoint(context:TooltipContext):DOMRect { + const { left, top } = context.chart.canvas.getBoundingClientRect(); + return new DOMRect(Math.round(left + context.tooltip.caretX), Math.round(top + context.tooltip.caretY), 0, 0); +} + +// Chart.js anchors a bar tooltip on the top edge of the hovered segment; the +// segment's box lets anchored-position centre the popover on it instead. +function barBox(context:TooltipContext<'bar'>):DOMRect { + const bar = context.tooltip.dataPoints[0]?.element as BarElement|undefined; + const { x, y, base, width } = bar?.getProps(['x', 'y', 'base', 'width']) ?? {}; + if (x == null || y == null || base == null || width == null) return caretPoint(context); + + const { left, top } = context.chart.canvas.getBoundingClientRect(); + return new DOMRect( + Math.round(left + x - width / 2), + Math.round(top + Math.min(y, base)), + Math.round(width), + Math.round(Math.abs(base - y)), + ); +} + function renderColorDot(color:string) { return html``; } @@ -156,7 +175,7 @@ export function createBarTooltipRenderer(host:HTMLElement, formatCurrency:Format const color = context.tooltip.labelColors[i]?.backgroundColor as string; return renderTooltipItem(color, dp.dataset.label ?? '', formatCurrency(dp.parsed.y ?? 0), dateStr); }); - tooltip.update(context, items); + tooltip.update(context, items, barBox(context)); }; return Object.assign(renderer, { destroy: () => tooltip.destroy() }); } diff --git a/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.spec.ts b/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.spec.ts index 3a6f8300b438..5f667a6ba440 100644 --- a/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.spec.ts +++ b/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.spec.ts @@ -63,7 +63,7 @@ describe('ActualCostsComponent', () => { opacity, caretX: 50, caretY: 40, - dataPoints: [{ parsed: { x: 0, y: 10 }, dataset: { label: 'Labour' } }], + dataPoints: [{ parsed: { x: 0, y: 10 }, dataset: { label: 'Labour' }, element: { getProps: () => ({ x: 50, y: 20, base: 120, width: 30 }) } }], labelColors: [{ backgroundColor: '#123456' }], }, } as unknown as BarTooltipContext); From 584a8b5ae1663b205195a299335f3d166b2b94b5 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Thu, 3 Sep 2026 20:07:23 +0100 Subject: [PATCH 6/6] Tie the chart tooltip renderer to its host The tooltip renderer was created on the first Chart.js callback and only destroyed with the component, so its scroll and resize listeners outlived the host once the chart lost its data. Creates and destroys the renderer from the host's presence instead. --- .../overview/actual-costs.component.spec.ts | 13 +++++++++ .../overview/actual-costs.component.ts | 27 +++++++++---------- .../budget-by-cost-type.component.spec.ts | 13 +++++++++ .../overview/budget-by-cost-type.component.ts | 27 +++++++++---------- 4 files changed, 52 insertions(+), 28 deletions(-) diff --git a/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.spec.ts b/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.spec.ts index 5f667a6ba440..30dc084aa62c 100644 --- a/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.spec.ts +++ b/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.spec.ts @@ -81,6 +81,19 @@ describe('ActualCostsComponent', () => { expect(element.querySelector('canvas')).toBeNull(); }); + it('drops the tooltip renderer together with its host', () => { + renderWith([{ label: 'Labour', data: [10] }]); + const removeListener = vi.spyOn(document, 'removeEventListener'); + + renderWith([]); + expect(removeListener).toHaveBeenCalledWith('scroll', expect.any(Function), expect.anything()); + + removeListener.mockClear(); + renderWith([{ label: 'Labour', data: [10] }]); + fixture.destroy(); + expect(removeListener).toHaveBeenCalledWith('scroll', expect.any(Function), expect.anything()); + }); + it('opens the tooltip inside its host from the Chart.js callback', () => { renderWith([{ label: 'Labour', data: [10] }]); const external = fixture.componentInstance.barChartOptions()!.plugins!.tooltip!.external as (context:BarTooltipContext) => void; diff --git a/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.ts b/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.ts index 4960baaa9971..306c14e0ef35 100644 --- a/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.ts +++ b/frontend/src/app/shared/components/budget-graphs/overview/actual-costs.component.ts @@ -29,10 +29,10 @@ import { ChangeDetectionStrategy, Component, - DestroyRef, ElementRef, Signal, computed, + effect, inject, input, viewChild, @@ -57,10 +57,19 @@ export class ActualCostsComponent { private readonly tooltipHost = viewChild>('tooltipHost'); private renderer:ReturnType|null = null; - private renderedHost:HTMLElement|null = null; constructor() { - inject(DestroyRef).onDestroy(() => this.renderer?.destroy()); + effect((onCleanup) => { + const host = this.tooltipHost()?.nativeElement; + if (!host) return; + + const renderer = createBarTooltipRenderer(host, this.formatCurrency.bind(this)); + this.renderer = renderer; + onCleanup(() => { + renderer.destroy(); + this.renderer = null; + }); + }); } readonly chartData = input.required(); @@ -97,17 +106,7 @@ export class ActualCostsComponent { }, })); - private readonly tooltipRenderer = (context:BarTooltipContext) => { - const host = this.tooltipHost()?.nativeElement; - if (!host) return; - - if (host !== this.renderedHost) { - this.renderer?.destroy(); - this.renderer = createBarTooltipRenderer(host, this.formatCurrency.bind(this)); - this.renderedHost = host; - } - this.renderer?.(context); - }; + private readonly tooltipRenderer = (context:BarTooltipContext) => this.renderer?.(context); private formatCurrencyCompact(value:number):string { const currency = this.currency(); diff --git a/frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.spec.ts b/frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.spec.ts index 9026a6571c42..092e346e769e 100644 --- a/frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.spec.ts +++ b/frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.spec.ts @@ -81,6 +81,19 @@ describe('BudgetByCostTypeComponent', () => { expect(element.querySelector('canvas')).toBeNull(); }); + it('drops the tooltip renderer together with its host', () => { + renderWith([{ data: [10] }]); + const removeListener = vi.spyOn(document, 'removeEventListener'); + + renderWith([{ data: [] }]); + expect(removeListener).toHaveBeenCalledWith('scroll', expect.any(Function), expect.anything()); + + removeListener.mockClear(); + renderWith([{ data: [10] }]); + fixture.destroy(); + expect(removeListener).toHaveBeenCalledWith('scroll', expect.any(Function), expect.anything()); + }); + it('opens the tooltip inside its host from the Chart.js callback', () => { renderWith([{ data: [10] }]); const external = fixture.componentInstance.pieChartOptions()!.plugins!.tooltip!.external as (context:PieTooltipContext) => void; diff --git a/frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.ts b/frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.ts index 61305cf8b720..e4334c110f0b 100644 --- a/frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.ts +++ b/frontend/src/app/shared/components/budget-graphs/overview/budget-by-cost-type.component.ts @@ -29,10 +29,10 @@ import { ChangeDetectionStrategy, Component, - DestroyRef, ElementRef, Signal, computed, + effect, inject, input, viewChild, @@ -56,10 +56,19 @@ export class BudgetByCostTypeComponent { private readonly tooltipHost = viewChild>('tooltipHost'); private renderer:ReturnType|null = null; - private renderedHost:HTMLElement|null = null; constructor() { - inject(DestroyRef).onDestroy(() => this.renderer?.destroy()); + effect((onCleanup) => { + const host = this.tooltipHost()?.nativeElement; + if (!host) return; + + const renderer = createPieTooltipRenderer(host, this.formatCurrency.bind(this)); + this.renderer = renderer; + onCleanup(() => { + renderer.destroy(); + this.renderer = null; + }); + }); } readonly chartData = input.required(); @@ -80,17 +89,7 @@ export class BudgetByCostTypeComponent { }, })); - private readonly tooltipRenderer = (context:PieTooltipContext) => { - const host = this.tooltipHost()?.nativeElement; - if (!host) return; - - if (host !== this.renderedHost) { - this.renderer?.destroy(); - this.renderer = createPieTooltipRenderer(host, this.formatCurrency.bind(this)); - this.renderedHost = host; - } - this.renderer?.(context); - }; + private readonly tooltipRenderer = (context:PieTooltipContext) => this.renderer?.(context); private formatCurrency(value:number):string { const currency = this.currency();