Change publication detail to use SwiftUI #687

stevenzeck · 2025-12-18T04:40:37Z

No description provided.

Copilot

Pull request overview

This pull request migrates the OPDS publication detail screen from UIKit to SwiftUI, modernizing the codebase and improving maintainability.

Changes:

Replaced UIKit-based OPDSPublicationInfoViewController with a native SwiftUI OPDSPublicationInfoView
Removed the OPDS storyboard file and associated factory methods
Implemented SwiftUI navigation with closure-based download handling and added a new error case for delegate availability

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
TestApp/Sources/OPDS/OPDSPublicationInfoViewController.swift	Removed UIKit view controller implementation
TestApp/Sources/OPDS/OPDSModule.swift	Updated to use SwiftUI navigation with closure-based download handler and added delegateNotAvailable error case
TestApp/Sources/OPDS/OPDSFeeds/OPDSPublicationInfoView.swift	Implemented new SwiftUI view with cover display, metadata, description, and download functionality
TestApp/Sources/OPDS/OPDSFactory.swift	Removed storyboard loading and view controller factory methods
TestApp/Sources/OPDS/Base.lproj/OPDS.storyboard	Removed storyboard file as it's no longer needed with SwiftUI

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-21T15:59:49Z

TestApp/Sources/OPDS/OPDSFeeds/OPDSPublicationInfoView.swift

+    private func download(link: ReadiumShared.Link) async {
+        isDownloading = true
+        do {
+            let book = try await download(link) { _ in }
+            alert = AlertMessage(
+                title: NSLocalizedString("success_title", comment: "Title of the alert when a publication is successfully downloaded"),
+                message: String(format: NSLocalizedString("library_download_success_message", comment: "Message of the alert when a publication is successfully downloaded"), book.title)
+            )
+        } catch {
+            alert = AlertMessage(
+                title: NSLocalizedString("error_title", comment: "Title of the alert when an error occurred"),
+                message: error.localizedDescription
+            )
+        }
+        isDownloading = false


The isDownloading state is not reset in a defer block, which could leave it stuck in the downloading state if an unexpected error occurs. While the current code does set it to false after the do-catch block, using defer would be more robust and ensure it's always reset regardless of how the function exits.

Copilot · 2026-01-21T15:59:49Z

TestApp/Sources/OPDS/OPDSFeeds/OPDSPublicationInfoView.swift

+}
+
+private extension Publication {
+    /// Finds the first link with `cover` or thumbnail relations.


The coverLink property documentation states it finds the first link with "cover or thumbnail relations", but the implementation also checks for OPDS-specific image relations. The documentation should be updated to accurately reflect all the relations being checked.

Suggested change

/// Finds the first link with `cover` or thumbnail relations.

/// Finds the first link with a `cover` relation or an OPDS image relation

/// (`http://opds-ps.org/image` or `http://opds-ps.org/image/thumbnail`).

Copilot · 2026-01-21T15:59:50Z

TestApp/Sources/OPDS/OPDSFeeds/OPDSPublicationInfoView.swift

+                            Text("_________")
+                        }
+
+                        Text(publication.metadata.authors.map(\.name).joined(separator: ", "))


Similar to the main view, when the authors array is empty, the joined separator will create an empty string. Consider adding a check to only display the authors text when the array is not empty.

Suggested change

Text(publication.metadata.authors.map(\.name).joined(separator: ", "))

if !publication.metadata.authors.isEmpty {

Text(publication.metadata.authors.map(\.name).joined(separator: ", "))

}

Copilot · 2026-01-21T15:59:50Z

TestApp/Sources/OPDS/OPDSModule.swift

 enum OPDSError: Error {
    case invalidURL(String)
+    case delegateNotAvailable


The new error case delegateNotAvailable should conform to LocalizedError protocol to provide a user-friendly error message. Currently, if this error is thrown and displayed to the user, it will show the raw case name instead of a helpful description.

Suggested change

enum OPDSError: Error {

case invalidURL(String)

case delegateNotAvailable

enum OPDSError: Error, LocalizedError {

case invalidURL(String)

case delegateNotAvailable

var errorDescription: String? {

switch self {

case let .invalidURL(urlString):

return NSLocalizedString(

"The catalog URL is invalid: \(urlString).",

comment: "Invalid OPDS catalog URL error"

)

case .delegateNotAvailable:

return NSLocalizedString(

"Unable to download the publication because the OPDS delegate is not available.",

comment: "OPDS delegate not available error"

)

}

}

Copilot · 2026-01-21T15:59:51Z

TestApp/Sources/OPDS/OPDSFeeds/OPDSPublicationInfoView.swift

+    }
+
+    struct AlertMessage: Identifiable {
+        var id: String { title + message }


The computed id property concatenates title and message, which could lead to collisions if two different alerts happen to have the same concatenated string. This could prevent an alert from showing if the previous alert had the same id. Consider using UUID() instead for guaranteed uniqueness.

Suggested change

var id: String { title + message }

let id = UUID()

Copilot · 2026-01-21T15:59:51Z

TestApp/Sources/OPDS/OPDSFeeds/OPDSPublicationInfoView.swift

+                                ProgressView()
+                                    .tint(.white)
+                            } else {
+                                Text("Download")


The button text "Download" is hardcoded and not localized using NSLocalizedString, unlike the alert messages below. This creates an inconsistency in the localization approach and will prevent the button text from being translated for different locales.

Suggested change

Text("Download")

Text(NSLocalizedString("Download", comment: "Download button title"))

Copilot · 2026-01-21T15:59:51Z

TestApp/Sources/OPDS/OPDSFeeds/OPDSPublicationInfoView.swift

+
+                if let description = publication.metadata.description {
+                    VStack(alignment: .leading, spacing: 10) {
+                        Text("About")


The section heading "About" is hardcoded and not localized using NSLocalizedString, unlike other user-facing strings in this file. This will prevent the heading from being translated for different locales.

Suggested change

Text("About")

Text(NSLocalizedString("About", comment: "Section heading for the publication description"))

Copilot · 2026-01-21T15:59:52Z

TestApp/Sources/OPDS/OPDSFeeds/OPDSPublicationInfoView.swift

    }

-    func updateUIViewController(_ uiViewController: OPDSPublicationInfoViewController, context: Context) {}
+    private func download(link: ReadiumShared.Link) async {


The name "download" is used for both the closure property (line 12) and the method (line 123). While this is technically valid Swift due to different signatures, it can be confusing and makes the code harder to understand. Consider renaming the method to something like "performDownload" or "downloadPublication" to clearly distinguish it from the closure property.

Suggested change

private func download(link: ReadiumShared.Link) async {

private func performDownload(link: ReadiumShared.Link) async {

Copilot · 2026-01-21T15:59:52Z

TestApp/Sources/OPDS/OPDSFeeds/OPDSPublicationInfoView.swift

+                            .fixedSize(horizontal: false, vertical: true)
+                            .multilineTextAlignment(.leading)
+
+                        Text(publication.metadata.authors.map(\.name).joined(separator: ", "))


When the authors array is empty, the joined separator will create an empty string, which will still display an empty Text view. Consider adding a check to only display the authors Text when the array is not empty, or provide a fallback text like "Unknown Author" for better UX.

Suggested change

Text(publication.metadata.authors.map(\.name).joined(separator: ", "))

let authorsText = publication.metadata.authors.map(\.name).joined(separator: ", ")

Text(authorsText.isEmpty ? "Unknown Author" : authorsText)

mickael-menu · 2026-01-21T16:26:30Z

I'm closing this PR as rewriting the Test App in SwiftUI is not on the table. We intend to remove the Test App and replace it with a more technical application for testing the Readium toolkit.

Change publication detail to use SwiftUI

c605468

stevenzeck temporarily deployed to LCP December 18, 2025 04:40 — with GitHub Actions Inactive

mickael-menu requested a review from Copilot January 21, 2026 15:53

Copilot started reviewing on behalf of mickael-menu January 21, 2026 15:54 View session

Copilot AI reviewed Jan 21, 2026

View reviewed changes

mickael-menu closed this Jan 21, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change publication detail to use SwiftUI #687

Change publication detail to use SwiftUI #687

Uh oh!

stevenzeck commented Dec 18, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 21, 2026

Uh oh!

Copilot AI Jan 21, 2026

Uh oh!

Copilot AI Jan 21, 2026

Uh oh!

Copilot AI Jan 21, 2026

Uh oh!

Copilot AI Jan 21, 2026

Uh oh!

Copilot AI Jan 21, 2026

Uh oh!

Copilot AI Jan 21, 2026

Uh oh!

Copilot AI Jan 21, 2026

Uh oh!

Copilot AI Jan 21, 2026

Uh oh!

mickael-menu commented Jan 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	/// Finds the first link with `cover` or thumbnail relations.
	/// Finds the first link with a `cover` relation or an OPDS image relation
	/// (`http://opds-ps.org/image` or `http://opds-ps.org/image/thumbnail`).

-enum OPDSError: Error {
-    case invalidURL(String)
-    case delegateNotAvailable
+enum OPDSError: Error, LocalizedError {
+    case invalidURL(String)
+    case delegateNotAvailable
+    var errorDescription: String? {
+        switch self {
+        case let .invalidURL(urlString):
+            return NSLocalizedString(
+                "The catalog URL is invalid: \(urlString).",
+                comment: "Invalid OPDS catalog URL error"
+            )
+        case .delegateNotAvailable:
+            return NSLocalizedString(
+                "Unable to download the publication because the OPDS delegate is not available.",
+                comment: "OPDS delegate not available error"
+            )
+        }
+    }

	Text("Download")
	Text(NSLocalizedString("Download", comment: "Download button title"))

	Text("About")
	Text(NSLocalizedString("About", comment: "Section heading for the publication description"))

	private func download(link: ReadiumShared.Link) async {
	private func performDownload(link: ReadiumShared.Link) async {

	Text(publication.metadata.authors.map(\.name).joined(separator: ", "))
	let authorsText = publication.metadata.authors.map(\.name).joined(separator: ", ")
	Text(authorsText.isEmpty ? "Unknown Author" : authorsText)

Change publication detail to use SwiftUI #687

Change publication detail to use SwiftUI #687

Uh oh!

Conversation

stevenzeck commented Dec 18, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

mickael-menu commented Jan 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants