[DavideIadeluca]

Progresses #3533

Changes proposed in this pull request:
Rewrite the PostStreamScrubber in TS and improve extensibility

Reviewers should focus on:
This change in theory should be 100% backwards compatible. Do we though want to mark some of the internals as such or as private to not tie up our hands too much in the future in regards to making changes to this component?

Screenshot
(No-Op)

Before:

After:

Necessity

• Has the problem that is being solved here been clearly explained?
• If applicable, have various options for solving this problem been considered?
• For core PRs, does this need to be in core, or could it be in an extension?
• Are we willing to maintain this for years / potentially forever?

Confirmed

• Frontend changes: tested on a local Flarum installation.
• Frontend changes: tests are green (run yarn test in js/).
• Frontend changes: tests have been added, or are not appropriate here.
• Backend changes: tests are green (run composer test).
• Backend changes: tests have been added, or are not appropriate here.
• Where applicable, changes are suitable for all supported database drivers (MySQL, MariaDB, PostgreSQL, SQLite).
• Core developer confirmed locally this works as intended.
• The description above is written by me and describes what this pull request actually does.

Required changes:

• Related documentation PR: (Remove if irrelevant)

[imorland]

Note

Automated review for @DavideIadeluca. Multiple AI models with Flarum-specific knowledge reviewed this pull request for me, and a further agent verified their findings; I haven't read it myself yet. I'll look at this pull request personally as soon as I can.

• P2 — Keep configurable handle heights from reversing navigation (framework/core/js/src/forum/components/PostStreamScrubber.tsx:394-397; Codex reviewer, independently checked). The new minHandleHeight API accepts 150px, as the subclass in the new test demonstrates, but the track can be shorter than that (the mobile track is 40vh). With a 100px track, 40 posts and 3 visible posts, the calculation produces a 150% handle and index = -1.351…; a 10px downward drag consequently moves the index backwards by 7.4 posts. These values were reproduced by executing the calculation; the UI was not exercised. Bound the effective minimum relative to the available track height and handle the zero-range case so dragging and clicking remain well-defined when the track shrinks.

• P3 — Unmount the scrubbers in test cleanup (framework/core/js/tests/unit/forum/components/PostStreamScrubber.test.ts:66-68; Claude reviewer, independently checked by reading the lifecycle code). Clearing document.body.innerHTML bypasses Mithril's onremove, leaving each mounted scrubber's window/document handlers attached. The next mount calls trigger('resize'), which then also invokes the detached instances. Keep the mounted roots and unmount them with m.render(root, null) before removing them; also allow the deferred listener-start callback to settle before teardown. This prevents the new tests from accumulating listeners and exercising stale instances.

All three reviewers completed. The branch merges cleanly into 2.x. No additional findings from my own pass. Tests, type checking and browser interaction were not run as part of this review.

[DavideIadeluca]

I have let AI try to address the review feedback. Additionally I opted to restore the onhide behaviour for a true (at least in theory) 1:1 port of this component. @imorland May you review again please?

[imorland]

Requesting changes because conditional items added through the new extension hooks can permanently disable scrubber navigation. I reproduced this on a dev forum running 990736fe5, as a guest, with a 40-post discussion.

1. Conditional extension items leave the scrollbar without click and drag handlers

The new ItemLists have unkeyed children. When an extension adds or removes a button before the scrollbar, Mithril can replace the scrollbar DOM. Its event handlers are attached only in the parent component’s oncreate, which does not run again when those children are replaced.

To reproduce:

1. Add a conditional button to scrubberItems() at priority 50, between first and scrollbar. Add a separate control to toggle its visibility.
2. Open a discussion with enough posts to scrub through.
3. Drag the handle to confirm navigation works.
4. Toggle the conditional button off and allow Mithril to redraw.
5. Try dragging the replacement handle or clicking the track.

In my test, the initial drag moved from post 1 to post 25. Removing the conditional button replaced the scrollbar and cleared its handle sizing. Subsequent dragging and track clicks left the discussion at post 25.

Video: working navigation, then broken navigation after removing the extension item:

0cee359f-76d0-4c84-88cf-50d1fff33c22-d20509c3-1787-47e4-98d8-143d4191849b-mp4.mp4

This matters for ordinary conditional controls, such as a “first unread” button that disappears when there are no unread posts. The new hooks need to preserve the relevant DOM identities, or ensure replacement elements receive their handlers and sizing. Please add a test that toggles an extension item across redraws and then exercises navigation; the current tests cover only the initial render.

2. The oversized-handle test accepts endpoint jumps

The new clamp can make percentPerPost().index zero when the minimum handle height consumes the track. Dragging then divides by zero and jumps to an endpoint.

To reproduce using the configuration in the new test:

1. Set PostStreamScrubber.minHandleHeight = 150.
2. Set .Scrubber-scrollbar to a height and maximum height of 100px.
3. Open the 40-post discussion around post 6.
4. Drag the handle down with a vertical displacement of 1px.

I observed the scrubber change from “6 of 40 posts” to “39 of 40 posts” during movement, then navigate to the final post on release.

Video: oversized handle and endpoint jump:

0cee359f-76d0-4c84-88cf-50d1fff33c22-18445759-a25f-47a9-a662-9ce3d5c58af2-mp4.mp4

The underlying short-track problem predates this PR: the old implementation also divided by zero at a 50px track, and moved backwards below that height. This is therefore incomplete edge-case handling rather than a wholly new regression. However, the newly supported minimum height exposes it, and the new index >= start assertion passes when dragging jumps straight to the end.

Please define the intended behavior when the handle fills the track and assert the actual resulting position in the test.