Respect mute preference for iOS foreground notification sounds#82045
Respect mute preference for iOS foreground notification sounds#82045MobileMage wants to merge 2 commits intoExpensify:mainfrom
Conversation
|
Would need a build triggered to test this one as it's done with push notifications. |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@MobileMage Can you try this method to mock a push notification to the simulator? |
|
Hey @eh2077 I don't have access to the Slack channel yet. |
|
I see, please check below then
|
|
@MobileMage friendly bump! Can you also add unit tests for the change please? |
JmillsExpensify
left a comment
There was a problem hiding this comment.
Recently discussed in Slack. It's a good change in line with product spec.
When 'Mute all sounds from Expensify' is toggled on, iOS foreground notifications still played sound because the presentation options callback always returned the defaults (including Sound). Now checks getIsMuted() and excludes Sound from presentation options when muted. Fixes Expensify#81282
|
|
|
All contributors have signed the CLA ✍️ ✅ |
aa9bc5f to
a89b6d1
Compare
|
Sorry about the force push, I had accidentally merged main into the branch which bloated the PR to 1500+ commits. The force push just removed that merge commit — no actual changes were lost. |
|
I've also added unit tests for the change. @eh2077 I did try the |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
Explanation of Change
When the "Mute all sounds from Expensify" preference is enabled, iOS foreground push notifications still play a sound. This fix checks the mute preference in the
setForegroundPresentationOptionsCallbackand, when muted, explicitly returns onlyList,Banner, andBadgepresentation options (omittingSound), instead of returningnullwhich uses the default presentation (which includes sound).Fixed Issues
$ #81282
PROPOSAL: #81282 (comment)
Tests
Offline tests
N/A - This change only affects iOS foreground push notification presentation options, which require an active network connection to receive.
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari