Add Manage Callouts toggles for sound effects and delays - #247
Open
MBtheOtaku wants to merge 1 commit into
Open
Conversation
Adds two new settings under Manage Callouts: Sound Effects and Delays. Persists both toggles in app settings with defaults enabled. Wires settings UI logic to surface and handle the new switches alongside existing callout options. Applies Sound Effects toggle to callout prefix/earcon behavior in automatic destination/intersection/POI paths. Applies Delays toggle to automatic callout spacing by enabling/disabling callout delay timing. Adds new en-US localization keys for the new toggle labels and helper text. Keeps existing Allow Callouts collapse behavior intact while including the new rows when enabled
📝 WalkthroughWalkthroughThis PR adds configurable sound effects and delay settings for callouts throughout the iOS app. It introduces new localization entries, expands the settings protocol and storage layer, integrates these settings into callout generators, updates callout models to conditionally apply prefix sounds, and extends the settings UI with new toggle options. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ios/GuideDogs/Code/Visual UI/View Controllers/Settings/SettingsViewController.swift (1)
46-60:⚠️ Potential issue | 🔴 Critical
allCases.countnow exposes rows that still have no valid cell path.Line 108 makes the callouts section return every
CalloutsRow, but the dynamic path only creates cells for.soundEffectsand.delays. When.safetyor.intersectionis requested,identifierfalls back to"default"and Line 141 force-casts that cell toCalloutSettingsCellView, which will crash unless those rows get real reuse IDs or programmatic creation too.Also applies to: 102-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/GuideDogs/Code/Visual` UI/View Controllers/Settings/SettingsViewController.swift around lines 46 - 60, The callouts mapping in cellIdentifiers and the table data source are out of sync: CalloutsRow.allCases is being used to report rows but cellIdentifiers only supplies reuse IDs for .soundEffects and .delays, so asking for .safety or .intersection yields "default" and the force-cast to CalloutSettingsCellView will crash; fix by either (A) adding explicit reuse identifiers for the missing CalloutsRow cases in the cellIdentifiers dictionary (use the same reuse IDs you register) or (B) change the callouts section row count (the method returning numberOfRows for Section.callouts) to only return the rows you create, and/or make the dequeue logic in tableView(_:cellForRowAt:) (where identifier and the force-cast to CalloutSettingsCellView occur) safely handle unknown identifiers by returning a properly registered default cell instead of force-casting. Ensure the symbols affected are cellIdentifiers, CalloutsRow, numberOfRows/section logic, identifier resolution, and the CalloutSettingsCellView dequeue/cast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ios/GuideDogs/Code/Behaviors/Default/Callouts/DestinationCallout.swift`:
- Around line 55-60: The beacon/geofence earcon is still played unconditionally;
update the playback sites (within DestinationCallout where
GlyphSound(.beaconFound) is invoked, around the logic covering lines ~66–115) to
respect the callout flags by only playing the earcon when causedAudioDisabled is
false (and if applicable also respect includePrefixSound) — i.e., wrap the
GlyphSound(.beaconFound) playback call in a conditional that checks
!self.causedAudioDisabled (and includePrefixSound when that flag controls prefix
sounds) so the sound-effects toggle on the callout is honored.
In `@apps/ios/GuideDogs/Code/Visual`
UI/Controls/Settings/CalloutSettingsCellView.swift:
- Around line 15-23: The CalloutSettingCellType enum still contains merge
markers and is out of sync with SettingsViewController; replace the conflicted
enum with a final set of cases that matches the controller (include all,
soundEffects, delays, poi, beacon, shake, transportation, intersection, safety
and remove/replace .mobility), then update both switch statements that read and
write settings (the "read" switch and the "write" switch that reference
CalloutSettingCellType) to handle every new case and map any old .mobility
usages to the appropriate new case (likely .transportation) so all switch arms
are exhaustive and compilation warnings/errors are resolved; ensure switch
defaults are removed or adjusted so SettingsViewController’s routing
(.transportation, .safety, .intersection) is honored.
In `@apps/ios/GuideDogs/Code/Visual` UI/View
Controllers/Settings/SettingsViewController.swift:
- Around line 24-43: Remove all merge conflict markers and reconcile the two
CalloutsRow variants so the enum is defined once (prefer the stashed expanded
list if those rows are used elsewhere) by updating private enum CalloutsRow:
Int, CaseIterable to contain the correct cases (e.g., all, soundEffects, delays,
poi, mobility, safety, intersection, beacon, shake) and ensure no
duplicate/conflicting definitions remain; then edit the Section enum to either
remove any reference to case .telemetry or re-add a matching telemetry case (and
its associated logic) so references to .telemetry compile; finally build to
verify there are no remaining conflict markers or undefined enum cases in
SettingsViewController.
---
Outside diff comments:
In `@apps/ios/GuideDogs/Code/Visual` UI/View
Controllers/Settings/SettingsViewController.swift:
- Around line 46-60: The callouts mapping in cellIdentifiers and the table data
source are out of sync: CalloutsRow.allCases is being used to report rows but
cellIdentifiers only supplies reuse IDs for .soundEffects and .delays, so asking
for .safety or .intersection yields "default" and the force-cast to
CalloutSettingsCellView will crash; fix by either (A) adding explicit reuse
identifiers for the missing CalloutsRow cases in the cellIdentifiers dictionary
(use the same reuse IDs you register) or (B) change the callouts section row
count (the method returning numberOfRows for Section.callouts) to only return
the rows you create, and/or make the dequeue logic in tableView(_:cellForRowAt:)
(where identifier and the force-cast to CalloutSettingsCellView occur) safely
handle unknown identifiers by returning a properly registered default cell
instead of force-casting. Ensure the symbols affected are cellIdentifiers,
CalloutsRow, numberOfRows/section logic, identifier resolution, and the
CalloutSettingsCellView dequeue/cast.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 39ff5367-8a36-4312-ab03-18ad006dc9ee
📒 Files selected for processing (10)
apps/ios/GuideDogs/Assets/Localization/en-US.lproj/Localizable.stringsapps/ios/GuideDogs/Code/App/Settings/AutoCalloutSettingsProvider.swiftapps/ios/GuideDogs/Code/App/Settings/SettingsContext.swiftapps/ios/GuideDogs/Code/Behaviors/Default/AutoCalloutGenerator.swiftapps/ios/GuideDogs/Code/Behaviors/Default/BeaconCalloutGenerator.swiftapps/ios/GuideDogs/Code/Behaviors/Default/Callouts/DestinationCallout.swiftapps/ios/GuideDogs/Code/Behaviors/Default/Callouts/IntersectionCallout.swiftapps/ios/GuideDogs/Code/Behaviors/Default/IntersectionGenerator.swiftapps/ios/GuideDogs/Code/Visual UI/Controls/Settings/CalloutSettingsCellView.swiftapps/ios/GuideDogs/Code/Visual UI/View Controllers/Settings/SettingsViewController.swift
Comment on lines
+55
to
60
| init(_ calloutOrigin: CalloutOrigin, _ entityKey: String, _ causedAudioDisabled: Bool = false, includePrefixSound: Bool = true) { | ||
| self.origin = calloutOrigin | ||
| self.entityKey = entityKey | ||
| self.causedAudioDisabled = causedAudioDisabled | ||
| self.includePrefixSound = includePrefixSound | ||
| } |
There was a problem hiding this comment.
Apply the sound-effects toggle to the beacon-geofence earcon too.
The new flag is stored on the callout, but Line 94 still plays GlyphSound(.beaconFound) unconditionally when causedAudioDisabled is true. With Sound Effects turned off, that automatic destination path will still emit an earcon.
Suggested fix
if causedAudioDisabled {
- let earcon = GlyphSound(.beaconFound)
var text = GDLocalizedString("beacon.beacon_location_within_audio_beacon_muted", formattedDistance)
// Append suggestion to launch NaviLens if available at location
if (poi != nil) && LocationDetail(entity: poi!).source.hasNaviLens {
text += " " + GDLocalizedString("beacon.suggest_navilens")
}
let tts = TTSSound(text, at: markerLocation)
+
+ guard includePrefixSound else {
+ return Sounds(tts)
+ }
+
+ let earcon = GlyphSound(.beaconFound)
guard let layered = LayeredSound(earcon, tts) else {
return Sounds([earcon, tts])
}Also applies to: 66-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ios/GuideDogs/Code/Behaviors/Default/Callouts/DestinationCallout.swift`
around lines 55 - 60, The beacon/geofence earcon is still played
unconditionally; update the playback sites (within DestinationCallout where
GlyphSound(.beaconFound) is invoked, around the logic covering lines ~66–115) to
respect the callout flags by only playing the earcon when causedAudioDisabled is
false (and if applicable also respect includePrefixSound) — i.e., wrap the
GlyphSound(.beaconFound) playback call in a conditional that checks
!self.causedAudioDisabled (and includePrefixSound when that flag controls prefix
sounds) so the sound-effects toggle on the callout is honored.
Comment on lines
15
to
+23
| internal enum CalloutSettingCellType { | ||
| <<<<<<< Updated upstream | ||
| case all, poi, mobility, beacon, shake | ||
| ======= | ||
| case all, soundEffects, delays, poi, beacon, shake | ||
| case transportation | ||
| case intersection | ||
| case safety | ||
| >>>>>>> Stashed changes |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
file='apps/ios/GuideDogs/Code/Visual UI/Controls/Settings/CalloutSettingsCellView.swift'
controller='apps/ios/GuideDogs/Code/Visual UI/View Controllers/Settings/SettingsViewController.swift'
echo "Conflict markers in the cell view:"
rg -n '^(<<<<<<<|=======|>>>>>>>)' "$file"
echo
echo "Callout setting cases referenced by the cell view and controller:"
rg -n '\.(all|soundEffects|delays|poi|mobility|transportation|safety|intersection|beacon|shake)\b' "$file" "$controller"Repository: soundscape-community/soundscape
Length of output: 6958
🏁 Script executed:
cat -n 'apps/ios/GuideDogs/Code/Visual UI/Controls/Settings/CalloutSettingsCellView.swift' | sed -n '10,130p'Repository: soundscape-community/soundscape
Length of output: 5050
Finish the merge resolution and synchronize the enum with its switches.
The file contains unresolved merge conflicts at lines 16–23 (enum), 42–49 (read switch), and 88–98 (write switch). After resolution, line 53 references .mobility, while SettingsViewController.swift routes .transportation, .safety, and .intersection as separate cell types (lines 189–195). The enum definition and both switch statements must be aligned to match the controller's expectations. The file will not compile until conflicts are resolved and cases are synchronized.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ios/GuideDogs/Code/Visual`
UI/Controls/Settings/CalloutSettingsCellView.swift around lines 15 - 23, The
CalloutSettingCellType enum still contains merge markers and is out of sync with
SettingsViewController; replace the conflicted enum with a final set of cases
that matches the controller (include all, soundEffects, delays, poi, beacon,
shake, transportation, intersection, safety and remove/replace .mobility), then
update both switch statements that read and write settings (the "read" switch
and the "write" switch that reference CalloutSettingCellType) to handle every
new case and map any old .mobility usages to the appropriate new case (likely
.transportation) so all switch arms are exhaustive and compilation
warnings/errors are resolved; ensure switch defaults are removed or adjusted so
SettingsViewController’s routing (.transportation, .safety, .intersection) is
honored.
Comment on lines
+24
to
+43
| <<<<<<< Updated upstream | ||
| private enum CalloutsRow: Int, CaseIterable { | ||
| case all = 0 | ||
| case poi = 1 | ||
| case mobility = 2 | ||
| case beacon = 3 | ||
| case shake = 4 | ||
| ======= | ||
| /// Rows in the "Callouts" section | ||
| private enum CalloutsRow: Int, CaseIterable { | ||
| case all = 0 | ||
| case soundEffects = 1 | ||
| case delays = 2 | ||
| case poi = 3 | ||
| case mobility = 4 | ||
| case safety = 5 | ||
| case intersection = 6 | ||
| case beacon = 7 | ||
| case shake = 8 | ||
| >>>>>>> Stashed changes |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
file='apps/ios/GuideDogs/Code/Visual UI/View Controllers/Settings/SettingsViewController.swift'
echo "Conflict markers and likely compile blockers:"
rg -n '^(<<<<<<<|=======|>>>>>>>)|case \.telemetry|identifier \?\?' "$file"
echo
echo "Relevant excerpt:"
sed -n '24,170p' "$file"Repository: soundscape-community/soundscape
Length of output: 7433
🏁 Script executed:
rg -n 'enum Section' apps/ios/GuideDogs/Code/Visual\ UI/View\ Controllers/Settings/SettingsViewController.swift -A 15Repository: soundscape-community/soundscape
Length of output: 537
Resolve merge conflicts; do not merge the .telemetry case reference.
This file contains multiple unresolved merge conflict markers (lines 24, 68, 123, 145, 168) preventing compilation. Beyond those, line 150 references case .telemetry: in the Section enum, but .telemetry is commented out and not an active case—this causes a compile error in the stashed version. The file must have conflicts resolved and the .telemetry case removed or properly defined before this can build.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ios/GuideDogs/Code/Visual` UI/View
Controllers/Settings/SettingsViewController.swift around lines 24 - 43, Remove
all merge conflict markers and reconcile the two CalloutsRow variants so the
enum is defined once (prefer the stashed expanded list if those rows are used
elsewhere) by updating private enum CalloutsRow: Int, CaseIterable to contain
the correct cases (e.g., all, soundEffects, delays, poi, mobility, safety,
intersection, beacon, shake) and ensure no duplicate/conflicting definitions
remain; then edit the Section enum to either remove any reference to case
.telemetry or re-add a matching telemetry case (and its associated logic) so
references to .telemetry compile; finally build to verify there are no remaining
conflict markers or undefined enum cases in SettingsViewController.
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
1 participant
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.
Adds two new settings under Manage Callouts: Sound Effects and Delays. Persists both toggles in app settings with defaults enabled. Wires settings UI logic to surface and handle the new switches alongside existing callout options. Applies Sound Effects toggle to callout prefix/earcon behavior in automatic destination/intersection/POI paths. Applies Delays toggle to automatic callout spacing by enabling/disabling callout delay timing. Adds new en-US localization keys for the new toggle labels and helper text. Keeps existing Allow Callouts collapse behavior intact while including the new rows when enabled
Summary by CodeRabbit