Skip to content

Commit 7986f5a

Browse files
ampagenttmm
andcommitted
fix(zbugs-swift): sheets that fit an iPhone, and an AppKit list like the SwiftUI one
From running the app on a Mac and an iPhone: - The sheets' minimum size, for macOS, made them wider than an iPhone, clipping Create. On macOS the issue forms are grouped, like the others, not labels in a column beside the fields. - The AppKit list truncates its titles instead of clipping them, shows the status icon, and highlights the issue the detail shows, as the UIKit list now does on an iPad. - Tapping the issue the detail shows opens it again from the UIKit list, which doesn't change the selection. - A row's meta line doesn't wrap before its labels. - On a device, the UI test allows local network access. Amp-Thread-ID: https://ampcode.com/threads/T-c513a037-a257-45e0-b9a9-15c15edfe859 Co-authored-by: tmm <tmm@tmm.dev>
1 parent 2344a0a commit 7986f5a

6 files changed

Lines changed: 99 additions & 16 deletions

File tree

‎apps/zbugs-swift/App/UITests/ScreenshotTests.swift‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ final class ScreenshotTests: XCTestCase {
2525
app.launchEnvironment[key] = environment[key]
2626
}
2727
app.launch()
28+
#if os(iOS) && !targetEnvironment(simulator)
29+
// A device asks for local network access on the first launch.
30+
let allow = XCUIApplication(bundleIdentifier: "com.apple.springboard").alerts.buttons["Allow"]
31+
if allow.waitForExistence(timeout: 10) { allow.tap() }
32+
#endif
2833

2934
// The issue list, once zero-cache has synced it.
3035
let firstIssue = app.descendants(matching: .any).matching(identifier: "IssueRow").firstMatch

‎apps/zbugs-swift/Sources/ZBugsUI/IssueDetail.swift‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@
170170
TextField("Description", text: $description, axis: .vertical)
171171
.lineLimit(5...20)
172172
}
173+
.formStyle(.grouped)
173174
.navigationTitle("Edit Issue")
174175
.toolbar {
175176
ToolbarItem(placement: .cancellationAction) {
@@ -187,7 +188,10 @@
187188
}
188189
}
189190
}
190-
.frame(minWidth: 420, minHeight: 300)
191+
// A sheet on iOS is the screen's width.
192+
#if os(macOS)
193+
.frame(minWidth: 420, minHeight: 300)
194+
#endif
191195
}
192196
}
193197

‎apps/zbugs-swift/Sources/ZBugsUI/IssueList.swift‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,11 @@
7575
Text(issue.title).font(.headline).lineLimit(2)
7676
}
7777
HStack(spacing: 6) {
78-
Text(issue.subtitle)
78+
// On one line, before the labels.
79+
Text(issue.subtitle).lineLimit(1).layoutPriority(1)
7980
ForEach(issue.labels ?? [], id: \.id) { label in
8081
Text(label.name)
82+
.lineLimit(1)
8183
.padding(.horizontal, 6)
8284
.background(.quaternary, in: Capsule())
8385
}

‎apps/zbugs-swift/Sources/ZBugsUI/IssueTable.swift‎

Lines changed: 65 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,19 @@
4242
adapter = DiffableAdapter(view, section: 0, dataSource: dataSource)
4343
}
4444

45+
/// Highlights the issue the detail shows, except on an iPhone, where
46+
/// the list and the issue aren't side by side.
47+
func select(_ issueID: String?) {
48+
guard traitCollection.horizontalSizeClass != .compact else { return }
49+
let indexPath = issueID.flatMap { issueID in
50+
adapter?.rows.first { $0.value.id == issueID }
51+
.flatMap { dataSource?.indexPath(for: $0.key) }
52+
}
53+
if indexPath != tableView.indexPathForSelectedRow {
54+
tableView.selectRow(at: indexPath, animated: false, scrollPosition: .none)
55+
}
56+
}
57+
4558
override func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {
4659
guard let id = dataSource?.itemIdentifier(for: indexPath),
4760
let issue = adapter?.row(for: id)
@@ -53,15 +66,19 @@
5366
struct IssueTable: UIViewControllerRepresentable {
5467
let zero: Zero<AuthData?>
5568
let filter: IssueFilter
56-
@Binding var selection: String?
69+
/// The issue the detail shows.
70+
let selection: String?
71+
/// Opens an issue, even the one the detail shows.
72+
let open: (String) -> Void
5773

5874
func makeUIViewController(context: Context) -> IssueTableController {
5975
IssueTableController(style: .plain)
6076
}
6177

6278
func updateUIViewController(_ controller: IssueTableController, context: Context) {
63-
controller.onSelect = { selection = $0 }
79+
controller.onSelect = open
6480
controller.show(zero, filter)
81+
controller.select(selection)
6582
}
6683
}
6784
#elseif canImport(AppKit)
@@ -74,6 +91,9 @@
7491
private var adapter: DiffableAdapter<Issue, Int>?
7592
private var dataSource: NSTableViewDiffableDataSource<Int, RowID>?
7693
private var key: (zero: ObjectIdentifier, filter: IssueFilter)?
94+
/// Whether `select` is changing the selection, which isn't the user
95+
/// choosing an issue.
96+
private var selecting = false
7797

7898
override func loadView() {
7999
let column = NSTableColumn(identifier: .init("issue"))
@@ -86,18 +106,35 @@
86106
dataSource = NSTableViewDiffableDataSource(tableView: tableView) {
87107
[unowned self] _, _, _, id in
88108
let cell = NSTableCellView()
89-
cell.setAccessibilityIdentifier("IssueCell")
109+
let icon = NSImageView()
110+
icon.symbolConfiguration = .init(textStyle: .headline)
90111
let title = NSTextField(labelWithString: "")
91112
title.font = .preferredFont(forTextStyle: .headline)
92113
title.lineBreakMode = .byTruncatingTail
114+
// The cell view's identifier doesn't reach the accessibility tree.
115+
title.setAccessibilityIdentifier("IssueCell")
93116
let subtitle = NSTextField(labelWithString: "")
94117
subtitle.font = .preferredFont(forTextStyle: .caption1)
95118
subtitle.textColor = .secondaryLabelColor
119+
subtitle.lineBreakMode = .byTruncatingTail
120+
// Truncated, not clipped, in a column narrower than the text.
121+
for label in [title, subtitle] {
122+
label.setContentCompressionResistancePriority(.defaultLow, for: .horizontal)
123+
}
96124
if let issue = adapter?.row(for: id) {
97-
title.stringValue = (issue.open ? "○ " : "✓ ") + issue.title
125+
icon.image = NSImage(
126+
systemSymbolName: issue.open ? "circle" : "checkmark.circle.fill",
127+
accessibilityDescription: issue.open ? "Open" : "Closed")
128+
icon.contentTintColor = issue.open ? .systemGreen : .systemPurple
129+
title.stringValue = issue.title
98130
subtitle.stringValue = issue.subtitle
99131
}
100-
let stack = NSStackView(views: [title, subtitle])
132+
// So a selected row draws them in its colors.
133+
cell.imageView = icon
134+
cell.textField = title
135+
let heading = NSStackView(views: [icon, title])
136+
heading.spacing = 6
137+
let stack = NSStackView(views: [heading, subtitle])
101138
stack.orientation = .vertical
102139
stack.alignment = .leading
103140
stack.spacing = 2
@@ -127,8 +164,24 @@
127164
adapter = DiffableAdapter(view, section: 0, dataSource: dataSource)
128165
}
129166

167+
/// Highlights the issue the detail shows.
168+
func select(_ issueID: String?) {
169+
let row = issueID.flatMap { issueID in
170+
adapter?.rows.first { $0.value.id == issueID }
171+
.flatMap { dataSource?.snapshot().indexOfItem($0.key) }
172+
}
173+
guard (row ?? -1) != tableView.selectedRow else { return }
174+
selecting = true
175+
defer { selecting = false }
176+
if let row {
177+
tableView.selectRowIndexes([row], byExtendingSelection: false)
178+
} else {
179+
tableView.deselectAll(nil)
180+
}
181+
}
182+
130183
func tableViewSelectionDidChange(_ notification: Notification) {
131-
guard tableView.selectedRow >= 0,
184+
guard !selecting, tableView.selectedRow >= 0,
132185
let id = dataSource?.itemIdentifier(forRow: tableView.selectedRow),
133186
let issue = adapter?.row(for: id)
134187
else { return }
@@ -139,15 +192,19 @@
139192
struct IssueTable: NSViewControllerRepresentable {
140193
let zero: Zero<AuthData?>
141194
let filter: IssueFilter
142-
@Binding var selection: String?
195+
/// The issue the detail shows.
196+
let selection: String?
197+
/// Opens an issue.
198+
let open: (String) -> Void
143199

144200
func makeNSViewController(context: Context) -> IssueTableController {
145201
IssueTableController()
146202
}
147203

148204
func updateNSViewController(_ controller: IssueTableController, context: Context) {
149-
controller.onSelect = { selection = $0 }
205+
controller.onSelect = open
150206
controller.show(zero, filter)
207+
controller.select(selection)
151208
}
152209
}
153210
#endif

‎apps/zbugs-swift/Sources/ZBugsUI/RootView.swift‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,8 @@
7878
ContentUnavailableView("No Issue Selected", systemImage: "list.bullet.rectangle")
7979
}
8080
}
81-
// The UIKit and AppKit lists set the selection outside a List, which
82-
// a collapsed split view doesn't follow on its own.
81+
// A new issue is selected outside a List, which a collapsed split
82+
// view doesn't follow on its own.
8383
.onChange(of: selection) { _, selection in
8484
if selection != nil { compactColumn = .detail }
8585
}
@@ -114,7 +114,12 @@
114114
case .swiftUI:
115115
IssueList(filter: filter, selection: $selection)
116116
case .native:
117-
IssueTable(zero: model.zero, filter: filter, selection: $selection)
117+
IssueTable(zero: model.zero, filter: filter, selection: selection) { id in
118+
selection = id
119+
// Also when it's the issue the detail shows, which doesn't change
120+
// the selection.
121+
compactColumn = .detail
122+
}
118123
}
119124
}
120125

‎apps/zbugs-swift/Sources/ZBugsUI/Sheets.swift‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
TextField("Description", text: $description, axis: .vertical)
2828
.lineLimit(5...20)
2929
}
30+
.formStyle(.grouped)
3031
.navigationTitle("New Issue")
3132
.toolbar {
3233
ToolbarItem(placement: .cancellationAction) {
@@ -49,7 +50,10 @@
4950
}
5051
}
5152
}
52-
.frame(minWidth: 420, minHeight: 300)
53+
// A sheet on iOS is the screen's width.
54+
#if os(macOS)
55+
.frame(minWidth: 420, minHeight: 300)
56+
#endif
5357
}
5458

5559
private var project: Project? {
@@ -93,7 +97,10 @@
9397
}
9498
}
9599
}
96-
.frame(minWidth: 420, minHeight: 320)
100+
// A sheet on iOS is the screen's width.
101+
#if os(macOS)
102+
.frame(minWidth: 420, minHeight: 320)
103+
#endif
97104
}
98105

99106
private func logInWithGitHub() async {
@@ -156,7 +163,10 @@
156163
apiURL = model.apiURL
157164
}
158165
}
159-
.frame(minWidth: 420, minHeight: 220)
166+
// A sheet on iOS is the screen's width.
167+
#if os(macOS)
168+
.frame(minWidth: 420, minHeight: 220)
169+
#endif
160170
}
161171
}
162172
#endif

0 commit comments

Comments
 (0)