feat(notifications): surface tapback/reaction notifications (#2039)#2073
Merged
garthvh merged 4 commits intoJul 17, 2026
Merged
Conversation
bruschill
added
the
skip-docs-check
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughTapback reactions now produce localized notifications when the original message is stored locally. Notification actions target the reacted-to message, while cancellation uses the reaction packet. Tests, localization, documentation, and search metadata were updated. ChangesReaction notifications
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MeshPackets
participant ModelContext
participant LocalNotificationManager
participant MeshtasticAppDelegate
MeshPackets->>ModelContext: Fetch original message by replyID
ModelContext-->>MeshPackets: Return stored message or no match
MeshPackets->>LocalNotificationManager: Schedule localized reaction notification
LocalNotificationManager->>MeshtasticAppDelegate: Use replyMessageId for actions
Possibly related issues
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ic#2039) Receiving a tapback/reaction previously produced no local notification: both notification branches in textMessageAppPacket gated on isEmoji == false, so a reaction was stored but only visible with the conversation already open. Now, when a reaction arrives and the sender/channel isn't muted, look up the reacted-to message locally by messageId == replyID and schedule an iMessage-style notification (e.g. `Alice reacted 👍 to "See you soon"`). If the original message isn't known locally (a "phantom" tapback whose replyID matches nothing), the reaction is still stored but no notification is shown — mirroring Android's rememberReaction guard. - Add MeshPackets.reactionNotificationBody(replyID:emoji:senderName:context:) (static + context-injected so it's unit-testable) which returns nil for phantom tapbacks and formats the localized body otherwise. - Add makeMessageNotification(...) factory to collapse the DM/channel by regular/reaction Notification construction into one path. - Add the reaction body format key to Localizable.xcstrings. Out of scope (unchanged): unread-count/badge behavior. The isEmoji == false filters in the *EntityExtension unread counts stay, so a reaction notifies without bumping the conversation's unread badge (matching iMessage/Google Messages). Adds ReactionNotificationTests (7 tests) covering the phantom-tapback guard (unknown/zero/negative replyID), the formatted body, emoji fallbacks, and the nil-original-payload-still-notifies edge. Verified on iOS 18.2 (iPhone 16) and iOS 17-era simulators. Docs: update docs/user/messages.md (Tapback Reactions) to describe received reaction notifications, add a docs/user/whats-new.md entry, and regenerate the bundled HTML under Meshtastic/Resources/docs/.
bruschill
force-pushed
the
feat/reaction-notifications-2039
branch
from
July 11, 2026 19:25
b82500c to
daeea8b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Meshtastic/Helpers/MeshPackets.swift`:
- Line 1444: Update the channelNotificationsEnabled calculation in the
newMessage channel-notification flow to also require that
newMessage.fromUser?.mute is false or unset. Preserve the existing global
notification setting and unmuted-channel checks, ensuring notifications are
suppressed when either the sender or channel is muted.
- Around line 1167-1169: Update the conditional compilation guard around
CarPlayIntentDonation.incomingMessageIntent in the notification handling flow to
require iOS while excluding targetEnvironment(macCatalyst), keeping the donation
enabled for standard iOS builds only.
- Around line 1412-1416: Update the reaction notification construction around
makeMessageNotification so notification metadata keeps messageId for
cancellation while storing the original replyID in a separate field. Adjust the
reaction notification action handlers to read that separate original-reply
identifier, ensuring tapback/reply actions target the original message rather
than the reaction packet.
In `@MeshtasticTests/ReactionNotificationTests.swift`:
- Around line 65-89: Update the reaction notification tests around body(...) to
make formatting deterministic by forcing the English locale/bundle used by
reactionNotificationBody(...), or replace the hard-coded localized assertions
with locale-stable expectations. Apply this consistently to
knownReplyID_formatsBody, knownReplyID_emptyEmoji_fallsBackToHeart,
knownReplyID_nilEmoji_fallsBackToHeart, and
knownReplyID_nilOriginalPayload_stillNotifies.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7766e955-d66a-4b18-8838-5466a4fecf1c
📒 Files selected for processing (4)
Localizable.xcstringsMeshtastic.xcodeproj/project.pbxprojMeshtastic/Helpers/MeshPackets.swiftMeshtasticTests/ReactionNotificationTests.swift
bruschill
removed
the
skip-docs-check
- Exclude CarPlay donation from Mac Catalyst builds (#if os(iOS) && !targetEnvironment(macCatalyst)). - Split reaction notification metadata: carry replyMessageId separately from messageId so tapback/reply actions target the original message, not the reaction packet, while cancellation still keys on messageId. - Honor sender mute for channel notifications. - Pin reaction-body tests to locale-stable assertions.
) Follow-up to the reaction-notification review: apply the same !targetEnvironment(macCatalyst) guard to the incoming and outgoing CarPlayIntentDonation calls so SiriKit donation is consistently iPhone-only. CarPlay has no surface on macOS, so donating there is a no-op; this removes the asymmetry where only the notification-intent path was Catalyst-excluded. The INInteraction.donate path in LocalNotificationManager is already inert on Catalyst since senderIntent is now never set there.
…ications-2039 # Conflicts: # Localizable.xcstrings # Meshtastic.xcodeproj/project.pbxproj # Meshtastic/Resources/docs/index.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed?
Receiving a tapback/reaction now surfaces a local notification. Previously both notification branches in
MeshPackets.textMessageAppPacket(DM and channel) gated onnewMessage.isEmoji == false, so a reaction was stored but produced no notification at all — you could only see it with the conversation already open.Now, when a reaction arrives and the sender/channel isn't muted:
messageId == replyID.Alice reacted 👍 to "See you soon".replyIDmatches nothing locally — the reaction is still stored but no notification is shown, mirroring Android'srememberReactionguard.Implementation:
MeshPackets.reactionNotificationBody(replyID:emoji:senderName:context:)—static+ context-injected so it's unit-testable; returnsnilfor phantom tapbacks, otherwise the localized body.makeMessageNotification(...)factory collapsing the DM/channel × regular/reactionNotificationconstruction into one path.Localizable.xcstrings.Why did it change?
This is the iOS side of meshtastic/design#35 (cross-platform reaction-notification alignment). Android already notifies on a received reaction; iOS was silent. The phantom-tapback guard matches the behavior @garthvh raised in that issue — don't notify about a reaction when there's no local message to show it against.
Fixes #2039.
How is this tested?
MeshtasticTests/ReactionNotificationTests.swift— 7 tests, all passing. Verified on two runtimes:iPhone16-iOS18-test) — 7/7 passedreplyID,replyID == 0, negativereplyID) → no notificationAlice reacted 👍 to "See you soon")static+ context-injected specifically so the phantom guard and body formatting are exercised directly against an in-memory SwiftData context.function_body_length/type_body_lengthonMeshPacketswere left as-is, and the touched function shrank ~299 → ~265 lines via the factory extraction).Explicitly out of scope: unread-count/badge behavior is unchanged. The
isEmoji == falsefilters inChannelEntityExtension/MyInfoEntityExtension/UserEntityExtensionunread counts stay, so a reaction notifies without bumping the conversation's unread badge — matching iMessage and Google Messages per the design#35 analysis.Screenshots/Videos (when applicable)
N/A — notification-content/behavior change; covered by unit tests above.
Checklist
docs/user/ordocs/developer/, and updated accordingly. Updateddocs/user/messages.md(Tapback Reactions — now describes received-reaction notifications), added adocs/user/whats-new.mdentry, and regenerated the bundled HTML underMeshtastic/Resources/docs/viascripts/build-docs.sh.Summary by CodeRabbit
New Features
Bug Fixes
Documentation