Repository navigation
fix(mobile): show already-loaded custom emoji in the iOS reactions sheet - #9150
Conversation
The native reactions sheet downloaded each custom emoji again with URLSession and showed the `:shortcode:` label until that download succeeded, keeping it when the download failed. So an emoji visible on the message pill could appear as truncated shortcode text in the sheet. Flutter now keeps the encoded bytes of custom emoji it has downloaded (a bounded LRU, rebuilt with the media auth scope) and passes them to the sheet, which decodes them without a fetch. A custom emoji in the sheet or a reaction pill never shows its shortcode: the sheet stays blank while loading, and both show sparkles if the image cannot load, like the native emoji picker. Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tom Brow <tomb@squareup.com>
|
@codex review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c0c3ca18b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let decoded { self.show(decoded) } else { fallback() } | ||
| } | ||
| } | ||
| return |
There was a problem hiding this comment.
Fall back to the URL when cached decoding fails
When Flutter receives a transient malformed 2xx response, MediaImageProvider._loadAsync invokes onBytesLoaded before attempting its own decode, so these bytes are cached even though the pill displays its error state. This branch treats those bytes as authoritative: if native decoding fails it shows sparkles and returns without trying the URL, whereas the previous sheet implementation could make a fresh request and recover after the server response was fixed. Continue through the existing URL-loading path when cached-byte decoding returns nil.
Useful? React with 👍 / 👎.
| if let bytes = data["bytes"] as? FlutterStandardTypedData { | ||
| DispatchQueue.global(qos: .userInitiated).async { [weak self] in | ||
| let decoded = NativeMessageImageLoader.decode(bytes.data) |
There was a problem hiding this comment.
Deduplicate cached-byte decoding across glyphs
When a custom-emoji reaction has multiple visible reactors, the filter glyph and every reactor-row glyph receive the same encoded bytes, but each independently submits a full animated-image decode to the global queue. Rapidly scrolling a large reactor list can therefore enqueue many uncancelled duplicate decodes, bypassing the four-operation admission limit and decoded-image cache used by NativeMessageImageLoader; route cached bytes through a shared URL-keyed, bounded decoder/cache instead.
AGENTS.md reference: AGENTS.md:L249-L257
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[P2] Share decoded images across cached-byte glyphs
This also repeats during profile hydration: updateProfiles reloads visible rows, each row creates a fresh glyph, and each glyph decodes the same animation again. Could cached bytes use the shared decoded-image cache and admission limit, rather than dispatching a decode per glyph?
Posted by Codex (gpt-6.1-sol) on behalf of kylez
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c0c3ca18b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| filterQuality: FilterQuality.medium, | ||
| semanticLabel: ':$shortcode:', | ||
| errorBuilder: (_, _, _) => Text(':$shortcode:', style: fallbackStyle), | ||
| onBytesLoaded: (bytes) => bytesCache.remember(url, bytes), |
There was a problem hiding this comment.
Forward bytes that arrive after opening the sheet
When the sheet is opened while the pill's image request is still in flight, reactionData() snapshots a cache miss and UIKit starts its own request; this callback may then populate the cache, but the cache is non-reactive and updateReactions() only runs when the reaction list changes. If the Flutter request subsequently succeeds while UIKit's separate request fails, the pill displays the emoji but the open sheet remains on sparkles. Notify the active sheet when cached bytes arrive, or otherwise share the in-flight load.
Useful? React with 👍 / 👎.
🔐 Codex Security Review
Review SummaryOverall Risk: MEDIUM
Findings[MEDIUM]
|
|
🤖 Meli, Tyler’s AI agent, posting on his explicit behalf. The remaining gap is verification of the transient fetch boundary. Both main and PR render the loaded custom-emoji pill and native reactions sheet successfully in the online happy path, so those captures do not demonstrate the reported failure or its fix. Please exercise opening the sheet after the pill has loaded while a subsequent fetch fails, and show that the retained bytes still render the emoji. The full mobile suite passed; no new production defect is established by this verification gap. Reviewed head: |
kazemek21
left a comment
There was a problem hiding this comment.
Kyle reviewed this PR and the Nexus review-tracker notes. Approved with the non-blocking cached-byte decoding feedback in the existing thread.
Posted by Codex (gpt-6.1-sol) on behalf of kylez
…ction-p * origin/main: (111 commits) feat(mobile): tag the reacted-to message author on reactions (#9141) fix(mobile): steady New DM picker rows and animate picks (#9249) perf(mobile): keep remote images on disk and show small ones as thumbnails (#9158) fix(relay): deliver slow cross-node events late instead of closing their recipients (#9235) fix(mobile): read only shown events from filtered Activity rows (#9215) fix(mobile): show the channel name as the notification subtitle (#9173) fix(mobile): keep Home's header clean under sheets (#9152) fix(mobile): show already-loaded custom emoji in the iOS reactions sheet (#9150) mobile: add follow and remind actions to inbox row menu (#9142) fix(mobile): start no threads in Solo chats (#9123) mobile: persist user profiles so names and avatars render on launch (#9116) fix(mobile): keep a sent thread reply above the composer (#9026) [buzz] Recover database pool capacity after stalled connections (#9267) feat: send a User-Agent on relay WebSocket upgrades (#9246) Record why relay connections close and quiet close warnings (#9245) fix(relay): say "channel owner or admin" in the call_webhook save refusal (#9169) feat(mobile): consolidate Solo chat settings in one sheet (#9255) fix(cli): preserve attachments across successive edits (#9226) fix(mobile): swipe left to reply so swipe-back keeps rightward drags (#9127) mobile: long-press a message link for open, copy and share (#9135) ... Signed-off-by: Matthew Boston <mboston@squareup.com>
…-profile * origin/main: (307 commits) fix: inline ARM64 atomics in Sprig image build (#9291) fix(mobile): show observed thread replies in Activity (#9286) Enable mobile huddles with Codex agents (#9269) fix(mobile): read mentions taller than the screen (#9243) test(relay): give every media e2e upload its own blob hash (#9272) Add invite preview video and fallback poster (#9282) NEXUS-4336: Wait for desktop-provider socket ownership proof (#9270) fix: unblock image audit with patched MCP SDK (#9287) feat(ci): allow admin dispatch of desktop candidate validation (#9288) feat(mobile): tag the reacted-to message author on reactions (#9141) fix(mobile): steady New DM picker rows and animate picks (#9249) perf(mobile): keep remote images on disk and show small ones as thumbnails (#9158) fix(relay): deliver slow cross-node events late instead of closing their recipients (#9235) fix(mobile): read only shown events from filtered Activity rows (#9215) fix(mobile): show the channel name as the notification subtitle (#9173) fix(mobile): keep Home's header clean under sheets (#9152) fix(mobile): show already-loaded custom emoji in the iOS reactions sheet (#9150) mobile: add follow and remind actions to inbox row menu (#9142) fix(mobile): start no threads in Solo chats (#9123) mobile: persist user profiles so names and avatars render on launch (#9116) ... Signed-off-by: Franco Sola <fsola@squareup.com> # Conflicts: # docs/nips/NIP-FA.md # docs/nips/NIP-FI.md
…e-mode * origin/main: (36 commits) feat(relay): let an operator mint invites for a community owner or admin (#9265) fix: inline ARM64 atomics in Sprig image build (#9291) fix(mobile): show observed thread replies in Activity (#9286) Enable mobile huddles with Codex agents (#9269) fix(mobile): read mentions taller than the screen (#9243) test(relay): give every media e2e upload its own blob hash (#9272) Add invite preview video and fallback poster (#9282) NEXUS-4336: Wait for desktop-provider socket ownership proof (#9270) fix: unblock image audit with patched MCP SDK (#9287) feat(ci): allow admin dispatch of desktop candidate validation (#9288) feat(mobile): tag the reacted-to message author on reactions (#9141) fix(mobile): steady New DM picker rows and animate picks (#9249) perf(mobile): keep remote images on disk and show small ones as thumbnails (#9158) fix(relay): deliver slow cross-node events late instead of closing their recipients (#9235) fix(mobile): read only shown events from filtered Activity rows (#9215) fix(mobile): show the channel name as the notification subtitle (#9173) fix(mobile): keep Home's header clean under sheets (#9152) fix(mobile): show already-loaded custom emoji in the iOS reactions sheet (#9150) mobile: add follow and remind actions to inbox row menu (#9142) fix(mobile): start no threads in Solo chats (#9123) ... Signed-off-by: Will Pfleger <wpfleger@block.xyz>
On iOS, the reactions sheet (long-press a reaction pill) could show a custom emoji as truncated
:shortcode:text, for example:eye…, even though the same emoji rendered as an image on the message pill a moment before.The sheet is a native UIKit view that downloaded each custom emoji again with its own
URLSessionrequest and showed the shortcode label until that download finished, and for good when it failed. A shortcode is illegible in a chip that size.Now:
Before / after
Reactions sheet header and chips right after opening, on a message with two custom-emoji reactions (synthetic test channel, captured from a 30 fps screen recording).
Validation
just mobile-checkpass; the iOS simulator build compiles.:qa-...shortcodes for a few frames before the images arrive; on this branch they render as images from the first frame (see Before / after). Limitations: the emoji images load fine over the network in this setup, so the failed-download sparkles fallback was covered by tests only, not seen in the app; a long press on the combined reaction tray opened the message menu in the simulator, a tap opened the sheet.🤖 Generated with Claude Code