From 1ba6fb0a60e2b6ede354b013b482af5886f335fe Mon Sep 17 00:00:00 2001 From: Mauro <34335419+Velin92@users.noreply.github.com> Date: Tue, 24 Oct 2023 19:09:36 +0200 Subject: [PATCH] Suggestions view design improvements (#1955) * snapshot tests * making the padding of the composer internal * setting the max width using the line fragment * setting the max width using the line fragment * Update ElementX/Sources/Screens/ComposerToolbar/View/CompletionSuggestionView.swift Co-authored-by: Doug <6060466+pixlwave@users.noreply.github.com> * updated tests and pr suggestions * better stencil --------- Co-authored-by: Doug <6060466+pixlwave@users.noreply.github.com> --- .../xcshareddata/swiftpm/Package.resolved | 2 +- ElementX/Sources/Other/AvatarSize.swift | 3 ++ .../Sources/Other/Pills/PillConstants.swift | 3 +- .../Other/Pills/PillTextAttachment.swift | 1 + ElementX/Sources/Other/Pills/PillView.swift | 6 +++- .../View/CompletionSuggestionView.swift | 30 ++++++------------- .../View/ComposerToolbar.swift | 2 ++ .../View/MentionSuggestionItemView.swift | 6 ++-- .../Screens/RoomScreen/View/RoomScreen.swift | 2 -- Tools/Prefire/PreviewTests.stencil | 4 +-- .../test_completionSuggestion.1.png | 4 +-- .../test_completionSuggestion.2.png | 4 +-- .../PreviewTests/test_composerToolbar.1.png | 4 +-- .../test_composerToolbar.Voice-Message.png | 4 +-- .../test_composerToolbar.With-Suggestions.png | 4 +-- .../test_mentionSuggestionItemView.1.png | 4 +-- .../test_mentionSuggestionItemView.2.png | 4 +-- ...secureBackupLogoutConfirmationScreen.1.png | 3 ++ 18 files changed, 44 insertions(+), 46 deletions(-) create mode 100644 UnitTests/__Snapshots__/PreviewTests/test_secureBackupLogoutConfirmationScreen.1.png diff --git a/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index c0744d988..5c63b33f6 100644 --- a/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -262,7 +262,7 @@ { "identity" : "swiftui-introspect", "kind" : "remoteSourceControl", - "location" : "https://github.com/siteline/SwiftUI-Introspect.git", + "location" : "https://github.com/siteline/SwiftUI-Introspect", "state" : { "revision" : "b94da693e57eaf79d16464b8b7c90d09cba4e290", "version" : "0.9.2" diff --git a/ElementX/Sources/Other/AvatarSize.swift b/ElementX/Sources/Other/AvatarSize.swift index 100a05c4e..ebc70bd52 100644 --- a/ElementX/Sources/Other/AvatarSize.swift +++ b/ElementX/Sources/Other/AvatarSize.swift @@ -51,6 +51,7 @@ enum UserAvatarSizeOnScreen { case inviteUsers case readReceipt case editUserDetails + case suggestions var value: CGFloat { switch self { @@ -60,6 +61,8 @@ enum UserAvatarSizeOnScreen { return 32 case .home: return 32 + case .suggestions: + return 32 case .settings: return 52 case .roomDetails: diff --git a/ElementX/Sources/Other/Pills/PillConstants.swift b/ElementX/Sources/Other/Pills/PillConstants.swift index fb6e98e5a..4f1784c8d 100644 --- a/ElementX/Sources/Other/Pills/PillConstants.swift +++ b/ElementX/Sources/Other/Pills/PillConstants.swift @@ -25,5 +25,6 @@ enum PillConstants { /// Used by the WYSIWYG as the urlString value to identify @room mentions static let composerAtRoomURLString = "#" - static let maxWidth: CGFloat = 235 + /// Used only to mock the max width in previews since the real max width is calculated by the line fragment width + static let mockMaxWidth: CGFloat = 235 } diff --git a/ElementX/Sources/Other/Pills/PillTextAttachment.swift b/ElementX/Sources/Other/Pills/PillTextAttachment.swift index 666172439..6ef49d6cf 100644 --- a/ElementX/Sources/Other/Pills/PillTextAttachment.swift +++ b/ElementX/Sources/Other/Pills/PillTextAttachment.swift @@ -33,6 +33,7 @@ final class PillTextAttachment: NSTextAttachment { let fontData = pillData.fontData // Align the pill text vertically with the surrounding text. rect.origin.y = fontData.descender + (fontData.lineHeight - rect.height) / 2.0 + rect.size.width = min(rect.size.width, lineFrag.width) return rect } } diff --git a/ElementX/Sources/Other/Pills/PillView.swift b/ElementX/Sources/Other/Pills/PillView.swift index 7f6d211a3..d280d7219 100644 --- a/ElementX/Sources/Other/Pills/PillView.swift +++ b/ElementX/Sources/Other/Pills/PillView.swift @@ -42,7 +42,6 @@ struct PillView: View { .padding(.trailing, 6) .padding(.vertical, 1) .background { Capsule().foregroundColor(backgroundColor) } - .frame(maxWidth: PillConstants.maxWidth) .onChange(of: context.viewState.displayText) { _ in didChangeText() } @@ -55,18 +54,23 @@ struct PillView_Previews: PreviewProvider, TestablePreview { static var previews: some View { PillView(imageProvider: mockMediaProvider, context: PillContext.mock(type: .loadUser(isOwn: false))) { } + .frame(maxWidth: PillConstants.mockMaxWidth) .previewDisplayName("Loading") PillView(imageProvider: mockMediaProvider, context: PillContext.mock(type: .loadUser(isOwn: true))) { } + .frame(maxWidth: PillConstants.mockMaxWidth) .previewDisplayName("Loading Own") PillView(imageProvider: mockMediaProvider, context: PillContext.mock(type: .loadedUser(isOwn: false))) { } + .frame(maxWidth: PillConstants.mockMaxWidth) .previewDisplayName("Loaded Long") PillView(imageProvider: mockMediaProvider, context: PillContext.mock(type: .loadedUser(isOwn: true))) { } + .frame(maxWidth: PillConstants.mockMaxWidth) .previewDisplayName("Loaded Long Own") PillView(imageProvider: mockMediaProvider, context: PillContext.mock(type: .allUsers)) { } + .frame(maxWidth: PillConstants.mockMaxWidth) .previewDisplayName("All Users") } } diff --git a/ElementX/Sources/Screens/ComposerToolbar/View/CompletionSuggestionView.swift b/ElementX/Sources/Screens/ComposerToolbar/View/CompletionSuggestionView.swift index ca2394c8c..f072fbb9b 100644 --- a/ElementX/Sources/Screens/ComposerToolbar/View/CompletionSuggestionView.swift +++ b/ElementX/Sources/Screens/ComposerToolbar/View/CompletionSuggestionView.swift @@ -23,19 +23,11 @@ struct CompletionSuggestionView: View { private enum Constants { static let topPadding: CGFloat = 8.0 - static let listItemPadding: CGFloat = 4.0 - static let lineSpacing: CGFloat = 10.0 - static let maxHeight: CGFloat = 300.0 + static let listItemPadding: CGFloat = 6.0 + // added by the list itself when presenting the divider + static let listItemSpacing: CGFloat = 4.0 + static let leadingPadding: CGFloat = 16.0 static let maxVisibleRows = 4 - - /* - As of iOS 16.0, SwiftUI's List uses `UICollectionView` instead - of `UITableView` internally, this value is an adjustment to apply - to the list items in order to be as close as possible as the - `UITableView` display. - */ - @available(iOS 16.0, *) - static let collectionViewPaddingCorrection: CGFloat = -5.0 } // MARK: Public @@ -73,16 +65,16 @@ struct CompletionSuggestionView: View { } } .modifier(ListItemPaddingModifier(isFirst: items.first?.id == item.id)) + .listRowInsets(.init(top: 0, leading: Constants.leadingPadding, bottom: 0, trailing: 0)) } .listStyle(PlainListStyle()) - .frame(height: min(Constants.maxHeight, - min(contentHeightForRowCount(Constants.maxVisibleRows), - contentHeightForRowCount(items.count)))) + .frame(height: min(contentHeightForRowCount(Constants.maxVisibleRows), + contentHeightForRowCount(items.count))) .background(Color.compound.bgCanvasDefault) } private func contentHeightForRowCount(_ count: Int) -> CGFloat { - (prototypeListItemFrame.height + (Constants.listItemPadding * 2) + Constants.lineSpacing) * CGFloat(count) + Constants.topPadding + (prototypeListItemFrame.height + Constants.listItemPadding * 2 + Constants.listItemSpacing) * CGFloat(count) - Constants.listItemSpacing / 2 + Constants.topPadding - Constants.listItemPadding } private struct ListItemPaddingModifier: ViewModifier { @@ -93,12 +85,8 @@ struct CompletionSuggestionView: View { } func body(content: Content) -> some View { - var topPadding: CGFloat = isFirst ? Constants.listItemPadding + Constants.topPadding : Constants.listItemPadding + var topPadding: CGFloat = isFirst ? Constants.topPadding : Constants.listItemPadding var bottomPadding: CGFloat = Constants.listItemPadding - if #available(iOS 16.0, *) { - topPadding += Constants.collectionViewPaddingCorrection - bottomPadding += Constants.collectionViewPaddingCorrection - } return content .padding(.top, topPadding) diff --git a/ElementX/Sources/Screens/ComposerToolbar/View/ComposerToolbar.swift b/ElementX/Sources/Screens/ComposerToolbar/View/ComposerToolbar.swift index da5f7f513..0338e0783 100644 --- a/ElementX/Sources/Screens/ComposerToolbar/View/ComposerToolbar.swift +++ b/ElementX/Sources/Screens/ComposerToolbar/View/ComposerToolbar.swift @@ -46,6 +46,8 @@ struct ComposerToolbar: View { bottomBar } } + .padding(.leading, 5) + .padding(.trailing, 8) .background { ViewFrameReader(frame: $frame) } diff --git a/ElementX/Sources/Screens/ComposerToolbar/View/MentionSuggestionItemView.swift b/ElementX/Sources/Screens/ComposerToolbar/View/MentionSuggestionItemView.swift index 88981c56c..273222c92 100644 --- a/ElementX/Sources/Screens/ComposerToolbar/View/MentionSuggestionItemView.swift +++ b/ElementX/Sources/Screens/ComposerToolbar/View/MentionSuggestionItemView.swift @@ -21,13 +21,13 @@ struct MentionSuggestionItemView: View { let item: MentionSuggestionItem var body: some View { - HStack(alignment: .center) { + HStack(alignment: .center, spacing: 16) { LoadableAvatarImage(url: item.avatarURL, name: item.displayName, contentID: item.id, - avatarSize: .custom(42), + avatarSize: .user(on: .suggestions), imageProvider: imageProvider) - VStack(alignment: .leading) { + VStack(alignment: .leading, spacing: 0) { Text(item.displayName ?? item.id) .font(.compound.bodyLG) .foregroundColor(.compound.textPrimary) diff --git a/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift b/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift index c04235914..ce40ba00a 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift +++ b/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift @@ -35,8 +35,6 @@ struct RoomScreen: View { .background(Color.compound.bgCanvasDefault.ignoresSafeArea()) .safeAreaInset(edge: .bottom, spacing: 0) { composerToolbar - .padding(.leading, 5) - .padding(.trailing, 8) .padding(.bottom, composerToolbarContext.composerActionsEnabled ? 8 : 12) .background { if composerToolbarContext.composerActionsEnabled { diff --git a/Tools/Prefire/PreviewTests.stencil b/Tools/Prefire/PreviewTests.stencil index 0e2422d3e..d479701df 100644 --- a/Tools/Prefire/PreviewTests.stencil +++ b/Tools/Prefire/PreviewTests.stencil @@ -60,9 +60,7 @@ class PreviewTests: XCTestCase { file: file{% endif %}, testName: testName) - if let failure, - !failure.contains("No reference was found on disk."), - !failure.contains("to test against the newly-recorded snapshot") { + if let failure { XCTFail(failure) } diff --git a/UnitTests/__Snapshots__/PreviewTests/test_completionSuggestion.1.png b/UnitTests/__Snapshots__/PreviewTests/test_completionSuggestion.1.png index 56189798d..885e97fae 100644 --- a/UnitTests/__Snapshots__/PreviewTests/test_completionSuggestion.1.png +++ b/UnitTests/__Snapshots__/PreviewTests/test_completionSuggestion.1.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:1053b6b8361e483fdbfc3cab89610b5e6ba9642ee0c659a90880dc05779a0ee1 -size 87735 +oid sha256:371391f6600bef326ac712db125726be12ed5cb3733ac87e46f0a9bfbc303c38 +size 85314 diff --git a/UnitTests/__Snapshots__/PreviewTests/test_completionSuggestion.2.png b/UnitTests/__Snapshots__/PreviewTests/test_completionSuggestion.2.png index ad35d5626..56ba0ce4a 100644 --- a/UnitTests/__Snapshots__/PreviewTests/test_completionSuggestion.2.png +++ b/UnitTests/__Snapshots__/PreviewTests/test_completionSuggestion.2.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:7bcd00bc4f83ce36c67954aa301858ad9ea1e35eb04c735e596f6481e75070eb -size 77023 +oid sha256:a2e17fe71944d32e3c53a61946de933c39ea14c43e35b3ab7b1dc5bae99e13d6 +size 74940 diff --git a/UnitTests/__Snapshots__/PreviewTests/test_composerToolbar.1.png b/UnitTests/__Snapshots__/PreviewTests/test_composerToolbar.1.png index 6b081dd33..1066b108c 100644 --- a/UnitTests/__Snapshots__/PreviewTests/test_composerToolbar.1.png +++ b/UnitTests/__Snapshots__/PreviewTests/test_composerToolbar.1.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:1b9b4304a57abd20f6bf48530ac699e89d6e0ad16c90174b3a695a494df0e081 -size 66913 +oid sha256:fc90ea1180e2ccf681582cc1709ab45fd1e0deb499bfaaaf5bd8eafc87eab42d +size 65543 diff --git a/UnitTests/__Snapshots__/PreviewTests/test_composerToolbar.Voice-Message.png b/UnitTests/__Snapshots__/PreviewTests/test_composerToolbar.Voice-Message.png index 5730edb98..a871362dc 100644 --- a/UnitTests/__Snapshots__/PreviewTests/test_composerToolbar.Voice-Message.png +++ b/UnitTests/__Snapshots__/PreviewTests/test_composerToolbar.Voice-Message.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:4847895e8d01132c5fbef0cd2a5b3ac8c394ae197a15b37b7d9fff53cfa0bab9 -size 95668 +oid sha256:5d45505a9c6125393c99b7cd29846a1f712eb32ce8f8c044d7483fefb48b7db5 +size 95515 diff --git a/UnitTests/__Snapshots__/PreviewTests/test_composerToolbar.With-Suggestions.png b/UnitTests/__Snapshots__/PreviewTests/test_composerToolbar.With-Suggestions.png index 63b954161..a49ba456a 100644 --- a/UnitTests/__Snapshots__/PreviewTests/test_composerToolbar.With-Suggestions.png +++ b/UnitTests/__Snapshots__/PreviewTests/test_composerToolbar.With-Suggestions.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:c9b4ab3c5f407fd0cd1086e74b8b8097ce26c6a980e009b03ee2bbcf6009d088 -size 99560 +oid sha256:a85c5c383730bee4b56e4fdfaba5784f501bebee3168df7b455b937f7c42d3ca +size 96734 diff --git a/UnitTests/__Snapshots__/PreviewTests/test_mentionSuggestionItemView.1.png b/UnitTests/__Snapshots__/PreviewTests/test_mentionSuggestionItemView.1.png index 80dae5d06..a63d78089 100644 --- a/UnitTests/__Snapshots__/PreviewTests/test_mentionSuggestionItemView.1.png +++ b/UnitTests/__Snapshots__/PreviewTests/test_mentionSuggestionItemView.1.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:dc30346f730c3ceaf00311d6d1f50c7ebb97879d658424c8fc6624e8f78c80ee -size 63677 +oid sha256:1defe9e39eb1c83547c366965989250def123d785fd193dba29e9a57b82041a6 +size 63216 diff --git a/UnitTests/__Snapshots__/PreviewTests/test_mentionSuggestionItemView.2.png b/UnitTests/__Snapshots__/PreviewTests/test_mentionSuggestionItemView.2.png index 7289f9c8a..1b1b06da2 100644 --- a/UnitTests/__Snapshots__/PreviewTests/test_mentionSuggestionItemView.2.png +++ b/UnitTests/__Snapshots__/PreviewTests/test_mentionSuggestionItemView.2.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:792706480d9eb6b51fb407226f5ffa729ac54fc967c8f9925f2e30a6e3fe9ef2 -size 60057 +oid sha256:9a292ff4ebb5340397f839ec822ca603dcb8c443b5d7af6a31f12be361e401f4 +size 59683 diff --git a/UnitTests/__Snapshots__/PreviewTests/test_secureBackupLogoutConfirmationScreen.1.png b/UnitTests/__Snapshots__/PreviewTests/test_secureBackupLogoutConfirmationScreen.1.png new file mode 100644 index 000000000..ec029945c --- /dev/null +++ b/UnitTests/__Snapshots__/PreviewTests/test_secureBackupLogoutConfirmationScreen.1.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:f299bc135e6bba9f61d4dda13a9f88df211ee976b32dcba796cbf0b9d16b4d56 +size 110197