Skip to content

Commit

Permalink
Fix favorites grid layout issues on New Tab Page (#3568)
Browse files Browse the repository at this point in the history
<!--
Note: This checklist is a reminder of our shared engineering
expectations. Feel free to change it, although assigning a GitHub
reviewer and the items in bold are required.

⚠️ If you're an external contributor, please file an issue first before
working on a PR, as we can't guarantee that we will accept your changes
if they haven't been discussed ahead of time. Thanks!
-->

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)
dus7 authored Nov 12, 2024
1 parent b020e7f commit d62e0c4
Showing 6 changed files with 63 additions and 19 deletions.
2 changes: 1 addition & 1 deletion DuckDuckGo/EditableShortcutsView.swift
Original file line number Diff line number Diff line change
@@ -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 {
6 changes: 4 additions & 2 deletions DuckDuckGo/FavoritesView.swift
Original file line number Diff line number Diff line change
@@ -36,10 +36,12 @@ struct FavoritesView<Model: FavoritesViewModel>: 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()
3 changes: 2 additions & 1 deletion DuckDuckGo/FavoritesViewModel.swift
Original file line number Diff line number Diff line change
@@ -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
56 changes: 46 additions & 10 deletions DuckDuckGo/NewTabPageGridView.swift
Original file line number Diff line number Diff line change
@@ -24,14 +24,15 @@ struct NewTabPageGridView<Content: View>: 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<Content: View>: 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<Content: View>: 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
}
2 changes: 1 addition & 1 deletion DuckDuckGo/ShortcutsView.swift
Original file line number Diff line number Diff line change
@@ -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)
13 changes: 9 additions & 4 deletions DuckDuckGo/SimpleNewTabPageView.swift
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit d62e0c4

Please sign in to comment.