From c7bfe7a0a19d3a1f2510f85869bbc2d436ae7bf7 Mon Sep 17 00:00:00 2001 From: Valentin Perignon Date: Tue, 30 May 2023 14:38:34 +0200 Subject: [PATCH 1/6] feat: Remove minimumDuration --- Mail/Views/Thread List/ThreadListCell.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Mail/Views/Thread List/ThreadListCell.swift b/Mail/Views/Thread List/ThreadListCell.swift index 97201fd5b..593cd565f 100644 --- a/Mail/Views/Thread List/ThreadListCell.swift +++ b/Mail/Views/Thread List/ThreadListCell.swift @@ -54,7 +54,7 @@ struct ThreadListCell: View { ) .background(SelectionBackground(selectionType: selectionType, paddingLeading: 4)) .onTapGesture { didTapCell() } - .onLongPressGesture(minimumDuration: 0.3) { didLongPressCell() } + .onLongPressGesture { didLongPressCell() } .swipeActions(thread: thread, viewModel: viewModel, multipleSelectionViewModel: multipleSelectionViewModel) .listRowInsets(.init(top: 0, leading: 0, bottom: 0, trailing: 0)) .listRowSeparator(.hidden) From 5917d46ee127f7c3ebcbf77204fe9315df47aeb3 Mon Sep 17 00:00:00 2001 From: Valentin Perignon Date: Wed, 31 May 2023 13:44:38 +0200 Subject: [PATCH 2/6] feat: Do not animate cell selection --- Mail/Components/CheckboxView.swift | 2 ++ Mail/Components/SelectionBackground.swift | 2 ++ 2 files changed, 4 insertions(+) diff --git a/Mail/Components/CheckboxView.swift b/Mail/Components/CheckboxView.swift index 2cddb342d..be0c7a532 100644 --- a/Mail/Components/CheckboxView.swift +++ b/Mail/Components/CheckboxView.swift @@ -22,6 +22,7 @@ import SwiftUI struct CheckboxView: View { @AppStorage(UserDefaults.shared.key(.accentColor)) private var accentColor = DefaultPreferences.accentColor + let isSelected: Bool let size: CGFloat @@ -41,6 +42,7 @@ struct CheckboxView: View { .frame(height: UIConstants.checkmarkSize) .opacity(isSelected ? 1 : 0) } + .animation(nil, value: isSelected) } } diff --git a/Mail/Components/SelectionBackground.swift b/Mail/Components/SelectionBackground.swift index 7dd2e8eb5..f0ba725a0 100644 --- a/Mail/Components/SelectionBackground.swift +++ b/Mail/Components/SelectionBackground.swift @@ -60,6 +60,7 @@ struct SelectionBackground: View { let selectionType: SelectionBackgroundKind var paddingLeading: CGFloat = 8 + var withAnimation = true var body: some View { Rectangle() @@ -68,6 +69,7 @@ struct SelectionBackground: View { .padding(.leading, paddingLeading) .padding(.vertical, selectionType.verticalPadding) .opacity(selectionType.opacity) + .animation(withAnimation ? .default : nil, value: selectionType.opacity) } } From ef1003af1873330ffc67acdf649749e1e0b12d9b Mon Sep 17 00:00:00 2001 From: Valentin Perignon Date: Wed, 31 May 2023 13:45:09 +0200 Subject: [PATCH 3/6] fix: Display circle for all cells --- Mail/Views/Thread List/ThreadListCell.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Mail/Views/Thread List/ThreadListCell.swift b/Mail/Views/Thread List/ThreadListCell.swift index 593cd565f..8fc80774d 100644 --- a/Mail/Views/Thread List/ThreadListCell.swift +++ b/Mail/Views/Thread List/ThreadListCell.swift @@ -29,7 +29,7 @@ struct ThreadListCell: View { let thread: Thread let viewModel: ThreadListViewModel - let multipleSelectionViewModel: ThreadListMultipleSelectionViewModel + @ObservedObject var multipleSelectionViewModel: ThreadListMultipleSelectionViewModel let threadDensity: ThreadDensity @@ -52,7 +52,8 @@ struct ThreadListCell: View { isMultipleSelectionEnabled: multipleSelectionViewModel.isEnabled, isSelected: isMultiSelected ) - .background(SelectionBackground(selectionType: selectionType, paddingLeading: 4)) + .background(SelectionBackground(selectionType: selectionType, paddingLeading: 4, withAnimation: false)) + .contentShape(Rectangle()) .onTapGesture { didTapCell() } .onLongPressGesture { didLongPressCell() } .swipeActions(thread: thread, viewModel: viewModel, multipleSelectionViewModel: multipleSelectionViewModel) From a3edab11fd74f78df8fd73a5b4b3c64f8d558e56 Mon Sep 17 00:00:00 2001 From: Valentin Perignon Date: Wed, 31 May 2023 15:56:28 +0200 Subject: [PATCH 4/6] feat: Improve default et compact animations --- Mail/Components/ThreadCell.swift | 36 +++++++++---------- Mail/Utils/FloatingActionButtonModifier.swift | 3 +- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/Mail/Components/ThreadCell.swift b/Mail/Components/ThreadCell.swift index 56e5a5b27..8f95f6462 100644 --- a/Mail/Components/ThreadCell.swift +++ b/Mail/Components/ThreadCell.swift @@ -21,15 +21,8 @@ import MailResources import SwiftUI extension Animation { - static func threadListCheckbox(isMultipleSelectionEnabled isEnabled: Bool) -> Animation { - .default.delay(isEnabled ? 0.5 : 0) - } - - static func threadListSlide(density: ThreadDensity, isMultipleSelectionEnabled isEnabled: Bool) -> Animation { - if density == .large { - return .default - } - return .default.speed(2).delay(isEnabled ? 0 : 0.22) + static var threadListSlide: Animation { + .default.speed(2) } } @@ -89,6 +82,8 @@ struct ThreadCell: View { let isMultipleSelectionEnabled: Bool let isSelected: Bool + @State private var shouldDisplayCheckbox = false + private var checkboxSize: CGFloat { density == .large ? UIConstants.checkboxLargeSize : UIConstants.checkboxSize } @@ -124,10 +119,6 @@ struct ThreadCell: View { var body: some View { HStack(spacing: 8) { UnreadIndicatorView(hidden: !thread.hasUnseenMessages) - .animation( - .threadListSlide(density: density, isMultipleSelectionEnabled: isMultipleSelectionEnabled), - value: isMultipleSelectionEnabled - ) .accessibilityLabel(additionalAccessibilityLabel) .accessibilityHidden(additionalAccessibilityLabel.isEmpty) @@ -137,14 +128,12 @@ struct ThreadCell: View { AvatarView(avatarDisplayable: recipient, size: 40) CheckboxView(isSelected: isSelected, density: density) .opacity(isSelected ? 1 : 0) + .animation(nil, value: isSelected) } .accessibility(hidden: true) } else if isMultipleSelectionEnabled { CheckboxView(isSelected: isSelected, density: density) - .animation( - .threadListCheckbox(isMultipleSelectionEnabled: isMultipleSelectionEnabled), - value: isMultipleSelectionEnabled - ) + .opacity(shouldDisplayCheckbox ? 1 : 0) } } .padding(.trailing, 4) @@ -159,7 +148,7 @@ struct ThreadCell: View { } } .animation( - .threadListSlide(density: density, isMultipleSelectionEnabled: isMultipleSelectionEnabled), + isMultipleSelectionEnabled ? .threadListSlide : .threadListSlide.delay(0.3), value: isMultipleSelectionEnabled ) } @@ -168,6 +157,17 @@ struct ThreadCell: View { .padding(.vertical, density.cellVerticalPadding) .clipped() .accessibilityElement(children: .combine) + .onChange(of: isMultipleSelectionEnabled) { newValue in + if newValue { + DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) { + withAnimation { + self.shouldDisplayCheckbox = true + } + } + } else { + self.shouldDisplayCheckbox = false + } + } } } diff --git a/Mail/Utils/FloatingActionButtonModifier.swift b/Mail/Utils/FloatingActionButtonModifier.swift index 55e2942eb..68fc263a3 100644 --- a/Mail/Utils/FloatingActionButtonModifier.swift +++ b/Mail/Utils/FloatingActionButtonModifier.swift @@ -42,7 +42,8 @@ struct FloatingActionButtonModifier: ViewModifier { } extension View { - func floatingActionButton(isEnabled: Bool = true, icon: MailResourcesImages, title: String, action: @escaping () -> Void) -> some View { + func floatingActionButton(isEnabled: Bool = true, icon: MailResourcesImages, title: String, + action: @escaping () -> Void) -> some View { modifier(FloatingActionButtonModifier(isEnabled: isEnabled, icon: icon, title: title, action: action)) } } From ccea903d28d32d06d7005d58af7cb2b5f0abf97b Mon Sep 17 00:00:00 2001 From: Valentin Perignon Date: Thu, 1 Jun 2023 09:29:35 +0200 Subject: [PATCH 5/6] refactor: Clean code --- Mail/Components/ThreadCell.swift | 31 ++++++++++++++++--------------- MailCore/UI/UIConstants.swift | 3 +++ 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/Mail/Components/ThreadCell.swift b/Mail/Components/ThreadCell.swift index 8f95f6462..6e8254221 100644 --- a/Mail/Components/ThreadCell.swift +++ b/Mail/Components/ThreadCell.swift @@ -74,6 +74,10 @@ struct ThreadCellDataHolder { struct ThreadCell: View { @EnvironmentObject private var mailboxManager: MailboxManager + + /// With normal or compact density, the checkbox should appear and disappear at different times of the cell offset. + @State private var shouldDisplayCheckbox = false + let thread: Thread let dataHolder: ThreadCellDataHolder @@ -82,8 +86,6 @@ struct ThreadCell: View { let isMultipleSelectionEnabled: Bool let isSelected: Bool - @State private var shouldDisplayCheckbox = false - private var checkboxSize: CGFloat { density == .large ? UIConstants.checkboxLargeSize : UIConstants.checkboxSize } @@ -99,12 +101,7 @@ struct ThreadCell: View { return label } - init( - thread: Thread, - density: ThreadDensity, - isMultipleSelectionEnabled: Bool = false, - isSelected: Bool = false - ) { + init(thread: Thread, density: ThreadDensity, isMultipleSelectionEnabled: Bool = false, isSelected: Bool = false) { self.thread = thread dataHolder = ThreadCellDataHolder(thread: thread) @@ -134,6 +131,7 @@ struct ThreadCell: View { } else if isMultipleSelectionEnabled { CheckboxView(isSelected: isSelected, density: density) .opacity(shouldDisplayCheckbox ? 1 : 0) + .animation(.default.speed(1.5), value: shouldDisplayCheckbox) } } .padding(.trailing, 4) @@ -148,7 +146,7 @@ struct ThreadCell: View { } } .animation( - isMultipleSelectionEnabled ? .threadListSlide : .threadListSlide.delay(0.3), + isMultipleSelectionEnabled ? .threadListSlide : .threadListSlide.delay(UIConstants.checkboxDisappearOffsetDelay), value: isMultipleSelectionEnabled ) } @@ -157,15 +155,18 @@ struct ThreadCell: View { .padding(.vertical, density.cellVerticalPadding) .clipped() .accessibilityElement(children: .combine) - .onChange(of: isMultipleSelectionEnabled) { newValue in - if newValue { - DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) { - withAnimation { + .onChange(of: isMultipleSelectionEnabled) { isEnabled in + guard density != .large else { return } + + withAnimation { + if isEnabled { + // We should wait a bit before showing the checkbox + DispatchQueue.main.asyncAfter(deadline: .now() + UIConstants.checkboxAppearDelay) { self.shouldDisplayCheckbox = true } + } else { + self.shouldDisplayCheckbox = false } - } else { - self.shouldDisplayCheckbox = false } } } diff --git a/MailCore/UI/UIConstants.swift b/MailCore/UI/UIConstants.swift index 038036d45..10343988a 100644 --- a/MailCore/UI/UIConstants.swift +++ b/MailCore/UI/UIConstants.swift @@ -91,6 +91,9 @@ public enum UIConstants { public static let checkmarkSize: CGFloat = 14 public static let checkboxLargeSize: CGFloat = 40 + public static let checkboxAppearDelay = 0.2 + public static let checkboxDisappearOffsetDelay = 0.35 + public static let buttonsRadius: CGFloat = 16 public static let buttonsIconSize: CGFloat = 16 From 57ebe62766e4fe34809b870a8f40a86e95d8cf16 Mon Sep 17 00:00:00 2001 From: Valentin Perignon Date: Thu, 1 Jun 2023 11:53:42 +0200 Subject: [PATCH 6/6] feat: Improve animation when checkbox is filled --- Mail/Components/CheckboxView.swift | 2 +- Mail/Views/Thread List/ThreadListCell.swift | 14 +++++++------- Mail/Views/Thread List/ThreadListView.swift | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Mail/Components/CheckboxView.swift b/Mail/Components/CheckboxView.swift index be0c7a532..a1d34a6f4 100644 --- a/Mail/Components/CheckboxView.swift +++ b/Mail/Components/CheckboxView.swift @@ -34,7 +34,7 @@ struct CheckboxView: View { var body: some View { ZStack { Circle() - .strokeBorder(Color.accentColor, lineWidth: 2) + .strokeBorder(Color.accentColor, lineWidth: isSelected ? 0 : 2) .background(Circle().fill(isSelected ? Color.accentColor : Color.clear)) .frame(width: size, height: size) MailResourcesAsset.check.swiftUIImage diff --git a/Mail/Views/Thread List/ThreadListCell.swift b/Mail/Views/Thread List/ThreadListCell.swift index 8fc80774d..f6f96ea05 100644 --- a/Mail/Views/Thread List/ThreadListCell.swift +++ b/Mail/Views/Thread List/ThreadListCell.swift @@ -26,18 +26,18 @@ import SwiftUI struct ThreadListCell: View { @EnvironmentObject var splitViewManager: SplitViewManager - let thread: Thread - let viewModel: ThreadListViewModel @ObservedObject var multipleSelectionViewModel: ThreadListMultipleSelectionViewModel + @Binding var editedMessageDraft: Draft? + + let thread: Thread + let threadDensity: ThreadDensity let isSelected: Bool let isMultiSelected: Bool - @Binding var editedMessageDraft: Draft? - private var selectionType: SelectionBackgroundKind { if multipleSelectionViewModel.isEnabled { return isMultiSelected ? .multiple : .none @@ -101,15 +101,15 @@ struct ThreadListCell: View { struct ThreadListCell_Previews: PreviewProvider { static var previews: some View { ThreadListCell( - thread: PreviewHelper.sampleThread, viewModel: ThreadListViewModel(mailboxManager: PreviewHelper.sampleMailboxManager, folder: PreviewHelper.sampleFolder, isCompact: false), multipleSelectionViewModel: ThreadListMultipleSelectionViewModel(mailboxManager: PreviewHelper.sampleMailboxManager), + editedMessageDraft: .constant(nil), + thread: PreviewHelper.sampleThread, threadDensity: .large, isSelected: false, - isMultiSelected: false, - editedMessageDraft: .constant(nil) + isMultiSelected: false ) } } diff --git a/Mail/Views/Thread List/ThreadListView.swift b/Mail/Views/Thread List/ThreadListView.swift index f88e85434..efbfc8e5a 100644 --- a/Mail/Views/Thread List/ThreadListView.swift +++ b/Mail/Views/Thread List/ThreadListView.swift @@ -120,14 +120,14 @@ struct ThreadListView: View { ForEach(viewModel.sections) { section in Section { ForEach(section.threads) { thread in - ThreadListCell(thread: thread, - viewModel: viewModel, + ThreadListCell(viewModel: viewModel, multipleSelectionViewModel: multipleSelectionViewModel, + editedMessageDraft: $editedMessageDraft, + thread: thread, threadDensity: threadDensity, isSelected: viewModel.selectedThread?.uid == thread.uid, isMultiSelected: multipleSelectionViewModel.selectedItems - .contains { $0.id == thread.id }, - editedMessageDraft: $editedMessageDraft) + .contains { $0.id == thread.id }) .id(thread.id) } } header: {