Skip to content

[op-19850] very long loading times for project timeline widget - #24616

Open
bsatarnejad wants to merge 1 commit into
release/17.7from
op-19850-very-long-loading-times-for-project-timeline-widget
Open

[op-19850] very long loading times for project timeline widget#24616
bsatarnejad wants to merge 1 commit into
release/17.7from
op-19850-very-long-loading-times-for-project-timeline-widget

Conversation

@bsatarnejad

@bsatarnejad bsatarnejad commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Ticket

https://community.openproject.org/wp/OP-19850

What are you trying to accomplish?

The widget previously waited until no changed event had been emitted for one second. Since vis-timeline performs periodic resize checks, the debounce could restart repeatedly and leave the loading skeleton visible after the timeline had already finished rendering.

What approach did you choose and why?

  • Uses onInitialDrawComplete to reveal the timeline after its initial render.
  • Keeps clustering and the current-time marker deferred until rendering completes.
  • Removes the obsolete RxJS event and cleanup.

@bsatarnejad bsatarnejad self-assigned this Aug 5, 2026
@myabc
myabc requested a lite review from Copilot August 5, 2026 13:49

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

This PR addresses long loading times in the project timeline widget by removing the previous RxJS-based “wait for no changed events for 1s” strategy (which could be perpetually reset by vis-timeline resize checks) and instead revealing the widget right after vis-timeline’s initial draw completes.

Changes:

  • Switches the loading reveal trigger to onInitialDrawComplete, calling revealTimeline() after the first render.
  • Keeps showCurrentTime and clustering disabled during the initial draw, enabling them only after reveal.
  • Removes obsolete RxJS subjects/operators and teardown related to the previous debounce-based readiness detection, and adds a unit test for revealTimeline.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
frontend/src/app/shared/components/project-timeline-graph/project-timeline-graph.component.ts Replaces debounce-based “ready” detection with onInitialDrawComplete, and simplifies teardown.
frontend/src/app/shared/components/project-timeline-graph/project-timeline-graph.component.spec.ts Adds unit coverage ensuring revealTimeline() enables options and marks the component as ready.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

Warning

Flaky specs

  • rspec ./spec/features/projects/creation_wizard/wizard_from_template_flow_spec.rb[1:1]
  • rspec ./spec/features/roles/report_spec.rb[1:1]
  • rspec ./spec/features/roles/report_spec.rb[1:3]
  • rspec ./spec/features/work_packages/progress_modal_spec.rb[1:3:6:2: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 #24616, linked for reference only):

- `rspec ./spec/features/projects/creation_wizard/wizard_from_template_flow_spec.rb[1:1]`
- `rspec ./spec/features/roles/report_spec.rb[1:1]`
- `rspec ./spec/features/roles/report_spec.rb[1:3]`
- `rspec ./spec/features/work_packages/progress_modal_spec.rb[1:3:6:2:2]`

Treat this as a standalone task, unrelated to PR #24616. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #24616 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 marked this pull request as ready for review August 5, 2026 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants