-
-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(app): widgets #54
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
fd8ca95
to
2a5111b
Compare
bbef970
to
5ca40b9
Compare
Warning Rate limit exceeded@krystxf has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 5 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe pull request introduces significant updates to the In the application code, a new import statement for Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 17
🧹 Outside diff range and nitpick comments (16)
apps/mobile/metro-now/widgets/Info.plist (1)
7-8
: Enhance the location usage descriptionThe current description "Show the closest metro stop" is quite brief. Consider providing a more detailed explanation to help users understand why location access is needed and how it benefits them.
- <string>Show the closest metro stop</string> + <string>Your location is used to find and display departure times for the nearest metro stops, helping you quickly access relevant transit information.</string>apps/mobile/metro-now/widgets/departures/DeparturesWidgetManager.swift (1)
72-72
: Remove debug print statementThe
Apply this diff to fix:
- print("API Request URL for Departures: \(url)")
apps/mobile/metro-now/widgets/frequency/FrequencyWidgetTimelineProvider.swift (1)
103-104
: Remove debug print statementsThe
Apply this diff to fix:
- print("entries") - print(entries.count)apps/mobile/metro-now/widgets/widgetsBundle.swift (1)
8-8
: Use PascalCase for struct names to follow Swift naming conventionsStruct names should be in PascalCase. Rename
widgetsBundle
toWidgetsBundle
for consistency.Apply this diff to fix:
-struct widgetsBundle: WidgetBundle { +struct WidgetsBundle: WidgetBundle {Also, update any references to
widgetsBundle
in the code.apps/mobile/metro-now/widgets/frequency/FrequencyWidgetTimelineEntry.swift (1)
6-10
: Add documentation comments for the public struct and its properties.While the code is well-structured, adding documentation comments would improve API clarity. Consider adding:
- A description of the struct's purpose
- Property documentation explaining the role of each field
- Example usage if applicable
Here's a suggested improvement:
+/// A timeline entry representing the frequency of metro departures at a specific stop. struct FrequencyWidgetTimelineEntry: TimelineEntry { + /// The date when this timeline entry is valid. let date: Date + /// The name of the metro stop. let stopName: String + /// The time interval between departures, measured in seconds. let frequency: TimeInterval // in seconds }apps/mobile/metro-now/widgets/departures/DeparturesWidgetTimelineEntry.swift (1)
7-12
: Add documentation comments and consider using a structured type for departures.While the structure is well-defined, consider these improvements:
- Add documentation comments for better API clarity
- Consider using a more structured type for departures instead of string array
Here's a suggested improvement:
+/// A timeline entry representing upcoming departures from the closest metro stop. struct DeparturesWidgetTimelineEntry: TimelineEntry { + /// The date when this timeline entry is valid. let date: Date + /// The name of the closest metro stop. let closestStop: String + /// List of upcoming departures. let departures: [String] + /// The current location of the user, if available. let location: CLLocation? } +/// A structured representation of a departure +struct Departure { + let time: Date + let destination: String + let line: String + let platform: String? +}This would allow for more detailed departure information and type safety.
apps/mobile/metro-now/metro-now/pages/settings/subpages/settings-widgets.swift (1)
6-24
: Consider enhancing widget configuration options.While the current implementation provides clear information about widget limitations, consider adding:
- Visual previews of available widgets
- Configuration options for users (e.g., preferred stations)
- Accessibility labels for VoiceOver support
Example enhancement for accessibility:
Label( "Widgets don't show real-time data", systemImage: "exclamationmark.triangle" ) +.accessibilityLabel("Warning about widget limitations")
apps/mobile/metro-now/widgets/frequency/FrequencyWidget.swift (2)
21-23
: Consider supporting additional widget sizes.The widget currently only supports .systemSmall size. Consider if medium or large sizes could provide additional valuable information to users, such as multiple station frequencies or departure times.
27-35
: Add more preview scenarios.While the current preview is helpful, consider adding more scenarios to test edge cases:
- Long station names
- Different frequency ranges (very short/long)
- Error states
apps/mobile/metro-now/widgets/frequency/FrequencyWidgetView.swift (1)
8-17
: Consider making formatter static.The DateComponentsFormatter configuration is identical for all instances. Consider making it static to improve performance and reduce memory usage.
- private let formatter: DateComponentsFormatter + private static let formatter: DateComponentsFormatter = { + let formatter = DateComponentsFormatter() + formatter.unitsStyle = .brief + formatter.allowedUnits = [.hour, .minute, .second] + formatter.maximumUnitCount = 2 + return formatter + }() init(entry: FrequencyWidgetTimelineProvider.Entry) { self.entry = entry - formatter = DateComponentsFormatter() - - formatter.unitsStyle = .brief - formatter.allowedUnits = [.hour, .minute, .second] - formatter.maximumUnitCount = 2 }apps/mobile/metro-now/widgets/departures/DeparturesWidget.swift (2)
8-26
: Consider adding widget refresh rate configurationThe widget configuration looks good overall, but it might benefit from explicit refresh rate settings to optimize battery life and data freshness.
Add refresh interval configuration:
StaticConfiguration(kind: kind, provider: DeparturesWidgetTimelineProvider()) { entry in + .refreshInterval(.minutes(5)) // Adjust interval based on metro schedule updates
28-40
: Use realistic preview dataThe preview data contains placeholder values. Consider using real-world sample data to better represent the actual widget appearance.
- "Route A to Station 1 at 12:45 PM", - "Route B to Station 2 at 12:50 PM", + "Line A to Nemocnice Motol at 12:45", + "Line B to Černý Most at 12:50",apps/mobile/metro-now/metro-now/pages/settings/settings-page.view.swift (1)
37-41
: Consider adding widget configuration optionsThe navigation link is well-placed, but consider adding basic widget configuration options (e.g., refresh frequency, display preferences) in the widgets settings page.
apps/mobile/metro-now/widgets/departures/DeparturesWidgetTimelineProvider.swift (2)
20-29
: Consider enhancing error handling and user feedbackThe error case only shows "Unknown" without providing more context about why data might be unavailable (e.g., location permissions, network issues).
Consider enhancing the error feedback:
- let entry = DeparturesWidgetTimelineEntry(date: Date(), closestStop: "Unknown", departures: [], location: nil) + let entry = DeparturesWidgetTimelineEntry( + date: Date(), + closestStop: stopManager?.hasLocationPermission == true ? "No stops nearby" : "Location access needed", + departures: [], + location: nil + )
48-52
: Consider locale-aware time formattingThe time formatter should respect the user's locale settings.
private func formattedDepartureTime(_ departureDate: Date) -> String { let formatter = DateFormatter() + formatter.locale = Locale.current formatter.timeStyle = .short return formatter.string(from: departureDate) }
apps/mobile/metro-now/metro-now/pages/settings/subpages/settings-changelog.swift (1)
35-40
: Consider adding more descriptive changelog entryThe current entry "frequency widget" could be more descriptive about the feature's benefits.
ChangelogItem( version: "v0.3.6", changes: [ - "frequency widget", + "Added home screen widget for quick metro departure times", ] )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (22)
apps/mobile/metro-now/metro-now.xcodeproj/project.pbxproj
(22 hunks)apps/mobile/metro-now/metro-now/metro_nowApp.swift
(1 hunks)apps/mobile/metro-now/metro-now/pages/settings/settings-page.view.swift
(1 hunks)apps/mobile/metro-now/metro-now/pages/settings/subpages/settings-about.swift
(1 hunks)apps/mobile/metro-now/metro-now/pages/settings/subpages/settings-changelog.swift
(1 hunks)apps/mobile/metro-now/metro-now/pages/settings/subpages/settings-widgets.swift
(1 hunks)apps/mobile/metro-now/widgets/Assets.xcassets/AccentColor.colorset/Contents.json
(1 hunks)apps/mobile/metro-now/widgets/Assets.xcassets/AppIcon.appiconset/Contents.json
(1 hunks)apps/mobile/metro-now/widgets/Assets.xcassets/Contents.json
(1 hunks)apps/mobile/metro-now/widgets/Assets.xcassets/WidgetBackground.colorset/Contents.json
(1 hunks)apps/mobile/metro-now/widgets/Info.plist
(1 hunks)apps/mobile/metro-now/widgets/departures/DeparturesWidget.swift
(1 hunks)apps/mobile/metro-now/widgets/departures/DeparturesWidgetManager.swift
(1 hunks)apps/mobile/metro-now/widgets/departures/DeparturesWidgetTimelineEntry.swift
(1 hunks)apps/mobile/metro-now/widgets/departures/DeparturesWidgetTimelineProvider.swift
(1 hunks)apps/mobile/metro-now/widgets/departures/DeparturesWidgetView.swift
(1 hunks)apps/mobile/metro-now/widgets/frequency/FrequencyWidget.swift
(1 hunks)apps/mobile/metro-now/widgets/frequency/FrequencyWidgetManager.swift
(1 hunks)apps/mobile/metro-now/widgets/frequency/FrequencyWidgetTimelineEntry.swift
(1 hunks)apps/mobile/metro-now/widgets/frequency/FrequencyWidgetTimelineProvider.swift
(1 hunks)apps/mobile/metro-now/widgets/frequency/FrequencyWidgetView.swift
(1 hunks)apps/mobile/metro-now/widgets/widgetsBundle.swift
(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- apps/mobile/metro-now/widgets/Assets.xcassets/Contents.json
- apps/mobile/metro-now/widgets/Assets.xcassets/WidgetBackground.colorset/Contents.json
- apps/mobile/metro-now/widgets/Assets.xcassets/AppIcon.appiconset/Contents.json
- apps/mobile/metro-now/widgets/Assets.xcassets/AccentColor.colorset/Contents.json
🔇 Additional comments (22)
apps/mobile/metro-now/metro-now/metro_nowApp.swift (1)
12-14
: 🛠️ Refactor suggestion
Consider optimizing widget timeline reload strategy.
The current implementation reloads all widget timelines on every app launch, which might be inefficient. Consider:
- Adding error handling for the reload operation
- Implementing a more selective reload strategy
- Preventing multiple reloads if ContentView refreshes
Here's a suggested improvement:
.task {
+ do {
+ // Only reload if necessary (e.g., data has changed)
+ if await shouldReloadWidgets() {
WidgetCenter.shared.reloadAllTimelines()
+ }
+ } catch {
+ print("Failed to reload widget timelines: \(error)")
+ }
}
Let's check if there are any existing widget reload calls elsewhere in the codebase:
apps/mobile/metro-now/metro-now/pages/settings/subpages/settings-widgets.swift (1)
10-15
: LGTM! Well-designed warning label.
The warning is appropriately styled with orange color and an exclamation icon, making it visually distinct and attention-grabbing.
apps/mobile/metro-now/widgets/departures/DeparturesWidget.swift (1)
13-20
: Verify iOS version-specific layout behavior
The conditional layout handling looks good, but ensure testing on both iOS versions to verify the visual consistency.
✅ Verification successful
Let me check the implementation in FrequencyWidget.swift to ensure consistent handling.
iOS 17 version-specific layouts are consistently implemented
Both DeparturesWidget and FrequencyWidget follow the same pattern for handling iOS 17 compatibility, using the new .containerBackground
modifier for iOS 17+ and fallback styling for older versions. The implementation is consistent across widgets.
DeparturesWidget
: Uses.containerBackground(.fill.tertiary, for: .widget)
with padding and background fallbackFrequencyWidget
: Uses.containerBackground(.background, for: .widget)
with appropriate fallback
While the styling parameters differ slightly (tertiary vs background), this appears intentional for their respective UI needs.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other iOS version checks to ensure consistent handling
rg -g '*.swift' "if #available\(iOS 17.0"
Length of output: 251
Script:
#!/bin/bash
# Get the context of iOS 17 check in FrequencyWidget.swift
rg -g '*.swift' -B 2 -A 4 "if #available\(iOS 17.0" apps/mobile/metro-now/widgets/frequency/FrequencyWidget.swift
Length of output: 482
apps/mobile/metro-now/metro-now/pages/settings/subpages/settings-about.swift (1)
30-30
: LGTM! Text alignment improvement
The multiline text alignment change improves readability and maintains visual consistency.
apps/mobile/metro-now/metro-now.xcodeproj/project.pbxproj (18)
13-15
: Integration of WidgetKit and SwiftUI frameworks
The addition of WidgetKit.framework
and SwiftUI.framework
, along with widgetsExtension.appex
in the PBXBuildFile
section, is appropriate for supporting the new widget functionality.
34-40
: Correct setup of PBXContainerItemProxy for widgetsExtension
The PBXContainerItemProxy
for widgetsExtension
is correctly configured, ensuring proper linkage between targets.
55-65
: Proper configuration of Embed Foundation Extensions build phase
The Embed Foundation Extensions
build phase is correctly set up to embed widgetsExtension.appex
into the app bundle. The dstSubfolderSpec
value of 13
corresponds to the PlugIns
directory, which is appropriate for app extensions.
72-74
: Addition of file references for widgetsExtension and frameworks
The PBXFileReference
entries for widgetsExtension.appex
, WidgetKit.framework
, and SwiftUI.framework
are accurately added, ensuring these components are included in the project.
119-137
: Exceptions for widgetsExtension target
The exceptions defined for the widgets
and common
folders in the widgetsExtension
target correctly specify the files that should be included or excluded, ensuring proper organization and build behavior.
166-173
: Addition of widgets group to the project structure
The new widgets
group is appropriately integrated into the project, and exceptions are properly set for the widgetsExtension
target.
200-209
: Update of Frameworks build phase for widgetsExtension
The PBXFrameworksBuildPhase
for widgetsExtension
includes the necessary frameworks: Alamofire
, SwiftUI.framework
, and WidgetKit.framework
, ensuring all dependencies are met.
232-232
: Inclusion of widgetsExtension.appex in Products group
The widgetsExtension.appex
is correctly added to the Products
group, making it part of the build output.
237-245
: Creation of Frameworks group for new frameworks
The Frameworks
group is properly created and includes WidgetKit.framework
and SwiftUI.framework
, organizing the project assets effectively.
263-263
: Addition of PBXTargetDependency for widgetsExtension
The target dependency for widgetsExtension
is correctly added to the main app target, establishing the necessary build relationships.
323-345
: Configuration of widgetsExtension target
The widgetsExtension
target is well-configured with the appropriate build phases (Sources
, Frameworks
, Resources
), product dependencies, and file system groups. This setup ensures that the widget extension is built and integrated correctly.
366-368
: Setting of CreatedOnToolsVersion for widgetsExtension
The CreatedOnToolsVersion
is set to 16.1
for widgetsExtension
, which aligns with the tools used for other targets, ensuring compatibility.
391-391
: Inclusion of widgetsExtension in project targets
The widgetsExtension
is included in the project's targets list, ensuring it is part of the build process.
418-424
: Setup of Resources build phase for widgetsExtension
The Resources
build phase for widgetsExtension
is appropriately configured, even though it currently has no files. This allows for future resource additions without additional configuration.
449-455
: Configuration of Sources build phase for widgetsExtension
The Sources
build phase for widgetsExtension
is correctly set up, ready to include source files as needed.
469-473
: Proper target dependency configuration
The PBXTargetDependency
for widgetsExtension
ensures that dependencies are properly managed during the build process.
868-876
: Build configuration list for widgetsExtension
The build configuration list for widgetsExtension
is correctly set up with Debug
and Release
configurations, and the default configuration name is properly set to Release
.
901-905
: Adding Alamofire package dependency for widgetsExtension
The Alamofire
package product dependency is accurately added for the widgetsExtension
target, ensuring necessary networking capabilities.
request.validate().responseDecodable(of: [ApiStop].self) { response in | ||
switch response.result { | ||
case let .success(stops): | ||
DispatchQueue.main.async { | ||
self.metroStops = stops | ||
self.updateClosestMetroStop() | ||
} | ||
case let .failure(error): | ||
print("Failed to fetch metro stops: \(error)") | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevent potential retain cycles by capturing self
weakly in closure
When using closures that capture self
, it's best practice to capture self
weakly to prevent retain cycles. Consider modifying the closure to capture self
as weak and safely unwrap it within the closure.
Apply this diff to fix the issue:
- request.validate().responseDecodable(of: [ApiStop].self) { response in
+ request.validate().responseDecodable(of: [ApiStop].self) { [weak self] response in
Inside the closure, update references to self
to safely unwrap:
guard let self = self else { return }
request.validate().responseDecodable(of: [ApiDeparture].self, decoder: decoder) { response in | ||
switch response.result { | ||
case let .success(departures): | ||
DispatchQueue.main.async { | ||
self.nearestDepartures = departures | ||
} | ||
case let .failure(error): | ||
print("Failed to fetch departures: \(error)") | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevent potential retain cycles by capturing self
weakly in closure
To avoid potential retain cycles, capture self
weakly in the closure and safely unwrap it when needed.
Apply this diff to fix the issue:
- request.validate().responseDecodable(of: [ApiDeparture].self, decoder: decoder) { response in
+ request.validate().responseDecodable(of: [ApiDeparture].self, decoder: decoder) { [weak self] response in
Inside the closure, safely unwrap self
:
guard let self = self else { return }
request.validate().responseDecodable(of: [ApiStop].self) { response in | ||
switch response.result { | ||
case let .success(stops): | ||
DispatchQueue.main.async { | ||
self.metroStops = stops | ||
self.updateClosestMetroStop() | ||
} | ||
case let .failure(error): | ||
print("Failed to fetch metro stops: \(error)") | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevent potential retain cycles by capturing self
weakly in closure
Capture self
weakly in the closure to prevent retain cycles.
Apply this diff to fix the issue:
- request.validate().responseDecodable(of: [ApiStop].self) { response in
+ request.validate().responseDecodable(of: [ApiStop].self) { [weak self] response in
Remember to safely unwrap self
inside the closure:
guard let self = self else { return }
request.validate().responseDecodable(of: [ApiDeparture].self, decoder: decoder) { response in | ||
switch response.result { | ||
case let .success(departures): | ||
DispatchQueue.main.async { | ||
self.nearestDepartures = departures | ||
} | ||
case let .failure(error): | ||
print("Failed to fetch departures: \(error)") | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevent potential retain cycles by capturing self
weakly in closure
Avoid retain cycles by capturing self
weakly.
Apply this diff to fix the issue:
- request.validate().responseDecodable(of: [ApiDeparture].self, decoder: decoder) { response in
+ request.validate().responseDecodable(of: [ApiDeparture].self, decoder: decoder) { [weak self] response in
Safely unwrap self
inside the closure:
guard let self = self else { return }
apps/mobile/metro-now/widgets/departures/DeparturesWidgetTimelineProvider.swift
Show resolved
Hide resolved
2DD9D1882CF3B8A90037CB95 /* Debug */ = { | ||
isa = XCBuildConfiguration; | ||
buildSettings = { | ||
ASSETCATALOG_COMPILER_GLOBAL_ACCENT_COLOR_NAME = AccentColor; | ||
ASSETCATALOG_COMPILER_WIDGET_BACKGROUND_COLOR_NAME = WidgetBackground; | ||
CODE_SIGN_STYLE = Automatic; | ||
CURRENT_PROJECT_VERSION = 7; | ||
DEVELOPMENT_TEAM = R6WU5ABNG2; | ||
GENERATE_INFOPLIST_FILE = YES; | ||
INFOPLIST_FILE = widgets/Info.plist; | ||
INFOPLIST_KEY_CFBundleDisplayName = widgets; | ||
INFOPLIST_KEY_NSHumanReadableCopyright = ""; | ||
IPHONEOS_DEPLOYMENT_TARGET = 18.0; | ||
LD_RUNPATH_SEARCH_PATHS = ( | ||
"$(inherited)", | ||
"@executable_path/Frameworks", | ||
"@executable_path/../../Frameworks", | ||
); | ||
MARKETING_VERSION = 0.3.6; | ||
PRODUCT_BUNDLE_IDENTIFIER = "com.krystof.metro-now.widgets"; | ||
PRODUCT_NAME = "$(TARGET_NAME)"; | ||
SDKROOT = iphoneos; | ||
SKIP_INSTALL = YES; | ||
SWIFT_EMIT_LOC_STRINGS = YES; | ||
SWIFT_VERSION = 5.0; | ||
TARGETED_DEVICE_FAMILY = "1,2"; | ||
}; | ||
name = Debug; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent deployment target for widgetsExtension
The IPHONEOS_DEPLOYMENT_TARGET
for widgetsExtension
is set to 18.0
, while the main app targets iOS 17
. This discrepancy may lead to compatibility issues, as users on iOS 17 will not be able to use the widget extension.
It's recommended to align the IPHONEOS_DEPLOYMENT_TARGET
with the main app's deployment target to ensure consistent availability across all supported iOS versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2DD9D1892CF3B8A90037CB95 /* Release */ = { | ||
isa = XCBuildConfiguration; | ||
buildSettings = { | ||
ASSETCATALOG_COMPILER_GLOBAL_ACCENT_COLOR_NAME = AccentColor; | ||
ASSETCATALOG_COMPILER_WIDGET_BACKGROUND_COLOR_NAME = WidgetBackground; | ||
CODE_SIGN_STYLE = Automatic; | ||
CURRENT_PROJECT_VERSION = 7; | ||
DEVELOPMENT_TEAM = R6WU5ABNG2; | ||
GENERATE_INFOPLIST_FILE = YES; | ||
INFOPLIST_FILE = widgets/Info.plist; | ||
INFOPLIST_KEY_CFBundleDisplayName = widgets; | ||
INFOPLIST_KEY_NSHumanReadableCopyright = ""; | ||
IPHONEOS_DEPLOYMENT_TARGET = 18.0; | ||
LD_RUNPATH_SEARCH_PATHS = ( | ||
"$(inherited)", | ||
"@executable_path/Frameworks", | ||
"@executable_path/../../Frameworks", | ||
); | ||
MARKETING_VERSION = 0.3.6; | ||
PRODUCT_BUNDLE_IDENTIFIER = "com.krystof.metro-now.widgets"; | ||
PRODUCT_NAME = "$(TARGET_NAME)"; | ||
SDKROOT = iphoneos; | ||
SKIP_INSTALL = YES; | ||
SWIFT_EMIT_LOC_STRINGS = YES; | ||
SWIFT_VERSION = 5.0; | ||
TARGETED_DEVICE_FAMILY = "1,2"; | ||
VALIDATE_PRODUCT = YES; | ||
}; | ||
name = Release; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent deployment target for widgetsExtension Release configuration
Similarly, the Release configuration for widgetsExtension
also has IPHONEOS_DEPLOYMENT_TARGET
set to 18.0
, which may exclude iOS 17 users from accessing the widget functionality.
Consider updating the deployment target to match the main app's target version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
apps/web/src/app/page.tsx (1)
Line range hint
1-156
: Consider showcasing the new iOS widgets on the landing page.Since this PR introduces iOS widgets functionality, consider updating the landing page to:
- Add widget mockup images to showcase the feature
- Include widget descriptions in the marketing copy
- Update the iPhone mockup to show the widget in action
This would help promote the new widgets feature to users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
.prettierignore
(1 hunks)apps/backend/package.json
(0 hunks)apps/backend/src/app.module.ts
(1 hunks)apps/backend/src/constants/api.ts
(1 hunks)apps/backend/src/constants/swagger.const.ts
(1 hunks)apps/backend/src/main.ts
(1 hunks)apps/web/package.json
(0 hunks)apps/web/src/app/page.tsx
(1 hunks)apps/web/src/app/robots.ts
(1 hunks)apps/web/src/app/sitemap.ts
(1 hunks)apps/web/src/constants/api.ts
(1 hunks)apps/web/theme.config.tsx
(1 hunks)package.json
(0 hunks)packages/constants/.gitignore
(0 hunks)packages/constants/package.json
(0 hunks)packages/constants/src/external.ts
(0 hunks)packages/constants/src/index.ts
(0 hunks)packages/constants/tsconfig.cjs.json
(0 hunks)packages/constants/tsconfig.esm.json
(0 hunks)packages/constants/tsconfig.json
(0 hunks)pnpm-workspace.yaml
(0 hunks)
💤 Files with no reviewable changes (11)
- packages/constants/.gitignore
- packages/constants/tsconfig.cjs.json
- packages/constants/tsconfig.json
- packages/constants/package.json
- packages/constants/tsconfig.esm.json
- pnpm-workspace.yaml
- packages/constants/src/external.ts
- apps/web/package.json
- packages/constants/src/index.ts
- apps/backend/package.json
- package.json
✅ Files skipped from review due to trivial changes (4)
- .prettierignore
- apps/web/src/app/robots.ts
- apps/backend/src/constants/swagger.const.ts
- apps/web/src/constants/api.ts
🔇 Additional comments (7)
apps/web/src/app/sitemap.ts (1)
1-1
: Verify if this change belongs in the iOS widgets PR
This constants reorganization appears unrelated to the PR's main objective of adding iOS widgets. Consider moving this change to a separate PR focused on constants restructuring.
apps/web/theme.config.tsx (2)
1-1
: Where is the iOS widgets implementation?
The PR title and description mention adding iOS widgets, but the reviewed files only contain constant definitions and import path changes. Please ensure that:
- The actual widget implementation files are included in the PR
- The constants reorganization is moved to a separate PR for better clarity and review focus
1-1
: Consider moving constants reorganization to a separate PR
Like the sitemap.ts changes, this constants restructuring appears unrelated to the iOS widgets feature. Consider creating a separate PR for these changes to maintain clear feature boundaries.
✅ Verification successful
Let me verify the content of the api.ts file and check if there are any other files using the old import path to ensure this change is consistent.
Import path change is valid, but should be in a separate PR
The SOURCE_CODE_URL constant exists in the new location (apps/web/src/constants/api.ts) and no other files are importing it from the old @metro-now/constants path. While the refactoring is valid, it should be separated from the iOS widgets feature PR for better change management.
- Move the constants reorganization changes to a dedicated PR
- Keep only iOS widgets-related changes in this PR
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the relative import path exists and contains SOURCE_CODE_URL
fd -t f "api.ts" apps/web/src/ --exec grep -l "SOURCE_CODE_URL"
Length of output: 91
Script:
#!/bin/bash
# Check the content of api.ts to verify SOURCE_CODE_URL export
cat apps/web/src/constants/api.ts
# Check for any remaining imports from @metro-now/constants
rg "@metro-now/constants.*SOURCE_CODE_URL"
Length of output: 654
apps/backend/src/main.ts (1)
6-6
: LGTM! Verify constants relocation.
The import path change is consistent with the broader refactoring of moving constants to local paths.
Let's verify the constants are properly relocated:
apps/backend/src/app.module.ts (1)
10-10
: LGTM! Import path update is consistent.
The GRAPHQL_PATH constant import change aligns with the project-wide constants relocation.
apps/web/src/app/page.tsx (2)
11-11
: LGTM! Import path update is consistent.
The URL constants import change aligns with the project-wide constants relocation.
Line range hint 1-1
: Request: Please provide the iOS widget implementation code.
The current changes only show constants relocation. To properly review the iOS widgets feature, we need to see:
- The widget implementation code
- Widget configuration files
- Any new assets or resources used by the widgets
Let's check for widget-related files:
Changes