Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR implements version 2.7.3 working changes focused on removing non-functional features, fixing iOS location services bugs, improving mesh map functionality, and optimizing performance.
- Removes the non-functional excluded module display override button from settings
- Fixes iOS 26 onboarding location services bug by updating authorization status handling
- Prevents ignored nodes from appearing on the mesh map
- Improves performance of online node animations by extracting reusable animated components
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Meshtastic/Views/Settings/Settings.swift | Removes moduleOverride state and related UI elements |
| Meshtastic/Views/Settings/Config/LoRaConfig.swift | Removes fixedSize modifier from region picker |
| Meshtastic/Views/Onboarding/DeviceOnboarding.swift | Fixes location authorization status handling for iOS 26 |
| Meshtastic/Views/Nodes/Helpers/Map/PositionPopover.swift | Simplifies animation by removing custom pulsing logic |
| Meshtastic/Views/Nodes/Helpers/Map/MapContent/MeshMapContent.swift | Filters ignored nodes and refactors to use new AnimatedNodePin component |
| Meshtastic/Views/Nodes/Helpers/Map/MapContent/AnimatedNodePin.swift | New reusable component for animated node pins with performance optimization |
| Meshtastic/Helpers/LocationsHandler.swift | Adjusts timeout and return value for location permission requests |
| Meshtastic.xcodeproj/project.pbxproj | Updates marketing version to 2.7.3 and adds AnimatedNodePin.swift to build |
| Localizable.xcstrings | Removes unused localization strings related to module override feature |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
* Set hopLimit for DM messages (DM's and Exchange position) to the hops away value for the node you are sending to. * Update Meshtastic/Accessory/Accessory Manager/AccessoryManager+ToRadio.swift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Jake-B <jake-b@users.noreply.github.com>
* DataDog action logging * Filter version hash --------- Co-authored-by: Jake-B <jake-b@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 29 out of 42 changed files in this pull request and generated 5 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| ForEach(positions, id: \.id) { position in | ||
| if !showFavorites || (position.nodePosition?.favorite == true) { | ||
| /// Node color from node.num | ||
| if (!showFavorites || (position.nodePosition?.favorite == true)) && !(position.nodePosition?.ignored == true) { |
There was a problem hiding this comment.
[nitpick] The condition is complex and hard to read. Consider extracting it into a computed property or helper method like shouldShowPosition(position:) for better readability.
Comment on lines
+379
to
380
| if locationStatus != .notDetermined && locationStatus != .restricted { | ||
| navigationPath.append(.localNetwork) |
There was a problem hiding this comment.
The condition excludes .denied status but doesn't handle it appropriately. Users with denied location permissions will proceed to the next onboarding step, which may cause issues if location functionality is expected later.
Suggested change
| if locationStatus != .notDetermined && locationStatus != .restricted { | |
| navigationPath.append(.localNetwork) | |
| // Only proceed if location is authorized (not denied, not restricted, not notDetermined) | |
| if locationStatus == .authorizedAlways || locationStatus == .authorizedWhenInUse { | |
| navigationPath.append(.localNetwork) | |
| } else if locationStatus == .denied { | |
| // Optionally, show an alert or message to the user to enable location in settings | |
| // For now, do not proceed to the next step |
…wift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…swift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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
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.
Uh oh!
There was an error while loading. Please reload this page.