diff --git a/TablePro/Views/Settings/Appearance/ThemeLibrarySection.swift b/TablePro/Views/Settings/Appearance/ThemeLibrarySection.swift index ac41ca7b59..16ce33c852 100644 --- a/TablePro/Views/Settings/Appearance/ThemeLibrarySection.swift +++ b/TablePro/Views/Settings/Appearance/ThemeLibrarySection.swift @@ -29,11 +29,20 @@ internal struct ThemeLibrarySection: View { let fileName: String } - // Fits the Settings pane's default 500 pt height under the two sections above it. - private static let listHeight: CGFloat = 176 + private static let minimumListHeight: CGFloat = 160 + /// A grouped Form centers its sections at most this wide (measured on macOS 27), so the list keeps + /// the edges of the sections above it. + private static let formContentWidth: CGFloat = 704 + private static let formMargin: CGFloat = 20 + /// How far a grouped Form indents its section headers from the section's edge. + private static let headerInset: CGFloat = 10 internal var body: some View { - Section { + VStack(alignment: .leading, spacing: 8) { + Text("Themes") + .font(.headline) + .padding(.horizontal, Self.headerInset) + .accessibilityAddTraits(.isHeader) list .alert( model.alert?.title ?? "", @@ -66,9 +75,11 @@ internal struct ThemeLibrarySection: View { contentType: .json, defaultFilename: exporting?.fileName ) { _ in } - } header: { - Text("Themes") } + .frame(maxWidth: Self.formContentWidth) + .padding(.horizontal, Self.formMargin) + .padding(.bottom, Self.formMargin) + .frame(maxWidth: .infinity) } // MARK: - List @@ -110,7 +121,8 @@ internal struct ThemeLibrarySection: View { } } .listStyle(.bordered(alternatesRowBackgrounds: false)) - .frame(height: Self.listHeight) + .frame(minHeight: Self.minimumListHeight, maxHeight: .infinity) + .accessibilityIdentifier("appearance-themes-list") .contextMenu(forSelectionType: ThemeLibraryItem.self) { items in rowMenu(for: items.first) } primaryAction: { items in diff --git a/TablePro/Views/Settings/AppearanceSettingsView.swift b/TablePro/Views/Settings/AppearanceSettingsView.swift index 37d750ee7c..504099bb28 100644 --- a/TablePro/Views/Settings/AppearanceSettingsView.swift +++ b/TablePro/Views/Settings/AppearanceSettingsView.swift @@ -10,6 +10,15 @@ internal struct AppearanceSettingsView: View { private static let modes: [AppAppearanceMode] = [.auto, .light, .dark] internal var body: some View { + VStack(spacing: 0) { + settingsForm + ThemeLibrarySection() + } + } + + // A List inside a scrolling grouped Form neither scrolls nor takes keyboard focus (measured on + // macOS 27), so the Form keeps its own height and the theme list sits below it. + private var settingsForm: some View { Form { Section { Picker(String(localized: "Appearance"), selection: $settingsManager.appearanceMode) { @@ -28,10 +37,10 @@ internal struct AppearanceSettingsView: View { } footer: { Text("Themes color the editor, the data grid, other content views and the window background. Controls, sidebars and Settings follow the system appearance.") } - - ThemeLibrarySection() } .formStyle(.grouped) + .scrollDisabled(true) + .fixedSize(horizontal: false, vertical: true) } private static func title(for mode: AppAppearanceMode) -> String { diff --git a/TableProTests/Views/Settings/AppearancePaneListTests.swift b/TableProTests/Views/Settings/AppearancePaneListTests.swift new file mode 100644 index 0000000000..1ec4be82b4 --- /dev/null +++ b/TableProTests/Views/Settings/AppearancePaneListTests.swift @@ -0,0 +1,70 @@ +import AppKit +@testable import TablePro +import SwiftUI +import Testing + +/// A List inside the pane's scrolling grouped Form had its scrolling turned off by SwiftUI, so the +/// themes below the first few rows could never be reached. +@MainActor +struct AppearancePaneListTests { + private static func find(_ type: T.Type, in view: NSView) -> [T] { + var found: [T] = [] + var pending = [view] + while let next = pending.popLast() { + if let match = next as? T { found.append(match) } + pending.append(contentsOf: next.subviews) + } + return found + } + + private static func scrollEvent(over view: NSView, in window: NSWindow) throws -> NSEvent { + let event = try #require(CGEvent( + scrollWheelEvent2Source: nil, + units: .pixel, + wheelCount: 1, + wheel1: -80, + wheel2: 0, + wheel3: 0 + )) + let center = view.convert(NSPoint(x: view.bounds.midX, y: view.bounds.midY), to: nil) + let screenPoint = window.convertPoint(toScreen: center) + let screenHeight = NSScreen.screens.first?.frame.height ?? 0 + event.location = CGPoint(x: screenPoint.x, y: screenHeight - screenPoint.y) + return try #require(NSEvent(cgEvent: event)) + } + + @Test("The Themes list scrolls on its own when it holds more themes than it shows") + func themesListScrolls() throws { + let paneSize = SettingsPaneTabViewController.paneSize + let window = NSWindow( + contentRect: NSRect(origin: .zero, size: paneSize), + styleMask: [.titled], + backing: .buffered, + defer: false + ) + window.isReleasedWhenClosed = false + // As the Settings window hosts a pane: the window keeps its size whatever the content asks. + let hosting = NSHostingController(rootView: AppearanceSettingsView(settings: .constant(.default))) + hosting.sizingOptions = [] + window.contentViewController = hosting + window.setContentSize(paneSize) + window.layoutIfNeeded() + RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.3)) + defer { window.orderOut(nil) } + + let content = try #require(window.contentView) + let table = try #require( + Self.find(NSTableView.self, in: content).first { $0.numberOfRows > 10 }, + "The pane should show the Themes list" + ) + let scrollView = try #require(table.enclosingScrollView) + #expect(table.frame.height > scrollView.contentView.bounds.height, "The fixture needs more themes than fit") + + let before = scrollView.contentView.bounds.origin.y + scrollView.scrollWheel(with: try Self.scrollEvent(over: scrollView, in: window)) + RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.2)) + + #expect(scrollView.contentView.bounds.origin.y > before, "A scroll over the list should move its rows") + #expect(scrollView.verticalScrollElasticity != .none) + } +} diff --git a/TableProUITests/AppearanceSettingsUITests.swift b/TableProUITests/AppearanceSettingsUITests.swift index 0834c4fcab..dbd356f936 100644 --- a/TableProUITests/AppearanceSettingsUITests.swift +++ b/TableProUITests/AppearanceSettingsUITests.swift @@ -52,12 +52,40 @@ final class AppearanceSettingsUITests: UITestCase { ) } + func testThemesListScrollsAndTakesArrowKeys() throws { + let app = try launchOpeningURL("tablepro://settings/appearance") + let settings = try appearancePane(in: app) + let list = settings.outlines.containing(NSPredicate(format: "value == %@", "Default Light")).firstMatch + XCTAssertTrue(list.waitToExist(timeout: 10), "The Themes list should be on the Appearance pane") + // The bar under the list marks where its visible rows end; an outline's own frame spans every row. + let footer = element("appearance-theme-add", in: settings) + XCTAssertTrue(footer.waitToExist(timeout: 10)) + + let last = row("Tokyo Night Light", in: list) + XCTAssertTrue(last.waitToExist(timeout: 10)) + let deadline = Date(timeIntervalSinceNow: 10) + while last.frame.maxY > footer.frame.minY, Date() < deadline { + list.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.1)).scroll(byDeltaX: 0, deltaY: -60) + } + XCTAssertLessThanOrEqual(last.frame.maxY, footer.frame.minY, "A scroll over the list should bring its last theme into view") + + let previous = outlineRow("Tokyo Night", in: list) + let next = outlineRow("Tokyo Night Light", in: list) + XCTAssertTrue(waitUntilHittable(previous, timeout: 5), "Tokyo Night should be visible above the last row") + previous.click() + XCTAssertTrue(waitForPredicate(timeout: 5) { previous.isSelected }, "A click should select the theme") + settings.typeKey(.downArrow, modifierFlags: []) + XCTAssertTrue( + waitForPredicate(timeout: 5) { next.isSelected && !previous.isSelected }, + "Down Arrow should move the selection, so the click must have given the list keyboard focus" + ) + } + // MARK: - Steps private func createTheme(basedOn source: String, in window: XCUIElement) { let add = element("appearance-theme-add", in: window) XCTAssertTrue(add.waitToExist(timeout: 10), "The Themes list should offer its add menu") - revealInForm(add, in: window) XCTAssertTrue(waitUntilHittable(add, timeout: 10), "The Themes list should offer its add menu") XCTAssertTrue( add.label == "Add Theme" || add.title == "Add Theme", @@ -108,7 +136,6 @@ final class AppearanceSettingsUITests: UITestCase { let actions = element("appearance-theme-actions", in: window) XCTAssertTrue(actions.waitToExist(timeout: 10)) - revealInForm(actions, in: window) XCTAssertTrue(waitUntilHittable(actions, timeout: 10)) XCTAssertTrue( actions.label == "Theme Actions" || actions.title == "Theme Actions", @@ -134,7 +161,6 @@ final class AppearanceSettingsUITests: UITestCase { themeRow.click() let remove = window.buttons["appearance-theme-remove"] XCTAssertTrue(remove.waitToExist(timeout: 5)) - revealInForm(remove, in: window) XCTAssertEqual(remove.label, "Move to Trash") XCTAssertTrue(waitForPredicate(timeout: 5) { remove.isEnabled }, "A selected custom theme can be trashed") remove.click() @@ -157,14 +183,6 @@ final class AppearanceSettingsUITests: UITestCase { return settings } - /// The Settings pane scrolls over its top sections, because a scroll over the theme list - /// scrolls the list instead. - private func revealInForm(_ control: XCUIElement, in window: XCUIElement) { - let form = window.scrollViews.firstMatch - XCTAssertTrue(form.waitToExist(timeout: 5)) - reveal(control, in: form, scrollingAt: CGVector(dx: 0.5, dy: 0.05)) - } - /// A control clipped by its scroll view still reads as hittable, and a click on it lands on /// whatever is drawn there, so the frames decide when it is in view. private func reveal(_ control: XCUIElement, in scrollView: XCUIElement, scrollingAt point: CGVector) { @@ -198,6 +216,10 @@ final class AppearanceSettingsUITests: UITestCase { window.staticTexts.matching(NSPredicate(format: "value == %@", name)).firstMatch } + private func outlineRow(_ name: String, in list: XCUIElement) -> XCUIElement { + list.outlineRows.containing(NSPredicate(format: "value == %@", name)).firstMatch + } + private func themeDocument(named name: String) -> [String: Any]? { guard let root = sandboxRoot else { return nil } let directory = root.appendingPathComponent("TablePro/Themes", isDirectory: true)