Refine iOS layout and torrent detail flows#100
Conversation
Introduce rename/move/label UI for torrent detail: add @State properties for dialog visibility and inputs, replace TorrentDetailToolbar with a new detailToolbar that presents an IOSTorrentActionsMenu, and wire .sheet modifiers to rename/move/label sheet builders. Implement sheet view factories (renameSheet, moveSheet, labelSheet), helper show* functions to prefill inputs, and a presentError helper. Also change visibility of IOSTorrentActionsMenu, IOSTorrentRenameSheet, and IOSTorrentMoveSheet from private to internal so they can be reused from the detail view.
Introduce a shared TorrentPiecesSectionState and move hasRenderablePieceData into TorrentDetailSupplementalPayload. Update iOS and macOS detail views to use the shared state, remove the macOS-specific enum, and wire a retry action that re-triggers supplementalStore.load on iOS. Add composed iOS views for pieces (loading/content/empty/failed) and unit tests covering resolve() behavior for the new state.
Encapsulate supplementalStore.load into a @mainactor helper (loadSupplementalDetails) and use .onChange(of: torrent.id, initial: true) to trigger loads (including retry) instead of .task. Pass torrentID into the pieces section and add a PiecesGridIdentity (torrentID, pieceCount, piecesHaveCount) as the PiecesGridView id so the grid updates correctly when torrent or piece data change.
Replace the LazyVGrid + GeometryReader implementation in PiecesGridView with a Canvas-based renderer that computes columns from the canvas size and paints rounded rectangles directly (using a computed unit for spacing). This simplifies layout, avoids per-cell SwiftUI views, and keeps the same fixed height. Also remove the .id usage and the PiecesGridIdentity type from the iOS torrent detail view: the torrentID prop and identity struct were removed from IOSTorrentPiecesSectionContent since the explicit identity is no longer needed.
Replace the modal connection error alert on iOS with an inline banner. Added iOSConnectionBannerView to Shared views (status icon, title, periodic retry countdown, last error text and a Retry button). iOSContentView now conditionally shows the banner when a host is configured and the connection is not connected, with a top transition and animation. TransmissionStore no longer toggles showConnectionErrorAlert for iOS (set to false), and the previous alert presentation in iOSContentView was removed. The Retry button is disabled when reconnect attempts are not allowed.
Replace the separate filter and sort menus with a single preferences popover accessed from the bottom-bar slider button. Adds @State showPrefs and a new prefsPopoverContent view (NavigationStack + List) that consolidates filter buttons, a sort property section and a segmented Picker for sort order, plus a Done toolbar button and presentation styling. Removes the old filterMenu and sortMenu implementations and updates checkmark styling for the selected sort property.
Replace the separate serverSelectionMenu with an inline "Servers" section inside the action toolbar menu. Add a ForEach that lists hosts as buttons (showing a checkmark for the current host) and call store.setHost on selection. Also update menu labels to "Add Server" and "Edit Servers" and remove the old Picker-based serverSelectionMenu view. This makes server selection directly accessible from the toolbar.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d78fd2593a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Introduce a managed loading mechanism in TorrentDetailSupplementalStore: add managedLoadTask and managedLoadGeneration, cancel the task on deinit, and add replaceLoad(for:using:onError:) which cancels previous tasks, increments a generation token, and only clears the task if the generation matches. Add a convenience replaceLoad overload that accepts Binding<Bool>/Binding<String> error handlers. Update iOS and macOS torrent detail views to call replaceLoad instead of spawning Tasks or calling load directly (including adjusting the onChange handler to forward the new torrent ID and replacing onRetry closures). This prevents overlapping loads and ensures only the latest requested load completes and cleans up its task.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59840a22a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Replace the direct async load with performStructuredTransmissionOperation and track a requestGeneration via mutateState(beginLoading:). On error, mark failure only if the generation matches and call onError; on success, apply the returned TransmissionTorrentDetailSnapshot with a new applyManagedSnapshot helper. Add clearManagedLoadTask(ifMatching:) to only clear the managedLoadTask when the generation matches, and handle cancellation via markCancellation. Overall this centralizes load/error/cancellation handling and keeps generation checks consistent.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary