From 823c2565dac3aa61988d0e18b4ebfb669766265e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20D=E2=80=99Aquino?= Date: Fri, 20 Sep 2024 13:10:54 -0700 Subject: [PATCH 1/3] Fix unclickable elements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The introduction of iOS 18 brought a new bug that made `KFAnimatedImage` not recognize tap gestures and become unclickable. (https://github.com/onevcat/Kingfisher/issues/2295) This commit addresses the issue with a workaround found here: https://github.com/onevcat/Kingfisher/issues/2046#issuecomment-1554068070 The workaround was suggested by the author of the library to fix a slightly different issue, but that property seems to work for our purposes. The issue is addressed by adding a `contentShape` property to usages of `KFAnimatedImage`, in order to make them clickable. A custom modifier was created to make the solution less obscure and more obvious. Furthermore, one empty tap gesture handler was removed as it was preventing other tap gesture handlers on the image carousel from being triggered on iOS 18 Testing ------- PASS Configurations: - iPhone 13 mini on iOS 18.0 - iPhone SE simulator on iOS 17.5 Damus: This commit Coverage: - Check that the following views are clickable: - Images in the carousel - Profile picture on notes - Profile picture on thread comments - Profile picture on profile page Changelog-Fixed: Fix items that became unclickable on iOS 18 Closes: https://github.com/damus-io/damus/issues/2342 Closes: https://github.com/damus-io/damus/issues/2370 Signed-off-by: Daniel D’Aquino --- damus.xcodeproj/project.pbxproj | 14 +++++++++++ damus/Components/ImageCarousel.swift | 11 +++++++-- damus/Views/BannerImageView.swift | 2 ++ .../Events/Highlight/HighlightEventRef.swift | 1 + .../Events/Highlight/HighlightLink.swift | 1 + .../Events/Longform/LongformPreview.swift | 1 + damus/Views/Images/ImageContainerView.swift | 1 + damus/Views/Images/ProfilePicImageView.swift | 1 + damus/Views/Profile/EditPictureControl.swift | 1 + damus/Views/Profile/ProfilePicView.swift | 1 + .../Profile/ProfilePictureSelector.swift | 1 + damus/Views/Relays/RelayPicView.swift | 1 + damus/Views/Utils/KFClickable.swift | 23 +++++++++++++++++++ 13 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 damus/Views/Utils/KFClickable.swift diff --git a/damus.xcodeproj/project.pbxproj b/damus.xcodeproj/project.pbxproj index fcaef816..747be1f8 100644 --- a/damus.xcodeproj/project.pbxproj +++ b/damus.xcodeproj/project.pbxproj @@ -1111,6 +1111,8 @@ D7CE1B482B0BE719002EDAD4 /* Message.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4C32B93D2A9AD44700DC3548 /* Message.swift */; }; D7CE1B492B0BE729002EDAD4 /* DisplayName.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4C9BB83029C0ED4F00FC4E37 /* DisplayName.swift */; }; D7D2A3812BF815D000E4B42B /* PushNotificationClient.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7D2A3802BF815D000E4B42B /* PushNotificationClient.swift */; }; + D7D68FF92C9E01BE0015A515 /* KFClickable.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7D68FF82C9E01B60015A515 /* KFClickable.swift */; }; + D7D68FFA2C9E01BE0015A515 /* KFClickable.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7D68FF82C9E01B60015A515 /* KFClickable.swift */; }; D7DBD41F2B02F15E002A6197 /* NostrKind.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4C3BEFD32819DE8F00B3DE84 /* NostrKind.swift */; }; D7DEEF2F2A8C021E00E0C99F /* NostrEventTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7DEEF2E2A8C021E00E0C99F /* NostrEventTests.swift */; }; D7EDED152B11776B0018B19C /* LibreTranslateServer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3AE45AF5297BB2E700C1D842 /* LibreTranslateServer.swift */; }; @@ -1959,6 +1961,7 @@ D7CBD1D32B8D21DC00BFD889 /* DamusPurpleNotificationManagement.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DamusPurpleNotificationManagement.swift; sourceTree = ""; }; D7CBD1D52B8D509800BFD889 /* DamusPurpleImpendingExpirationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DamusPurpleImpendingExpirationTests.swift; sourceTree = ""; }; D7D2A3802BF815D000E4B42B /* PushNotificationClient.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PushNotificationClient.swift; sourceTree = ""; }; + D7D68FF82C9E01B60015A515 /* KFClickable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = KFClickable.swift; sourceTree = ""; }; D7DEEF2E2A8C021E00E0C99F /* NostrEventTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NostrEventTests.swift; sourceTree = ""; }; D7EDED1B2B1178FE0018B19C /* NoteContent.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NoteContent.swift; sourceTree = ""; }; D7EDED1D2B11797D0018B19C /* LongformEvent.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LongformEvent.swift; sourceTree = ""; }; @@ -2540,6 +2543,7 @@ 4C75EFA227FA576C0006080F /* Views */ = { isa = PBXGroup; children = ( + D7D68FF72C9E01A80015A515 /* Utils */, D78DB85D2C20FE9E00F0AB12 /* Chat */, D71AC4CA2BA8E3320076268E /* Extensions */, BA3759952ABCCF360018D73B /* Camera */, @@ -3368,6 +3372,14 @@ path = Extensions; sourceTree = ""; }; + D7D68FF72C9E01A80015A515 /* Utils */ = { + isa = PBXGroup; + children = ( + D7D68FF82C9E01B60015A515 /* KFClickable.swift */, + ); + path = Utils; + sourceTree = ""; + }; E06336A72B7582D600A88E6B /* Assets */ = { isa = PBXGroup; children = ( @@ -3803,6 +3815,7 @@ B51C1CEA2B55A60A00E312A9 /* AddMuteItemView.swift in Sources */, 4C5D5C992A6AF8F80024563C /* NdbNote.swift in Sources */, 4CF0ABF029857E9200D66079 /* Bech32Object.swift in Sources */, + D7D68FFA2C9E01BE0015A515 /* KFClickable.swift in Sources */, 4C3D52B8298DB5C6001C5831 /* TextEvent.swift in Sources */, 4C216F362870A9A700040376 /* InputDismissKeyboard.swift in Sources */, D74AAFCF2B155D8C006CF0F4 /* ZapDataModel.swift in Sources */, @@ -4517,6 +4530,7 @@ D73E5F332C6A97F4007EB227 /* ZapEvent.swift in Sources */, D73E5F342C6A97F4007EB227 /* TextEvent.swift in Sources */, D73E5F352C6A97F4007EB227 /* WideEventView.swift in Sources */, + D7D68FF92C9E01BE0015A515 /* KFClickable.swift in Sources */, D73E5F8A2C6AA69C007EB227 /* SideMenuView.swift in Sources */, D73E5F362C6A97F4007EB227 /* LongformView.swift in Sources */, D73E5F372C6A97F4007EB227 /* LongformPreview.swift in Sources */, diff --git a/damus/Components/ImageCarousel.swift b/damus/Components/ImageCarousel.swift index 95480f50..4c1f9e66 100644 --- a/damus/Components/ImageCarousel.swift +++ b/damus/Components/ImageCarousel.swift @@ -236,6 +236,7 @@ struct ImageCarousel: View { Placeholder(url: url, geo_size: geo.size, num_urls: urls.count) } .aspectRatio(contentMode: filling ? .fill : .fit) + .kfClickable() .position(x: geo.size.width / 2, y: geo.size.height / 2) .tabItem { Text(url.absoluteString) @@ -274,8 +275,14 @@ struct ImageCarousel: View { var body: some View { VStack { - Medias - .onTapGesture { } + if #available(iOS 18.0, *) { + Medias + } else { + // An empty tap gesture recognizer is needed on iOS 17 and below to suppress other overlapping tap recognizers + // Otherwise it will both open the carousel and go to a note at the same time + Medias.onTapGesture { } + } + if urls.count > 1 { PageControlView(currentPage: $model.selectedIndex, numberOfPages: urls.count) diff --git a/damus/Views/BannerImageView.swift b/damus/Views/BannerImageView.swift index b1afd01a..3bb4e871 100644 --- a/damus/Views/BannerImageView.swift +++ b/damus/Views/BannerImageView.swift @@ -29,6 +29,7 @@ struct EditBannerImageView: View { Color(uiColor: .secondarySystemBackground) } .onFailureImage(defaultImage) + .kfClickable() EditPictureControl(uploader: damus_state.settings.default_media_uploader, pubkey: damus_state.pubkey, image_url: $banner_image, uploadObserver: viewModel, callback: callback) } @@ -54,6 +55,7 @@ struct InnerBannerImageView: View { Color(uiColor: .secondarySystemBackground) } .onFailureImage(defaultImage) + .kfClickable() } else { Image(uiImage: defaultImage).resizable() } diff --git a/damus/Views/Events/Highlight/HighlightEventRef.swift b/damus/Views/Events/Highlight/HighlightEventRef.swift index aa0c9203..059232b5 100644 --- a/damus/Views/Events/Highlight/HighlightEventRef.swift +++ b/damus/Views/Events/Highlight/HighlightEventRef.swift @@ -50,6 +50,7 @@ struct HighlightEventRef: View { FailedImage() } .frame(width: 35, height: 35) + .kfClickable() .clipShape(RoundedRectangle(cornerRadius: 10)) .overlay(RoundedRectangle(cornerRadius: 10).stroke(.gray.opacity(0.5), lineWidth: 0.5)) .scaledToFit() diff --git a/damus/Views/Events/Highlight/HighlightLink.swift b/damus/Views/Events/Highlight/HighlightLink.swift index 3e75e2c4..a3f689ba 100644 --- a/damus/Views/Events/Highlight/HighlightLink.swift +++ b/damus/Views/Events/Highlight/HighlightLink.swift @@ -61,6 +61,7 @@ struct HighlightLink: View { .background(DamusColors.adaptableWhite) } .frame(width: 35, height: 35) + .kfClickable() .clipShape(RoundedRectangle(cornerRadius: 10)) .scaledToFit() } else { diff --git a/damus/Views/Events/Longform/LongformPreview.swift b/damus/Views/Events/Longform/LongformPreview.swift index 26235804..cff2f532 100644 --- a/damus/Views/Events/Longform/LongformPreview.swift +++ b/damus/Views/Events/Longform/LongformPreview.swift @@ -98,6 +98,7 @@ struct LongformPreviewBody: View { } .aspectRatio(contentMode: .fill) .frame(maxWidth: .infinity, maxHeight: header ? .infinity : 150) + .kfClickable() .cornerRadius(1) } diff --git a/damus/Views/Images/ImageContainerView.swift b/damus/Views/Images/ImageContainerView.swift index 153a61a0..ac8c5b46 100644 --- a/damus/Views/Images/ImageContainerView.swift +++ b/damus/Views/Images/ImageContainerView.swift @@ -33,6 +33,7 @@ struct ImageContainerView: View { view.framePreloadCount = 3 } .imageModifier(ImageHandler(handler: $image)) + .kfClickable() .clipped() .modifier(ImageContextMenuModifier(url: url, image: image, settings: settings, showShareSheet: $showShareSheet)) .sheet(isPresented: $showShareSheet) { diff --git a/damus/Views/Images/ProfilePicImageView.swift b/damus/Views/Images/ProfilePicImageView.swift index d7b3f56e..987cc7c6 100644 --- a/damus/Views/Images/ProfilePicImageView.swift +++ b/damus/Views/Images/ProfilePicImageView.swift @@ -33,6 +33,7 @@ struct ProfileImageContainerView: View { .imageModifier(ImageHandler(handler: $image)) .clipShape(Circle()) .modifier(ImageContextMenuModifier(url: url, image: image, settings: settings, showShareSheet: $showShareSheet)) + .kfClickable() .sheet(isPresented: $showShareSheet) { ShareSheet(activityItems: [url]) } diff --git a/damus/Views/Profile/EditPictureControl.swift b/damus/Views/Profile/EditPictureControl.swift index 454ced3b..85d48290 100644 --- a/damus/Views/Profile/EditPictureControl.swift +++ b/damus/Views/Profile/EditPictureControl.swift @@ -71,6 +71,7 @@ struct EditPictureControl: View { } .scaledToFill() .frame(width: (size ?? 25) + 10, height: (size ?? 25) + 10) + .kfClickable() .foregroundColor(DamusColors.white) .clipShape(Circle()) .overlay(Circle().stroke(.white, lineWidth: 4)) diff --git a/damus/Views/Profile/ProfilePicView.swift b/damus/Views/Profile/ProfilePicView.swift index 4681efed..733d492c 100644 --- a/damus/Views/Profile/ProfilePicView.swift +++ b/damus/Views/Profile/ProfilePicView.swift @@ -57,6 +57,7 @@ struct InnerProfilePicView: View { } .scaledToFill() .frame(width: size, height: size) + .kfClickable() .clipShape(Circle()) .overlay(Circle().stroke(highlight_color(highlight), lineWidth: pfp_line_width(highlight))) } diff --git a/damus/Views/Profile/ProfilePictureSelector.swift b/damus/Views/Profile/ProfilePictureSelector.swift index 5559746c..5cae27b7 100644 --- a/damus/Views/Profile/ProfilePictureSelector.swift +++ b/damus/Views/Profile/ProfilePictureSelector.swift @@ -31,6 +31,7 @@ struct EditProfilePictureView: View { view.framePreloadCount = 3 } .scaledToFill() + .kfClickable() EditPictureControl(uploader: damus_state?.settings.default_media_uploader ?? .nostrBuild, pubkey: pubkey, image_url: $profile_url, uploadObserver: uploadObserver, callback: callback) } diff --git a/damus/Views/Relays/RelayPicView.swift b/damus/Views/Relays/RelayPicView.swift index 277f5740..bea5cc08 100644 --- a/damus/Views/Relays/RelayPicView.swift +++ b/damus/Views/Relays/RelayPicView.swift @@ -55,6 +55,7 @@ struct InnerRelayPicView: View { Placeholder(url: url) } .scaledToFit() + .kfClickable() } else { FailedRelayImage(url: nil) } diff --git a/damus/Views/Utils/KFClickable.swift b/damus/Views/Utils/KFClickable.swift new file mode 100644 index 00000000..6b5ac789 --- /dev/null +++ b/damus/Views/Utils/KFClickable.swift @@ -0,0 +1,23 @@ +// +// ClickableOverlay.swift +// damus +// +// Created by Daniel D’Aquino on 2024-09-20. +// + +import SwiftUI + +/// Applies a property that makes `KFAnimatedImage` clickable again on iOS 18+ +fileprivate struct KFClickable: ViewModifier { + func body(content: Content) -> some View { + content + .contentShape(Rectangle()) + } +} + +extension View { + /// Applies a property that makes `KFAnimatedImage` clickable again on iOS 18+ + func kfClickable() -> some View { + return self.modifier(KFClickable()) + } +} From 2a61440aedb5d7fecf600b6fb587ecaf74bf1829 Mon Sep 17 00:00:00 2001 From: William Casarin Date: Thu, 19 Sep 2024 12:58:37 -0700 Subject: [PATCH 2/3] relays: add some ping/pong and connection logs need this for debugging connection issues Signed-off-by: William Casarin --- damus/Nostr/RelayConnection.swift | 7 ++++--- damus/Nostr/RelayPool.swift | 1 + damus/Util/Log.swift | 1 + 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/damus/Nostr/RelayConnection.swift b/damus/Nostr/RelayConnection.swift index c3e5f625..f8a2e3e2 100644 --- a/damus/Nostr/RelayConnection.swift +++ b/damus/Nostr/RelayConnection.swift @@ -46,9 +46,10 @@ final class RelayConnection: ObservableObject { if err == nil { self.last_pong = .now + Log.info("Got pong from '%s'", for: .networking, self.relay_url.absoluteString) self.log?.add("Successful ping") } else { - print("pong failed, reconnecting \(self.relay_url.id)") + Log.info("Ping failed, reconnecting to '%s'", for: .networking, self.relay_url.absoluteString) self.isConnected = false self.isConnecting = false self.reconnect_with_backoff() @@ -126,7 +127,7 @@ final class RelayConnection: ObservableObject { self.receive(message: message) case .disconnected(let closeCode, let reason): if closeCode != .normalClosure { - print("⚠️ Warning: RelayConnection (\(self.relay_url)) closed with code \(closeCode), reason: \(String(describing: reason))") + Log.error("⚠️ Warning: RelayConnection (%d) closed with code: %s", for: .networking, String(describing: closeCode), String(describing: reason)) } DispatchQueue.main.async { self.isConnected = false @@ -134,7 +135,7 @@ final class RelayConnection: ObservableObject { self.reconnect() } case .error(let error): - print("⚠️ Warning: RelayConnection (\(self.relay_url)) error: \(error)") + Log.error("⚠️ Warning: RelayConnection (%s) error: %s", for: .networking, self.relay_url.absoluteString, error.localizedDescription) let nserr = error as NSError if nserr.domain == NSPOSIXErrorDomain && nserr.code == 57 { // ignore socket not connected? diff --git a/damus/Nostr/RelayPool.swift b/damus/Nostr/RelayPool.swift index 903b3f76..e63dad06 100644 --- a/damus/Nostr/RelayPool.swift +++ b/damus/Nostr/RelayPool.swift @@ -89,6 +89,7 @@ class RelayPool { } func ping() { + Log.info("Pinging %d relays", for: .networking, relays.count) for relay in relays { relay.connection.ping() } diff --git a/damus/Util/Log.swift b/damus/Util/Log.swift index d8a3208e..830f9e65 100644 --- a/damus/Util/Log.swift +++ b/damus/Util/Log.swift @@ -13,6 +13,7 @@ enum LogCategory: String { case nav case render case storage + case networking case push_notifications case damus_purple case image_uploading From 6639c002ed8a2209cdd31c671ba6f4321d7a9bd6 Mon Sep 17 00:00:00 2001 From: William Casarin Date: Thu, 19 Sep 2024 12:41:31 -0700 Subject: [PATCH 3/3] relay: don't reconnect when we don't have to We are reconnecting multiple times for two separate reasons: 1. On a cancellation "error" which does not warrant a reconnect 2. In our reconnection backoff it doesn't check If we are already connecting or connected. We check this so we don't reconnect multiple times. This fixes many reconnection issues and makes Damus feel wayyy snappier. Signed-off-by: William Casarin --- damus/Nostr/RelayConnection.swift | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/damus/Nostr/RelayConnection.swift b/damus/Nostr/RelayConnection.swift index f8a2e3e2..1df94875 100644 --- a/damus/Nostr/RelayConnection.swift +++ b/damus/Nostr/RelayConnection.swift @@ -141,6 +141,10 @@ final class RelayConnection: ObservableObject { // ignore socket not connected? return } + if nserr.domain == NSURLErrorDomain && nserr.code == -999 { + // these aren't real error, it just means task was cancelled + return + } DispatchQueue.main.async { self.isConnected = false self.isConnecting = false @@ -157,14 +161,21 @@ final class RelayConnection: ObservableObject { } func reconnect_with_backoff() { - self.backoff *= 1.5 + self.backoff *= 2.0 self.reconnect_in(after: self.backoff) } func reconnect() { guard !isConnecting && !isDisabled else { + self.log?.add("Cancelling reconnect, already connecting") return // we're already trying to connect or we're disabled } + + guard !self.isConnected else { + self.log?.add("Cancelling reconnect, already connected") + return + } + disconnect() connect() log?.add("Reconnecting...")