Skip to content

Hide album cover and apply bubble activity theme while loading music files - #95

Merged
macosswift merged 4 commits into
TelegramMessenger:masterfrom
CHUKEPC:fix/audio-file-loading-cover
Nov 1, 2026
Merged

macosswift merged 4 commits into
TelegramMessenger:masterfrom
CHUKEPC:fix/audio-file-loading-cover

Conversation

@CHUKEPC

@CHUKEPC CHUKEPC commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Key change

  • Subscribe music rows to managed full-download status while excluding external resource fetches such as player streaming and the automatic 500 KB stream-header preload. Add an opt-in FetchManager status projection; existing callers retain their previous default behavior. Pending outgoing uploads retain the inherited pending-message subscription.
  • Show the themed transfer indicator without album art for managed fetching/paused transfers. A matching active or paused player takes precedence, preserving the cover, now-playing animation and playback control even when a managed download overlaps streaming.
  • Extend playback activation from the previous 40×40 cover-only area to the title, duration and empty row body for all music rows that are not in-progress uploads, including received music and the user's own already-sent tracks, in remote, downloading, paused-transfer and local states. This is new behavior, not preservation of an existing body-click action. The visible transfer ring remains a separate cancel control; playback cover clicks remain reachable. In-progress uploads retain their existing cancellation behavior.
  • Music rows do not seed the audio superclass's temporary Local status before their first real transfer status. An already-downloading track therefore installs its ring in place on initial binding/reuse; later genuine status-kind changes for the same media still animate. Other audio rows retain their original Local initialization.
  • Consolidate status/theme updates and clear old subscriptions when rows are reused. The indicator uses the bubble activity colors and blending mode, including the outgoing cut-out glyph.

Reason

Album art could show through the loading indicator. Simply observing every MediaBox fetch also treats streaming and header preloading as a user-visible full download, obscuring playback and changing click behavior. The row now distinguishes managed transfers from background fetches and prioritizes the current player's presentation.

Screenshot/video result (before/after)

Before

Album art remained underneath the progress wheel:

Before - album cover peeking through progress circle

After

The loading indicator uses the bubble activity theme without album art underneath it:

After - clean progress wheel without cover artifact

These are the original captures, not evidence of the newly connected received-download path.

Confirmation

Verification environment: macOS 27.0 (Apple Silicon), Telegram 13.1.0, build 283357 (Debug).

  • Expanded the music regression suite to ten cases, including first-binding/reuse of an already-downloading track without a placeholder-driven animation, with assertions that the ring is fully visible and has no icon-swap animation. Mouse-path tests obtain the actual hit-tested view and dispatch mouse-down/up through the responder chain, covering cover/animation playback, title/duration/body clicks during managed downloads, the separate cancel ring, disabled interaction, and outgoing upload cancellation.

  • Status tests use the production FetchManager projection and music subscription entry point, covering external streaming/header fetches, managed fetching/queue/pause, legacy subscribers, streaming overlapping managed downloads, completion/cancellation, local resources, and reuse with obsolete signal emissions.

  • The playback probe replaces the final player action and feeds production playback-state rendering; it is not a real network/player session. This explicitly replaces the earlier direct executeInteraction(false) assertion that did not establish mouse reachability.

  • All ten actual music XCTest cases and both existing media-status transition tests pass (12 total, zero failures). Swift parsing and diff whitespace checks pass. The full Debug workspace build passed with the corrections integrated against upstream 30097e3b; the local build targets macOS 12.0 for Xcode compatibility. The populated workspace uses local Xcode compatibility settings excluded from the PR.

  • Description-only follow-up: checked the playback scope against ChatMusicContentView.draggingAbility and executeInteraction. The wording now explicitly includes the user's own already-sent tracks; no implementation or test changes were needed for this clarification.

  • I confirm that I have tested the affected behavior and verified that it works properly, or verified the content and links for a documentation-only change.

@macosswift

Copy link
Copy Markdown
Collaborator

Issues:

CONFIRMED: ChatMusicContentView.swift:32-37: the new .Fetching/.Paused handling is only reached for a message that is being sent; a received music file is never given a fetching status here, so the cover is never hidden while downloading and the "downloaded MP3" test step cannot show this behaviour

CONFIRMED: PR description: the diff also removes the init-time progress theme, passes blendMode: presentation.blendingMode (outgoing uploads now draw a cut-out glyph instead of a dark disc) and adds a second status switch in update(...); none of this is mentioned

CONFIRMED: ChatMusicContentView.swift:32-37: the if let _ = parameters check never has an effect because update(...) at 78-89 applies the theme regardless, and the same theme/visibility switch is written in two places

@CHUKEPC

CHUKEPC commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

Connected received music downloads to the resource-status signal while preserving outgoing upload status and click-to-play/stream behavior. Consolidated cover visibility and activity-theme updates into one helper, removing the duplicated switch and ineffective parameters check. The description now includes the initialization/blending changes and no longer claims manual verification of the previously unreachable download path.

Full Debug workspace build passed; all four new XCTest regression cases passed. The tests exercise actual view status and interaction APIs, not a live network download.

@macosswift

macosswift commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Issues:

CONFIRMED: ChatMusicContentView.swift:37-41, 77-86: the new status subscription also reports the player's own streaming download as fetching, so playing a received track that is not cached hides its cover and now-playing animation for the whole download and puts a progress ring on the play/pause spot; clicking it cancels the fetch instead of pausing, so the user cannot pause until the download ends (before the PR the same click paused).

CONFIRMED: ChatMusicContentView.swift:161-165: the automatic 500 KB stream-header preload that every uncached received track starts (on by default) is now shown as a download, so the album cover is briefly replaced by a progress ring when the row appears although the user started nothing.

CONFIRMED: PR description: the description says click-to-play is preserved while a received track downloads and a test asserts it, but while the ring is visible no mouse click reaches executeInteraction(false): with auto-download on, clicking a downloading track cancels the download (before the PR it played), and the test calls executeInteraction(false) directly, a path the UI cannot reach.

@macosswift

Copy link
Copy Markdown
Collaborator

This pull request is not filled in correctly: the Confirmation section does not state the macOS version. Please complete every section as described in CONTRIBUTING.md and update the PR so it can be reviewed.

@CHUKEPC

CHUKEPC commented Oct 28, 2026

Copy link
Copy Markdown
Contributor Author

Separated managed music transfers from raw streaming/header-preload fetches. The current song's cover and playback animation take precedence even when auto-download overlaps streaming, preserving the cover's pause/resume path. Title/duration/body clicks also reach playback while a separate visible progress ring still cancels the transfer.

Replaced the direct executeInteraction checks with real hit testing and mouse-down/up dispatch, including playback/download coexistence and reuse. All nine music XCTest cases, both existing media-status transition tests, and the full Debug workspace build pass. Updated the description to distinguish the tested pointer path from the final player action, which uses a controlled probe in the tests.

@macosswift

Copy link
Copy Markdown
Collaborator

Issues:

CONFIRMED: ChatMusicContentView.swift:93: when a row is bound to a received track that is already downloading, the view is first seeded with the .Local placeholder and the real .Fetching status then counts as a same-owner change, so instead of appearing in place the ring plays the icon-to-ring swap and fades in from zero every time the row is shown.

CONFIRMED: PR description: the description says received-track body clicks are kept on the playback path, but before this PR only the 40x40 cover played the track; the new bounds draggingAbility and open() routing in ChatMusicContentView.swift:59-69 make clicks on the title, duration or empty body play or pause every music message in every state, which is new behaviour and should be described as such.

@CHUKEPC

CHUKEPC commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Music now skips the superclass's temporary Local status when binding a new/reused received track. The first real Fetching status initializes the ring without the icon-to-ring/fade-in transition; genuine subsequent same-media state changes still animate. Added a regression covering repeated bindings, same-owner refresh and the unchanged base-audio initialization. All ten music tests plus both media-status transition tests pass, and the full Debug workspace build passed against upstream 30097e3b.

Corrected the description: title, duration and empty-body playback activation is new behavior beyond the previous cover-only hit area, not behavior preserved from master.

@macosswift

Copy link
Copy Markdown
Collaborator

Issues:

  • PR description: The description limits the new title, duration and empty-body playback activation to received music, but draggingAbility and executeInteraction in ChatMusicContentView apply it to every music row that is not an in-progress upload, including the user's own already-sent tracks.
@CHUKEPC

CHUKEPC commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Corrected the description to match ChatMusicContentView.draggingAbility and executeInteraction: title, duration and empty-body playback activation applies to all music rows that are not in-progress uploads, including the user’s own already-sent tracks, not just received music. The transfer ring remains a separate cancel control, and in-progress uploads keep their existing cancellation behavior. This is a description-only correction; the implementation is unchanged.

@macosswift
macosswift merged commit 9b02db3 into TelegramMessenger:master Nov 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants