From 6fae40368fb17d78bbf32559da09e0029e9b1e1a Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sun, 18 Oct 2026 10:01:57 +0700 Subject: [PATCH] fix(settings): stop the Themes list jumping when accessibility reads it --- .../Appearance/ThemeLibrarySection.swift | 4 + .../Settings/AppearancePaneListTests.swift | 80 +++++++++++++++++-- 2 files changed, 78 insertions(+), 6 deletions(-) diff --git a/TablePro/Views/Settings/Appearance/ThemeLibrarySection.swift b/TablePro/Views/Settings/Appearance/ThemeLibrarySection.swift index 16ce33c852..4409346595 100644 --- a/TablePro/Views/Settings/Appearance/ThemeLibrarySection.swift +++ b/TablePro/Views/Settings/Appearance/ThemeLibrarySection.swift @@ -30,6 +30,9 @@ internal struct ThemeLibrarySection: View { } private static let minimumListHeight: CGFloat = 160 + // A two-line theme row. The table sizes a row it has not measured at this estimate, and an + // accessibility read makes it drop and remeasure off-screen rows, so any other value moves the rows. + private static let rowHeight: CGFloat = 42 /// 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 @@ -121,6 +124,7 @@ internal struct ThemeLibrarySection: View { } } .listStyle(.bordered(alternatesRowBackgrounds: false)) + .environment(\.defaultMinListRowHeight, Self.rowHeight) .frame(minHeight: Self.minimumListHeight, maxHeight: .infinity) .accessibilityIdentifier("appearance-themes-list") .contextMenu(forSelectionType: ThemeLibraryItem.self) { items in diff --git a/TableProTests/Views/Settings/AppearancePaneListTests.swift b/TableProTests/Views/Settings/AppearancePaneListTests.swift index 1ec4be82b4..45f6b02e22 100644 --- a/TableProTests/Views/Settings/AppearancePaneListTests.swift +++ b/TableProTests/Views/Settings/AppearancePaneListTests.swift @@ -1,4 +1,5 @@ import AppKit +import ApplicationServices @testable import TablePro import SwiftUI import Testing @@ -7,6 +8,14 @@ import Testing /// themes below the first few rows could never be reached. @MainActor struct AppearancePaneListTests { + private struct HostedPane { + let window: NSWindow + let table: NSTableView + let scrollView: NSScrollView + } + + private static let windowTitle = "AppearancePaneListTests" + private static func find(_ type: T.Type, in view: NSView) -> [T] { var found: [T] = [] var pending = [view] @@ -33,8 +42,7 @@ struct AppearancePaneListTests { 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 { + private static func hostPane() throws -> HostedPane { let paneSize = SettingsPaneTabViewController.paneSize let window = NSWindow( contentRect: NSRect(origin: .zero, size: paneSize), @@ -43,6 +51,7 @@ struct AppearancePaneListTests { defer: false ) window.isReleasedWhenClosed = false + window.title = windowTitle // 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 = [] @@ -50,21 +59,80 @@ struct AppearancePaneListTests { 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 }, + 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") + return HostedPane(window: window, table: table, scrollView: scrollView) + } + + private static func elements(_ attribute: String, of element: AXUIElement) -> [AXUIElement] { + var value: CFTypeRef? + guard AXUIElementCopyAttributeValue(element, attribute as CFString, &value) == .success else { return [] } + return value as? [AXUIElement] ?? [] + } + + private static func title(of element: AXUIElement) -> String? { + var value: CFTypeRef? + guard AXUIElementCopyAttributeValue(element, kAXTitleAttribute as CFString, &value) == .success else { return nil } + return value as? String + } + + // The AX API reaches rows outside the visible rect, as an XCUITest snapshot or VoiceOver does; + // accessibilityChildren() called in process stops at the visible ones. + private static func readTree(of element: AXUIElement) -> Int { + for attribute in [kAXPositionAttribute, kAXSizeAttribute, kAXValueAttribute] { + var value: CFTypeRef? + _ = AXUIElementCopyAttributeValue(element, attribute as CFString, &value) + } + return elements(kAXChildrenAttribute, of: element).reduce(1) { $0 + readTree(of: $1) } + } + + @Test("The Themes list scrolls on its own when it holds more themes than it shows") + func themesListScrolls() throws { + let pane = try Self.hostPane() + defer { pane.window.orderOut(nil) } + let scrollView = pane.scrollView + #expect(pane.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)) + scrollView.scrollWheel(with: try Self.scrollEvent(over: scrollView, in: pane.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) } + + @Test("Reading the Themes list for accessibility leaves its rows where they are") + func themesListHoldsStillWhileAccessibilityReadsIt() throws { + let pane = try Self.hostPane() + pane.window.orderFront(nil) + defer { pane.window.orderOut(nil) } + let table = pane.table + let clipView = pane.scrollView.contentView + + let window = try #require( + Self.elements(kAXWindowsAttribute, of: AXUIElementCreateApplication(getpid())) + .first { Self.title(of: $0) == Self.windowTitle }, + "The pane's window should be in the accessibility tree" + ) + #expect(Self.readTree(of: window) > table.numberOfRows, "The read should reach every row") + + for _ in 0..<12 { + pane.scrollView.scrollWheel(with: try Self.scrollEvent(over: pane.scrollView, in: pane.window)) + RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.3)) + let shown = table.rows(in: clipView.documentVisibleRect) + _ = Self.readTree(of: window) + RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.1)) + let after = table.rows(in: clipView.documentVisibleRect) + #expect(after.location == shown.location, "Rows \(shown) moved to \(after) while nothing scrolled") + } + #expect( + table.rows(in: clipView.documentVisibleRect).contains(table.numberOfRows - 1), + "Scrolling to the end should show the last theme" + ) + } }