From d62e0c49f4e2b36e97e80d49a7fa3cc840a22dbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariusz=20=C5=9Apiewak?= Date: Tue, 12 Nov 2024 18:06:37 +0100 Subject: [PATCH] Fix favorites grid layout issues on New Tab Page (#3568) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task/Issue URL: https://app.asana.com/0/1206226850447395/1208709136086082/f Tech Design URL: CC: **Description**: Addresses feedback around updated favorites grid layout on New Tab Page. * Uses static column setup with 4 columns on iPhone and 5 on iPad when NTP customization is turned off. * Reduces the width of grid and makes it centered. * A special case handled for smaller screens so the static layout does not exceed margins. **Steps to test this PR**: For best baseline run 7.142.1 or earlier. 1. Add at least 5 favorites 2. Verify number of columns shown and their count does not change when rotated to landscape: a. 4 on iPhone b. 5 on iPad (unless in compact-sized Split View) **Definition of Done (Internal Only)**: * [x] Does this PR satisfy our [Definition of Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)? **Orientation Testing**: * [x] Portrait * [x] Landscape **Device Testing**: * [x] iPhone SE (1st Gen) * [ ] iPhone 8 * [ ] iPhone X * [x] iPhone 16 Pro * [x] iPad **OS Testing**: * [x] iOS 15 * [ ] iOS 16 * [x] iOS 18 **Theme Testing**: * [x] Light theme * [x] Dark theme --- ###### Internal references: [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) --- DuckDuckGo/EditableShortcutsView.swift | 2 +- DuckDuckGo/FavoritesView.swift | 6 ++- DuckDuckGo/FavoritesViewModel.swift | 3 +- DuckDuckGo/NewTabPageGridView.swift | 56 +++++++++++++++++++++----- DuckDuckGo/ShortcutsView.swift | 2 +- DuckDuckGo/SimpleNewTabPageView.swift | 13 ++++-- 6 files changed, 63 insertions(+), 19 deletions(-) diff --git a/DuckDuckGo/EditableShortcutsView.swift b/DuckDuckGo/EditableShortcutsView.swift index 08ac409016..df40c9cbc8 100644 --- a/DuckDuckGo/EditableShortcutsView.swift +++ b/DuckDuckGo/EditableShortcutsView.swift @@ -28,7 +28,7 @@ struct EditableShortcutsView: View { private let haptics = UIImpactFeedbackGenerator() var body: some View { - NewTabPageGridView(geometry: geometry) { _ in + NewTabPageGridView(geometry: geometry, isUsingDynamicSpacing: true) { _ in ReorderableForEach(model.itemsSettings, id: \.item.id, isReorderingEnabled: true) { setting in let isEnabled = model.enabledItems.contains(setting.item) Button { diff --git a/DuckDuckGo/FavoritesView.swift b/DuckDuckGo/FavoritesView.swift index bb65db7939..a8a7620052 100644 --- a/DuckDuckGo/FavoritesView.swift +++ b/DuckDuckGo/FavoritesView.swift @@ -36,10 +36,12 @@ struct FavoritesView: View { var body: some View { VStack(alignment: .center, spacing: 24) { - let columns = NewTabPageGrid.columnsCount(for: horizontalSizeClass, isLandscape: isLandscape) + let columns = NewTabPageGrid.columnsCount(for: horizontalSizeClass, + isLandscape: isLandscape, + isDynamic: model.isNewTabPageCustomizationEnabled) let result = model.prefixedFavorites(for: columns) - NewTabPageGridView(geometry: geometry) { _ in + NewTabPageGridView(geometry: geometry, isUsingDynamicSpacing: model.isNewTabPageCustomizationEnabled) { _ in ReorderableForEach(result.items) { item in viewFor(item) .previewShape() diff --git a/DuckDuckGo/FavoritesViewModel.swift b/DuckDuckGo/FavoritesViewModel.swift index babd8e20c2..fac9571bd4 100644 --- a/DuckDuckGo/FavoritesViewModel.swift +++ b/DuckDuckGo/FavoritesViewModel.swift @@ -54,7 +54,8 @@ class FavoritesViewModel: ObservableObject { private let favoriteDataSource: NewTabPageFavoriteDataSource private let pixelFiring: PixelFiring.Type private let dailyPixelFiring: DailyPixelFiring.Type - private let isNewTabPageCustomizationEnabled: Bool + + let isNewTabPageCustomizationEnabled: Bool var isEmpty: Bool { allFavorites.filter(\.isFavorite).isEmpty diff --git a/DuckDuckGo/NewTabPageGridView.swift b/DuckDuckGo/NewTabPageGridView.swift index f2b75d76b5..ffcffc0e17 100644 --- a/DuckDuckGo/NewTabPageGridView.swift +++ b/DuckDuckGo/NewTabPageGridView.swift @@ -24,14 +24,15 @@ struct NewTabPageGridView: View { @Environment(\.isLandscapeOrientation) var isLandscape let geometry: GeometryProxy? + let isUsingDynamicSpacing: Bool @ViewBuilder var content: (_ columnsCount: Int) -> Content @State private var width: CGFloat = .zero var body: some View { - let columnsCount = NewTabPageGrid.columnsCount(for: horizontalSizeClass, isLandscape: isLandscape) + let columnsCount = NewTabPageGrid.columnsCount(for: horizontalSizeClass, isLandscape: isLandscape, isDynamic: isUsingDynamicSpacing) - LazyVGrid(columns: flexibleColumns(columnsCount, width: width), spacing: 24, content: { + LazyVGrid(columns: createColumns(columnsCount), spacing: 24, content: { content(columnsCount) }) .frame(maxWidth: .infinity) @@ -41,12 +42,14 @@ struct NewTabPageGridView: View { return geometry[anchor].width }) .onPreferenceChange(FramePreferenceKey.self, perform: { value in - width = value + if isUsingDynamicSpacing { + width = value + } }) .padding(0) } - private func flexibleColumns(_ count: Int, width: CGFloat) -> [GridItem] { + private func flexibleColumns(_ count: Int) -> [GridItem] { let spacing: CGFloat? if width != .zero { let columnsWidth = NewTabPageGrid.Item.edgeSize * Double(count) @@ -62,6 +65,17 @@ struct NewTabPageGridView: View { alignment: .top), count: count) } + + private func staticColumns(_ count: Int) -> [GridItem] { + return Array(repeating: GridItem(.fixed(NewTabPageGrid.Item.edgeSize), + spacing: NewTabPageGrid.Item.staticSpacing, + alignment: .top), + count: count) + } + + private func createColumns(_ count: Int) -> [GridItem] { + isUsingDynamicSpacing ? flexibleColumns(count) : staticColumns(count) + } } private struct FramePreferenceKey: PreferenceKey { @@ -72,17 +86,39 @@ private struct FramePreferenceKey: PreferenceKey { } enum NewTabPageGrid { - enum ColumnCount { - static let compact = 4 - static let regular = 6 + static func columnsCount(for sizeClass: UserInterfaceSizeClass?, isLandscape: Bool, isDynamic: Bool) -> Int { + if isDynamic { + let usesWideLayout = isLandscape || sizeClass == .regular + return usesWideLayout ? ColumnCount.regular : ColumnCount.compact + } else { + return staticGridColumnsCount(for: sizeClass) + } + } + + static func staticGridWidth(for sizeClass: UserInterfaceSizeClass?) -> CGFloat { + let columnsCount = CGFloat(staticGridColumnsCount(for: sizeClass)) + return columnsCount * Item.edgeSize + (columnsCount - 1) * Item.staticSpacing + } + + private static func staticGridColumnsCount(for sizeClass: UserInterfaceSizeClass?) -> Int { + let isPad = UIDevice.current.userInterfaceIdiom == .pad + + return isPad && sizeClass == .regular ? ColumnCount.staticWideLayout : ColumnCount.compact } enum Item { static let edgeSize = 64.0 } +} - static func columnsCount(for sizeClass: UserInterfaceSizeClass?, isLandscape: Bool) -> Int { - let usesWideLayout = isLandscape || sizeClass == .regular - return usesWideLayout ? ColumnCount.regular : ColumnCount.compact +private extension NewTabPageGrid { + enum ColumnCount { + static let compact = 4 + static let regular = 6 + static let staticWideLayout = 5 } } + +private extension NewTabPageGrid.Item { + static let staticSpacing = 32.0 +} diff --git a/DuckDuckGo/ShortcutsView.swift b/DuckDuckGo/ShortcutsView.swift index 230d2a5393..0b83fc7b22 100644 --- a/DuckDuckGo/ShortcutsView.swift +++ b/DuckDuckGo/ShortcutsView.swift @@ -26,7 +26,7 @@ struct ShortcutsView: View { let proxy: GeometryProxy? var body: some View { - NewTabPageGridView(geometry: proxy) { _ in + NewTabPageGridView(geometry: proxy, isUsingDynamicSpacing: true) { _ in ForEach(shortcuts) { shortcut in Button { model.openShortcut(shortcut) diff --git a/DuckDuckGo/SimpleNewTabPageView.swift b/DuckDuckGo/SimpleNewTabPageView.swift index cf31697226..d674e59e32 100644 --- a/DuckDuckGo/SimpleNewTabPageView.swift +++ b/DuckDuckGo/SimpleNewTabPageView.swift @@ -81,7 +81,7 @@ private extension SimpleNewTabPageView { favoritesSectionView(proxy: proxy) } - .padding(Metrics.largePadding) + .padding(sectionsViewPadding(in: proxy)) } .withScrollKeyboardDismiss() } @@ -99,7 +99,7 @@ private extension SimpleNewTabPageView { } .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .top) } - .padding(Metrics.largePadding) + .padding(Metrics.regularPadding) } private var messagesSectionView: some View { @@ -115,6 +115,11 @@ private extension SimpleNewTabPageView { isAddingFavorite: .constant(false), geometry: proxy) } + + private func sectionsViewPadding(in geometry: GeometryProxy) -> CGFloat { + let requiredWidth = NewTabPageGrid.staticGridWidth(for: horizontalSizeClass) + Metrics.regularPadding + return geometry.frame(in: .local).width >= requiredWidth ? Metrics.regularPadding : Metrics.smallPadding + } } private extension View { @@ -130,8 +135,8 @@ private extension View { private struct Metrics { - static let regularPadding = 16.0 - static let largePadding = 24.0 + static let smallPadding = 12.0 + static let regularPadding = 24.0 static let sectionSpacing = 32.0 static let nonGridSectionTopPadding = -8.0