Skip to content

[1.x] fix: mobile discussion total post count not updating on reply#4684

Open
huoxin233 wants to merge 1 commit into
flarum:1.xfrom
huoxin233:huoxin/1.x-fix-mobile-post-count
Open

[1.x] fix: mobile discussion total post count not updating on reply#4684
huoxin233 wants to merge 1 commit into
flarum:1.xfrom
huoxin233:huoxin/1.x-fix-mobile-post-count

Conversation

@huoxin233
Copy link
Copy Markdown
Contributor

Fixes #4602

Same issue exits in 1.x also
Backport fix #4678 to 1.x

Changes proposed in this pull request:

Same as 2.x

This PR fixes a UI regression in the PostStreamScrubber component where the total post count in the mobile discussion header fails to update when a user posts a reply.

Previously, the viewing translation string was instantiated once as a static array of VNodes and injected into two distinct DOM locations (the dropdown header button and the scrubber handle). Because Mithril 2 does not support reusing VNode object references across multiple locations, the vnode.dom pointers for the header were silently overwritten by the scrubber handle. This caused subsequent m.redraw() cycles to update the total count and pluralization natively in the scrubber handle, but completely ignore the header button.

By wrapping the translation output in a viewing() closure function and calling {viewing()} in both JSX locations, we ensure fresh, independent VNodes are generated. This allows Mithril to accurately track and natively update both DOM elements during standard redraw cycles.

Reviewers should focus on:

Screenshot

Before:

PixPin_2026-06-02_18-37-02.mp4

After:

PixPin_2026-06-02_18-39-59.mp4

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.
  • Backend changes: tests are green (run composer test).
  • Core developer confirmed locally this works as intended.
  • Tests have been added, or are not appropriate here.

Required changes:

  • Related documentation PR: (Remove if irrelevant)

The scrubber's "viewing X of Y" label was built once as a vnode tree
and rendered in two places (the dropdown toggle and the Scrubber-info
strong). Reusing a single vnode instance in two positions breaks
Mithril's diffing, leaving one copy stale on redraw.

The current post index is force-written to the DOM via jQuery on
scroll, so it always updated; the total count is only rendered through
Mithril's view, so the stale copy (the dropdown toggle shown in the
mobile header) never refreshed after posting a reply.

Build the label as a function that returns a fresh vnode tree per
usage so both render sites diff correctly.

Backport fix flarum#4678

Co-authored-by: IanM <16573496+imorland@users.noreply.github.com>
@huoxin233 huoxin233 requested a review from a team as a code owner June 2, 2026 10:41
@huoxin233 huoxin233 changed the title fix: mobile discussion total post count not updating on reply [1.x] fix: mobile discussion total post count not updating on reply Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant