Skip to content

[SPPM-324] Milestone tooltip is off screen if milestone name is long - #24700

Closed
bsatarnejad wants to merge 1 commit into
devfrom
sppm-324-tooltip-is-off-screen-if-milestone-name-is-long
Closed

[SPPM-324] Milestone tooltip is off screen if milestone name is long#24700
bsatarnejad wants to merge 1 commit into
devfrom
sppm-324-tooltip-is-off-screen-if-milestone-name-is-long

Conversation

@bsatarnejad

Copy link
Copy Markdown
Contributor

Ticket

https://community.openproject.org/wp/SPPM-324

What are you trying to accomplish?

The project timeline widget renders its hover tooltip via vis-timeline, which appends it inside .vis-timeline (overflow: hidden). That element is in a dashboard grid cell (.grid--area) that both have overflow: hidden and owns a z-index stacking context, so a long milestone name was cut off and could be painted behind the widget below.

Move the tooltip into a body-level layer anchored to the timeline origin and stretched to the viewport edges. vis-timeline's overflowMethod: 'cap' then keeps the tooltip within the viewport, so it stays fully visible regardless of the milestone name length or position.

Screenshots

Screenshot 2026-08-11 at 11 46 01 Screenshot 2026-08-11 at 11 45 54

@github-actions

Copy link
Copy Markdown

Warning

Flaky specs

  • rspec ./modules/gantt/spec/features/timeline/timeline_dates_spec.rb[1:2:1]
  • rspec ./spec/features/notifications/split_panel_navigation_spec.rb[1:2]
🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #24700, linked for reference only):

- `rspec ./modules/gantt/spec/features/timeline/timeline_dates_spec.rb[1:2:1]`
- `rspec ./spec/features/notifications/split_panel_navigation_spec.rb[1:2]`

Treat this as a standalone task, unrelated to PR #24700. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #24700 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @bsatarnejad to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @bsatarnejad, and request a review from @bsatarnejad.
On every commit, set @bsatarnejad as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

@bsatarnejad
bsatarnejad requested a review from myabc August 11, 2026 10:20
@bsatarnejad bsatarnejad self-assigned this Aug 11, 2026
@bsatarnejad
bsatarnejad marked this pull request as ready for review August 17, 2026 11:46
@myabc
myabc requested a lite review from Copilot August 20, 2026 18:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Moves project timeline tooltips to a body-level layer so long milestone names remain visible within the viewport.

Changes:

  • Relocates and positions tooltip layers.
  • Adds cleanup and fixed-layer styling.
  • Adds relocation and lifecycle tests.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Summary
frontend/src/app/shared/components/project-timeline-graph/project-timeline-graph.component.ts Manages tooltip relocation and cleanup. Moderate issue (2 votes): the moved tooltip should remain aria-hidden="true" to preserve accessibility behavior.
frontend/src/app/shared/components/project-timeline-graph/project-timeline-graph.component.spec.ts Tests tooltip relocation and cleanup.
frontend/src/app/shared/components/project-timeline-graph/project-timeline-graph.component.sass Styles the viewport-level tooltip layer.
Suppressed comments (2)

frontend/src/app/shared/components/project-timeline-graph/project-timeline-graph.component.spec.ts:585

  • This test calls the private liftTooltip() directly, so it does not exercise the new itemover subscription at line 152 and would still pass if the event hookup were missing. It also never verifies the viewport-bound geometry that is central to this fix; trigger the actual timeline hover path and assert the layer's positioning (including after scrolling/resizing).
      (fixture.componentInstance as unknown as { liftTooltip:() => void }).liftTooltip();

frontend/src/app/shared/components/project-timeline-graph/project-timeline-graph.component.ts:184

  • The layer's fixed coordinates are calculated only while itemover fires. If the page is scrolled, the viewport is resized, or the grid reflows while the pointer remains over the same item, the timeline moves but this layer stays at its old left/top and size, so the tooltip can become detached or off-screen. Reposition the layer on the relevant scroll/resize/reflow events (and clean those listeners up on destroy), or otherwise keep its viewport anchoring current.
    const rect = root.getBoundingClientRect();
    this.tooltipLayer.style.left = `${rect.left}px`;
    this.tooltipLayer.style.top = `${rect.top}px`;
    this.tooltipLayer.style.width = `${Math.max(0, window.innerWidth - rect.left)}px`;
    this.tooltipLayer.style.height = `${Math.max(0, window.innerHeight - rect.top)}px`;

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +167 to +168
this.tooltipLayer.className = 'op-project-timeline-graph op-project-timeline-graph--tooltip-layer';
document.body.appendChild(this.tooltipLayer);
@myabc
myabc force-pushed the sppm-324-tooltip-is-off-screen-if-milestone-name-is-long branch from afe517c to f58f7bd Compare August 23, 2026 22:47
@github-actions

Copy link
Copy Markdown

Caution

The provided work package version does not match the core version

Details:

Please make sure that:

  • The work package version OR your pull request target branch is correct

@myabc myabc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bsatarnejad This works, but I think the z-index approach will prove to be brittle in the long-run. We already a Primer anchored-position-based solution for this in place for the time entry widgets (frontend/src/app/features/calendar/te-calendar/te-calendar.component.ts), so I think it makes sense to re-use that (plus we "own the code", so to speak).

Since I've ended up reviewing this PR so late (sorry!), and won't have a chance to talk this through with you, I took the liberty of just pushing my alternate solution as #24893.

@github-actions github-actions Bot locked and limited conversation to collaborators Aug 28, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Development

Successfully merging this pull request may close these issues.

3 participants