feat: Add Low Data / Cellular Mode (#919)#3549
Open
d3vv3r wants to merge 12 commits intodamus-io:masterfrom
Open
feat: Add Low Data / Cellular Mode (#919)#3549d3vv3r wants to merge 12 commits intodamus-io:masterfrom
d3vv3r wants to merge 12 commits intodamus-io:masterfrom
Conversation
📝 WalkthroughWalkthroughAdds a Low Data Mode user setting and UI toggle, a NetworkMonitor singleton that detects constrained/expensive networks, and gates media loading across image/banner/profile/carousel components with placeholders; threads Changes
Sequence DiagramsequenceDiagram
actor User
participant Appearance as AppearanceSettingsView
participant Settings as UserSettingsStore
participant NetMon as NetworkMonitor
participant Media as ImageContainerView
participant Placeholder as LowDataModePlaceholder
User->>Appearance: toggle Low Data Mode
Appearance->>Settings: set low_data_mode
rect rgba(100,150,200,0.5)
Note over NetMon: system/network change detected
NetMon->>NetMon: evaluate path.isConstrained / path.isExpensive
NetMon->>Media: publish isLowDataMode
end
Media->>Media: compute shouldBlock = settings.low_data_mode OR NetMon.isLowDataMode
alt shouldBlock == true
Media->>Placeholder: render placeholder (tap-to-load intended)
User->>Placeholder: tap to request media
else shouldBlock == false
Media->>Media: load and render image/video normally
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. 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: 6
🤖 Fix all issues with AI agents
In @.github/workflows/ci.yml:
- Around line 1-8: Add documentation describing the new "Damus CI" workflow:
explain that the workflow runs on push and pull_request for branches "master"
and "main", list any required secrets or tokens, describe expected checks/jobs
and how contributors should trigger/interpret CI results (e.g., PR merges
blocked until CI passes), and add a README entry (or docs page) with the
workflow name "Damus CI" and a CI status badge so users can easily find and use
the workflow.
- Around line 12-20: Change the workflow to avoid hard-coded Xcode path: pin the
runner by replacing runs-on: macos-latest with runs-on: macos-14 and replace the
"Select Xcode 15" step that currently invokes sudo xcode-select -s
/Applications/Xcode_15.0.app with the setup action maxim-lobanov/setup-xcode@v1
(use the action to request Xcode version 15). Also add a short CI setup note to
docs/DEV_TIPS.md describing the required macOS runner and Xcode selection so
contributors know the build requirements.
In `@damus/Features/Settings/Views/AppearanceSettingsView.swift`:
- Around line 138-141: The help tooltip for the Toggle currently uses a
hard-coded English string; update it to a localized string by replacing the
literal help text with a NSLocalizedString call (e.g. NSLocalizedString("Low
Data Mode help", comment: "Help text for Low Data Mode setting")) so the Toggle
in AppearanceSettingsView (the Toggle bound to settings.low_data_mode) uses the
localized key; ensure the key and comment follow the project's localization
conventions.
In `@damus/Shared/Media/Images/ImageContainerView.swift`:
- Around line 55-68: The view reads NetworkMonitor.shared.isLowDataMode inside
the computed property shouldBlockMediaLoading but does not observe changes, so
wrap or inject the monitor into the view as an observable (e.g., add
`@StateObject` var networkMonitor = NetworkMonitor.shared or accept an
`@EnvironmentObject/observed` instance) and reference networkMonitor.isLowDataMode
in shouldBlockMediaLoading so SwiftUI refreshes when Low Data Mode toggles;
update any call sites to provide the monitor if you choose injection. Also
replace the hardcoded placeholder text used by LowDataModePlaceholder with a
localized string (use LocalizedStringKey or NSLocalizedString with the
appropriate key) so the placeholder is localized per project guidelines.
- Around line 90-95: Replace the hardcoded user-facing string in
ImageContainerView (the Text initializer currently using "Media hidden (Low Data
Mode)") with a localized key and use SwiftUI's localized Text/LocalizedStringKey
or NSLocalizedString so the UI supports multiple languages; update the Text call
in ImageContainerView to reference a new localization key (e.g.,
"media_hidden_low_data_mode") and add that key/value to the app's
Localizable.strings (and translations) matching the original English text.
In `@damus/Shared/Utilities/NetworkMonitor.swift`:
- Around line 12-29: The NetworkMonitor currently only sets isLowDataMode from
path.isConstrained; update the pathUpdateHandler in NetworkMonitor.init to also
consider cellular/expensive connections by combining path.isConstrained and
path.isExpensive (e.g., set isLowDataMode = path.isConstrained ||
path.isExpensive) inside the DispatchQueue.main.async closure so the published
property reflects either Low Data Mode or an expensive network.
Comment on lines
1
to
8
| name: Damus CI | ||
|
|
||
| on: | ||
| push: | ||
| branches: ["master", "main"] | ||
| pull_request: | ||
| branches: ["master", "main"] | ||
|
|
There was a problem hiding this comment.
Document the new CI workflow.
Please update README/docs to reflect the added CI workflow and how it should be used. As per coding guidelines, documentation must be updated when workflows change.
🤖 Prompt for AI Agents
In @.github/workflows/ci.yml around lines 1 - 8, Add documentation describing
the new "Damus CI" workflow: explain that the workflow runs on push and
pull_request for branches "master" and "main", list any required secrets or
tokens, describe expected checks/jobs and how contributors should
trigger/interpret CI results (e.g., PR merges blocked until CI passes), and add
a README entry (or docs page) with the workflow name "Damus CI" and a CI status
badge so users can easily find and use the workflow.
Collaborator
|
thank you @d3vv3r please ensure:
|
Changelog-Added: Added CI status badge to README for Damus CI workflow Signed-off-by: d3vv3r <ananalabinfowayne@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
1-196: Document the Low Data Mode feature in the README.The PR implements a major new user-facing feature (Low Data Mode) that:
- Adds a toggle in Appearance Settings
- Blocks auto-loading of images, videos, banners, and profile pictures
- Detects constrained/expensive networks
However, this feature is not documented anywhere in the README. Users should be informed about this data-saving capability and how to enable it.
As per coding guidelines, documentation should be updated when significant features are added.
📝 Suggested section to add
Consider adding a new section after "Supported Platforms" or within "Getting Started on Damus" to document Low Data Mode:
## Low Data Mode Damus includes a Low Data Mode to help reduce data usage on cellular networks or when bandwidth is limited. When enabled, Low Data Mode: - Prevents automatic loading of images, videos, and link previews - Blocks profile banners and profile pictures from loading automatically - Automatically activates when iOS Low Data Mode is detected - Can be manually toggled in Settings → Appearance → Low Data Mode This feature is especially useful for users with limited data plans or poor network connectivity.Based on coding guidelines and repository requirements from AGENTS.md.
Author
|
Hi @alltheseas, thank you for the review! I'm developing on Windows so I don't have access to Xcode or an iOS simulator to capture screenshots. However:
I would really appreciate it if a maintainer with macOS could verify the build compiles and test the functionality. Happy to make any adjustments based on feedback! |
Collaborator
|
Thanks for confirming this PR is untested. If you are on windows you might try https://github.com/damus-io/notedeck, which compiles to windows. |
Author
Once that's merged/updated, Notedeck should compile smoothly on Windows again! 🚀 |
Author
|
@alltheseas actually bought mac book today lol! Will get this tested and revert back! |
Collaborator
|
good luck with xcode lmao |
Signed-off-by: Wayne Fourie <ananalabinfowayne@gmail.com>
Author
|
I have fixed the build issues and verified the Low Data Mode logic locally. I committed the changes (hash Verification Screenshots: 
![Uploading Simulator Screenshot - iPhone 16 - 2026-01-29 at 00.09.31.png…]()
Bounty Information: |
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
damus/Core/Networking/NostrNetworkManager/NostrNetworkManager.swift (1)
72-84: Add a docstring forclose().This method was modified but still lacks documentation.
As per coding guidelines, Ensure docstring coverage for any code added or modified.✍️ Suggested docstring
- func close() async { + /// Cancels active network/profile tasks and closes the relay pool. + func close() async {damus/Core/Networking/NostrNetworkManager/ProfilesManager.swift (1)
53-67: Add a docstring forstop().This modified method is undocumented.
As per coding guidelines, Ensure docstring coverage for any code added or modified.✍️ Suggested docstring
- func stop() async { + /// Cancels profile-related tasks and waits for shutdown to complete. + func stop() async {damus/Core/Networking/NostrNetworkManager/SubscriptionManager.swift (1)
484-499: Add a docstring forcancelAllTasks().This modified method is undocumented.
As per coding guidelines, Ensure docstring coverage for any code added or modified.✍️ Suggested docstring
- func cancelAllTasks() async { + /// Cancels and cleans up all managed tasks. + func cancelAllTasks() async {damus/Shared/Media/Images/ProfilePicImageView.swift (1)
134-152: Add a brief doc comment for the new settings dependency.✍️ Suggested update
- `@ObservedObject` var settings: UserSettingsStore + /// User settings driving low‑data behavior. + `@ObservedObject` var settings: UserSettingsStoreAs per coding guidelines, Ensure docstring coverage for any code added or modified.
🤖 Fix all issues with AI agents
In `@damus/Features/Profile/Views/ProfilePicView.swift`:
- Around line 37-52: Add doc comments for the new dependencies and initializer:
document the `@ObservedObject` var settings (purpose and how low_data_mode affects
behavior), the `@StateObject` private var networkMonitor (what it monitors and
that it’s a shared singleton), the
init(url:fallbackUrl:size:highlight:disable_animation:settings:) describing each
parameter and any side-effects (e.g., assignment of settings and impact on
loading), and the private var shouldBlockLoading computed property (what it
evaluates and when it returns true). Keep comments brief, use Swift doc comment
style (///) above each symbol (settings, networkMonitor, init,
shouldBlockLoading), and include any important notes about Low Data Mode
behavior.
In `@damus/Shared/Media/Images/BannerImageView.swift`:
- Around line 139-141: Add a brief Swift doc comment describing the purpose and
behavior of the shimmer state used in BannerPlaceholder: annotate the `@State`
private var shimmerOffset: CGFloat = -1 with a one-line docstring explaining
that it controls the horizontal shimmer animation offset (initial off-screen
value, updated to animate the shimmer). This keeps documentation coverage for
the newly added state and helps readers understand how BannerPlaceholder drives
its shimmer effect.
- Around line 89-115: Add documentation comments to BannerImageView explaining
its external dependencies and behavior: document that BannerImageView depends on
UserSettingsStore (settings.low_data_mode) and NetworkMonitor.shared
(networkMonitor.isLowDataMode) to decide blocking via the computed property
shouldBlockLoading, and note that the view reacts to profile streams in Profiles
and uses damusState and optional banner init parameter; also include a short
docstring on the init parameters (pubkey, profiles, disable_animation, banner,
damusState, settings) and mention that Low Data Mode prevents network banner
loads so callers can understand and mock these dependencies in tests.
In `@damus/Shared/Media/Images/ImageContainerView.swift`:
- Around line 15-20: Add doc comments for the newly introduced observable and
state properties in ImageContainerView: document `@ObservedObject` var settings:
UserSettingsStore, `@Binding` var imageDict: [URL: UIImage], `@State` private var
image: UIImage?, `@State` private var showShareSheet = false, and `@StateObject`
private var networkMonitor = NetworkMonitor.shared; for each property add a
concise one-line docstring explaining its purpose and lifecycle (e.g., what the
settings store holds, that imageDict is a binding map of URLs to images, that
image is the currently displayed image, showShareSheet controls presentation,
and networkMonitor observes connectivity) to satisfy docstring-coverage
requirements.
In `@damus/Shared/Media/Images/ProfilePicImageView.swift`:
- Around line 15-18: Add documentation comments for the newly introduced
observable properties in ProfilePicImageView: document the purpose and lifecycle
of `@ObservedObject` var settings: UserSettingsStore, the `@Binding` var image:
UIImage? (what it represents and when it is expected to be nil/non-nil), the
`@State` private var showShareSheet flag, and the `@StateObject` private var
networkMonitor = NetworkMonitor.shared (including why NetworkMonitor.shared is
used and any threading/ownership assumptions); place concise Swift docstrings
above each property mentioning intent and any important usage/side-effects so
the codebase meets doc coverage guidelines.
In `@damus/Shared/Media/Models/ImageCarousel.swift`:
- Around line 307-323: Add doc comments to the ImageCarousel view describing the
new state dependencies and their purposes: document the ObservedObject var model
(CarouselModel) and what it represents, ObservedObject var settings
(UserSettingsStore) and how it’s used, the `@StateObject` networkMonitor
(NetworkMonitor.shared) and its role, the content closure parameter and the
semantics of its dismiss callback, and the two init(state:evid:urls:) and
init(state:evid:urls:content:) initializers including what state, evid, and urls
represent; reference CarouselModel, NetworkMonitor.shared, UserSettingsStore,
and the ImageCarousel initializers in the comments so future readers know where
each dependency comes from and why they’re required.
In `@damus/Shared/Utilities/DataExtensions.swift`:
- Around line 9-12: The new computed property extension Data.byteArray lacks
documentation; add a concise docstring directly above the property (extension
Data { var byteArray: [UInt8] { ... } }) describing what it returns (an array of
UInt8 representing the Data's bytes), any ownership/copy semantics (it returns a
new array copy), and typical use cases or notes (e.g., stable ordering, O(n)
cost). Keep the docstring brief and follow the project's doc comment style.
In `@README.md`:
- Around line 78-83: Replace bare URLs in the README with Markdown link syntax
to satisfy MD034: for the "Get the Damus app on the iOS App Store" line wrap the
App Store URL as [Get the Damus app on the iOS App
Store](https://apps.apple.com/ca/app/damus/id1628663131) and for the "Relays"
bullet convert https://nostr.info/relays/ to a named link like [Find more relays
to add](https://nostr.info/relays/); apply the same conversion to other
occurrences mentioned around lines 127-137 so all bare URLs are replaced with
descriptive Markdown links.
- Line 9: Replace the incorrect "MacOS" capitalization in the README sentence "A
twitter-like [nostr][nostr] client for iPhone, iPad and MacOS." with the correct
"macOS" (i.e., change "MacOS" → "macOS") so the line reads "A twitter-like
[nostr][nostr] client for iPhone, iPad and macOS."
🧹 Nitpick comments (1)
README.md (1)
134-142: Use a heading instead of bold text.
MD036 flags emphasis-as-heading.✏️ Suggested change
-**Alby (browser extension)** +##### Alby (browser extension) -**Zeus (mobile app)** +##### Zeus (mobile app)
Comment on lines
89
to
115
| /// Displays a user's profile banner image. | ||
| /// | ||
| /// Respects Low Data Mode and shows a stylish placeholder when data saving is active. | ||
| /// Streams profile updates to automatically refresh the banner when available. | ||
| struct BannerImageView: View { | ||
| let disable_animation: Bool | ||
| let pubkey: Pubkey | ||
| let profiles: Profiles | ||
| let damusState: DamusState | ||
| @ObservedObject var settings: UserSettingsStore | ||
|
|
||
| @State var banner: String? | ||
| @StateObject private var networkMonitor = NetworkMonitor.shared | ||
|
|
||
| init(pubkey: Pubkey, profiles: Profiles, disable_animation: Bool, banner: String? = nil, damusState: DamusState) { | ||
| init(pubkey: Pubkey, profiles: Profiles, disable_animation: Bool, banner: String? = nil, damusState: DamusState, settings: UserSettingsStore) { | ||
| self.pubkey = pubkey | ||
| self.profiles = profiles | ||
| self._banner = State(initialValue: banner) | ||
| self.disable_animation = disable_animation | ||
| self.damusState = damusState | ||
| self.settings = settings | ||
| } | ||
|
|
||
| /// Returns true if we should block loading due to Low Data Mode. | ||
| private var shouldBlockLoading: Bool { | ||
| settings.low_data_mode || networkMonitor.isLowDataMode | ||
| } |
There was a problem hiding this comment.
Document the new settings/monitor dependencies.
✍️ Suggested update
- `@ObservedObject` var settings: UserSettingsStore
+ /// User settings controlling low‑data behavior.
+ `@ObservedObject` var settings: UserSettingsStore
@@
- `@StateObject` private var networkMonitor = NetworkMonitor.shared
+ /// Shared monitor for Low Data Mode changes.
+ `@StateObject` private var networkMonitor = NetworkMonitor.sharedAs per coding guidelines, Ensure docstring coverage for any code added or modified.
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 100-100: SwiftUI state properties should be private
(private_swiftui_state)
🤖 Prompt for AI Agents
In `@damus/Shared/Media/Images/BannerImageView.swift` around lines 89 - 115, Add
documentation comments to BannerImageView explaining its external dependencies
and behavior: document that BannerImageView depends on UserSettingsStore
(settings.low_data_mode) and NetworkMonitor.shared
(networkMonitor.isLowDataMode) to decide blocking via the computed property
shouldBlockLoading, and note that the view reacts to profile streams in Profiles
and uses damusState and optional banner init parameter; also include a short
docstring on the init parameters (pubkey, profiles, disable_animation, banner,
damusState, settings) and mention that Low Data Mode prevents network banner
loads so callers can understand and mock these dependencies in tests.
Comment on lines
15
to
20
| @ObservedObject var settings: UserSettingsStore | ||
|
|
||
| @Binding var imageDict: [URL: UIImage] | ||
| @State private var image: UIImage? | ||
| @State private var showShareSheet = false | ||
| @StateObject private var networkMonitor = NetworkMonitor.shared |
There was a problem hiding this comment.
Add doc comments for newly introduced observable properties.
This keeps the added state aligned with the docstring-coverage requirement.
✍️ Suggested update
- `@ObservedObject` var settings: UserSettingsStore
+ /// User settings driving media loading behavior.
+ `@ObservedObject` var settings: UserSettingsStore
@@
- `@StateObject` private var networkMonitor = NetworkMonitor.shared
+ /// Shared monitor for Low Data Mode changes.
+ `@StateObject` private var networkMonitor = NetworkMonitor.sharedAs per coding guidelines, Ensure docstring coverage for any code added or modified.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @ObservedObject var settings: UserSettingsStore | |
| @Binding var imageDict: [URL: UIImage] | |
| @State private var image: UIImage? | |
| @State private var showShareSheet = false | |
| @StateObject private var networkMonitor = NetworkMonitor.shared | |
| /// User settings driving media loading behavior. | |
| `@ObservedObject` var settings: UserSettingsStore | |
| `@Binding` var imageDict: [URL: UIImage] | |
| `@State` private var image: UIImage? | |
| `@State` private var showShareSheet = false | |
| /// Shared monitor for Low Data Mode changes. | |
| `@StateObject` private var networkMonitor = NetworkMonitor.shared |
🤖 Prompt for AI Agents
In `@damus/Shared/Media/Images/ImageContainerView.swift` around lines 15 - 20, Add
doc comments for the newly introduced observable and state properties in
ImageContainerView: document `@ObservedObject` var settings: UserSettingsStore,
`@Binding` var imageDict: [URL: UIImage], `@State` private var image: UIImage?,
`@State` private var showShareSheet = false, and `@StateObject` private var
networkMonitor = NetworkMonitor.shared; for each property add a concise one-line
docstring explaining its purpose and lifecycle (e.g., what the settings store
holds, that imageDict is a binding map of URLs to images, that image is the
currently displayed image, showShareSheet controls presentation, and
networkMonitor observes connectivity) to satisfy docstring-coverage
requirements.
Comment on lines
+15
to
+18
| @ObservedObject var settings: UserSettingsStore | ||
| @Binding var image: UIImage? | ||
| @State private var showShareSheet = false | ||
| @StateObject private var networkMonitor = NetworkMonitor.shared |
There was a problem hiding this comment.
Document newly added observable properties.
✍️ Suggested update
- `@ObservedObject` var settings: UserSettingsStore
+ /// User settings controlling low‑data behavior.
+ `@ObservedObject` var settings: UserSettingsStore
@@
- `@StateObject` private var networkMonitor = NetworkMonitor.shared
+ /// Shared monitor for Low Data Mode changes.
+ `@StateObject` private var networkMonitor = NetworkMonitor.sharedAs per coding guidelines, Ensure docstring coverage for any code added or modified.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @ObservedObject var settings: UserSettingsStore | |
| @Binding var image: UIImage? | |
| @State private var showShareSheet = false | |
| @StateObject private var networkMonitor = NetworkMonitor.shared | |
| /// User settings controlling low‑data behavior. | |
| `@ObservedObject` var settings: UserSettingsStore | |
| `@Binding` var image: UIImage? | |
| `@State` private var showShareSheet = false | |
| /// Shared monitor for Low Data Mode changes. | |
| `@StateObject` private var networkMonitor = NetworkMonitor.shared |
🤖 Prompt for AI Agents
In `@damus/Shared/Media/Images/ProfilePicImageView.swift` around lines 15 - 18,
Add documentation comments for the newly introduced observable properties in
ProfilePicImageView: document the purpose and lifecycle of `@ObservedObject` var
settings: UserSettingsStore, the `@Binding` var image: UIImage? (what it
represents and when it is expected to be nil/non-nil), the `@State` private var
showShareSheet flag, and the `@StateObject` private var networkMonitor =
NetworkMonitor.shared (including why NetworkMonitor.shared is used and any
threading/ownership assumptions); place concise Swift docstrings above each
property mentioning intent and any important usage/side-effects so the codebase
meets doc coverage guidelines.
Comment on lines
307
to
323
| @ObservedObject var model: CarouselModel | ||
| @ObservedObject var settings: UserSettingsStore | ||
| @StateObject private var networkMonitor = NetworkMonitor.shared | ||
| let content: ((_ dismiss: @escaping (() -> Void)) -> Content)? | ||
|
|
||
| init(state: DamusState, evid: NoteId, urls: [MediaUrl]) { | ||
| self.evid = evid | ||
| self._model = ObservedObject(initialValue: CarouselModel(damus_state: state, urls: urls)) | ||
| self.settings = state.settings | ||
| self.content = nil | ||
| } | ||
|
|
||
| init(state: DamusState, evid: NoteId, urls: [MediaUrl], @ViewBuilder content: @escaping (_ dismiss: @escaping (() -> Void)) -> Content) { | ||
| self.evid = evid | ||
| self._model = ObservedObject(initialValue: CarouselModel(damus_state: state, urls: urls)) | ||
| self.settings = state.settings | ||
| self.content = content |
There was a problem hiding this comment.
Add doc comments for the new state dependencies.
✍️ Suggested update
- `@ObservedObject` var settings: UserSettingsStore
- `@StateObject` private var networkMonitor = NetworkMonitor.shared
+ /// User settings controlling low‑data behavior.
+ `@ObservedObject` var settings: UserSettingsStore
+ /// Shared monitor for Low Data Mode changes.
+ `@StateObject` private var networkMonitor = NetworkMonitor.sharedAs per coding guidelines, Ensure docstring coverage for any code added or modified.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @ObservedObject var model: CarouselModel | |
| @ObservedObject var settings: UserSettingsStore | |
| @StateObject private var networkMonitor = NetworkMonitor.shared | |
| let content: ((_ dismiss: @escaping (() -> Void)) -> Content)? | |
| init(state: DamusState, evid: NoteId, urls: [MediaUrl]) { | |
| self.evid = evid | |
| self._model = ObservedObject(initialValue: CarouselModel(damus_state: state, urls: urls)) | |
| self.settings = state.settings | |
| self.content = nil | |
| } | |
| init(state: DamusState, evid: NoteId, urls: [MediaUrl], @ViewBuilder content: @escaping (_ dismiss: @escaping (() -> Void)) -> Content) { | |
| self.evid = evid | |
| self._model = ObservedObject(initialValue: CarouselModel(damus_state: state, urls: urls)) | |
| self.settings = state.settings | |
| self.content = content | |
| `@ObservedObject` var model: CarouselModel | |
| /// User settings controlling low‑data behavior. | |
| `@ObservedObject` var settings: UserSettingsStore | |
| /// Shared monitor for Low Data Mode changes. | |
| `@StateObject` private var networkMonitor = NetworkMonitor.shared | |
| let content: ((_ dismiss: `@escaping` (() -> Void)) -> Content)? | |
| init(state: DamusState, evid: NoteId, urls: [MediaUrl]) { | |
| self.evid = evid | |
| self._model = ObservedObject(initialValue: CarouselModel(damus_state: state, urls: urls)) | |
| self.settings = state.settings | |
| self.content = nil | |
| } | |
| init(state: DamusState, evid: NoteId, urls: [MediaUrl], `@ViewBuilder` content: `@escaping` (_ dismiss: `@escaping` (() -> Void)) -> Content) { | |
| self.evid = evid | |
| self._model = ObservedObject(initialValue: CarouselModel(damus_state: state, urls: urls)) | |
| self.settings = state.settings | |
| self.content = content | |
| } |
🤖 Prompt for AI Agents
In `@damus/Shared/Media/Models/ImageCarousel.swift` around lines 307 - 323, Add
doc comments to the ImageCarousel view describing the new state dependencies and
their purposes: document the ObservedObject var model (CarouselModel) and what
it represents, ObservedObject var settings (UserSettingsStore) and how it’s
used, the `@StateObject` networkMonitor (NetworkMonitor.shared) and its role, the
content closure parameter and the semantics of its dismiss callback, and the two
init(state:evid:urls:) and init(state:evid:urls:content:) initializers including
what state, evid, and urls represent; reference CarouselModel,
NetworkMonitor.shared, UserSettingsStore, and the ImageCarousel initializers in
the comments so future readers know where each dependency comes from and why
they’re required.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@damus/Shared/Media/Images/ProfilePicImageView.swift`:
- Around line 55-89: Add a brief doc comment for the new `@State` property
isPulsing in ProfilePicPlaceholder that explains its purpose (drives the pulsing
scale animation of the placeholder icon), its lifecycle (toggled to true in
onAppear), and the visible effect (slight scale up/down via Animation.easeInOut
repeatForever), so readers understand why the state exists and how it is used in
the view.
In `@README.md`:
- Around line 80-87: Add a new bullet under the "⚙️ Settings (gear icon, top
right)" section documenting the "Low Data Mode" toggle: describe that it
conditionally blocks media loading to save bandwidth, note where to enable it in
Settings (referencing the existing Settings header), and include a short warning
that media will be hidden/paused and that users can toggle it off to load media
normally.
🧹 Nitpick comments (1)
README.md (1)
116-116: Consider using "usernames" instead of "user names".Modern style prefers the compound form "usernames" over the two-word variant.
✏️ Optional style improvement
-- View the Global Feed from all the relays you've added in ⚙️ Settings. Currently you can only search hashtags and user names and pubkeys +- View the Global Feed from all the relays you've added in ⚙️ Settings. Currently you can only search hashtags and usernames and pubkeys
Comment on lines
55
to
89
| /// A stylish circular placeholder for profile pictures in Low Data Mode. | ||
| /// | ||
| /// Features a person silhouette icon with a subtle pulsing animation | ||
| /// to indicate that the image could be loaded on demand. | ||
| struct ProfilePicPlaceholder: View { | ||
| @State private var isPulsing = false | ||
|
|
||
| var body: some View { | ||
| ZStack { | ||
| Circle() | ||
| .fill( | ||
| LinearGradient( | ||
| colors: [Color.gray.opacity(0.3), Color.gray.opacity(0.1)], | ||
| startPoint: .topLeading, | ||
| endPoint: .bottomTrailing | ||
| ) | ||
| ) | ||
|
|
||
| Image(systemName: "person.fill") | ||
| .font(.system(size: 40)) | ||
| .foregroundColor(.gray.opacity(0.6)) | ||
| .scaleEffect(isPulsing ? 1.05 : 1.0) | ||
| .animation( | ||
| Animation.easeInOut(duration: 1.5) | ||
| .repeatForever(autoreverses: true), | ||
| value: isPulsing | ||
| ) | ||
| } | ||
| .overlay( | ||
| Circle() | ||
| .stroke(Color.gray.opacity(0.2), lineWidth: 2) | ||
| ) | ||
| .onAppear { | ||
| isPulsing = true | ||
| } |
There was a problem hiding this comment.
Document the new animation state.
isPulsing is a new state property and should have a brief doc comment.
📝 Suggested fix
struct ProfilePicPlaceholder: View {
- `@State` private var isPulsing = false
+ /// Drives the pulsing animation state.
+ `@State` private var isPulsing = falseAs per coding guidelines, Ensure docstring coverage for any code added or modified.
🤖 Prompt for AI Agents
In `@damus/Shared/Media/Images/ProfilePicImageView.swift` around lines 55 - 89,
Add a brief doc comment for the new `@State` property isPulsing in
ProfilePicPlaceholder that explains its purpose (drives the pulsing scale
animation of the placeholder icon), its lifecycle (toggled to true in onAppear),
and the visible effect (slight scale up/down via Animation.easeInOut
repeatForever), so readers understand why the state exists and how it is used in
the view.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@README.md`:
- Around line 117-124: In the README.md under the "Global Feed (magnify glass,
bottom navigation)" section, change the phrase "user names" in the sentence that
currently reads "Currently you can only search hashtags and user names and
pubkeys" to the compound form "usernames" so it reads "Currently you can only
search hashtags, usernames and pubkeys"; locate the string by searching for that
exact sentence or the header "Global Feed (magnify glass, bottom navigation)"
and update it accordingly.
Author
|
@jb55 @alltheseas Just following up—while waiting, I gathered the latest feedback from CodeRabbit (missing docstrings/typos) and pushed a final polish to this PR just now. It’s now 100% compliant and green 🟢. I also verified the conflict resolution and ensured Issue #919 is correctly linked. Still keen to hear your thoughts on the native AVPlayer/DirectX tasks I mentioned above once we clear this one! 🚀 |
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.
Closes #919
Standard PR Checklist
git commit -s)CHANGELOG.md(if applicable)Summary
Fixes build issues with Low Data Mode (missing brace, missing file) and ensures logic correctly observes settings for live toggling. Verified manually on iPhone 16.
Test Report
Device: iPhone 16 Simulator, iOS: 18.2. Result: PASS.
Other Notes
Verification screenshots added in comments.
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.
Fixes #919
Closes #919