diff --git a/CHANGELOG.md b/CHANGELOG.md index 8178c39c6e..9f048234df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Toolbar Save is a plain checkmark, and the Safe Mode icon is filled only at the two Full levels. (#3250) +- Actions, filter and Disconnect toolbar icons lose their circle on macOS 26 and later, and Actions its chevron. (#3250) + +### Fixed + +- Export in the Structure and object source views showing the Import icon. (#3250) +- VoiceOver reading the welcome window and Integrations filter menus as "chevron.pulldown". (#3250) +- Database icon filled in the database switcher and query editor, outline in the toolbar and sidebar. (#3250) +- Status bar buttons a point or two taller or shorter than each other depending on their icon. (#3250) + ## [0.77.1] - 2026-10-03 ### Added diff --git a/TablePro/Core/Plugins/PluginDriverAdapter.swift b/TablePro/Core/Plugins/PluginDriverAdapter.swift index a6ac056e8d..4e04feae4d 100644 --- a/TablePro/Core/Plugins/PluginDriverAdapter.swift +++ b/TablePro/Core/Plugins/PluginDriverAdapter.swift @@ -820,7 +820,7 @@ final class PluginDriverAdapter: DatabaseDriver, SchemaSwitchable, DatabaseRepor sizeBytes: pluginMeta.sizeBytes, lastAccessed: nil, isSystemDatabase: isSystem, - icon: isSystem ? "gearshape.fill" : "cylinder.fill" + icon: isSystem ? "gearshape" : "cylinder" ) } diff --git a/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Items.swift b/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Items.swift index bce05d46e8..6bc932b041 100644 --- a/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Items.swift +++ b/TablePro/Core/Services/Infrastructure/MainWindowToolbar+Items.swift @@ -104,18 +104,21 @@ extension MainWindowToolbar { /// The long tail of what a context can do, in one control whose menu changes with the tab. /// - /// `ellipsis.circle` is the glyph Finder gives its own Action pull-down. The menu is built by - /// `ConnectionActionsMenuDelegate` when it opens. The overflow entry is AppKit's own and is - /// left to it: measured on macOS 27, an `NSMenuToolbarItem` answers `menuFormRepresentation` - /// with a fresh item titled with its label over this same menu, whatever was assigned, so a - /// narrow window's overflow offers exactly what the control would. + /// The glyph and the missing indicator are Finder's own Action pull-down, which is why they + /// come from `ToolbarSymbols` rather than being named here: Finder draws it differently before + /// and after macOS 26. The menu is built by `ConnectionActionsMenuDelegate` when it opens. The + /// overflow entry is AppKit's own and is left to it: measured on macOS 27, an + /// `NSMenuToolbarItem` answers `menuFormRepresentation` with a fresh item titled with its label + /// over this same menu, whatever was assigned, so a narrow window's overflow offers exactly + /// what the control would. func makeActionsItem() -> NSToolbarItem { let label = String(localized: "Actions") let item = StatefulMenuToolbarItem(itemIdentifier: Self.actions) item.label = label item.paletteLabel = label item.isBordered = true - item.image = NSImage(systemSymbolName: "ellipsis.circle", accessibilityDescription: label) + item.image = NSImage(systemSymbolName: ToolbarSymbols.more(), accessibilityDescription: label) + item.showsIndicator = ToolbarSymbols.moreShowsIndicator() item.toolTip = String(localized: "Commands for the current tab and connection") item.isEnabledProvider = enablement(of: Self.actions) item.menu = menu(delegate: actionsMenuDelegate) @@ -217,14 +220,20 @@ extension MainWindowToolbar { /// Labelled with the verb the tab commits with, and re-labelled by `refreshCommitVerb(for:)` when /// the tab kind moves, so the palette, the overflow entry and the tooltip never offer to save a /// table definition that is about to be created. + /// + /// The overflow entry carries no image. A check drawn in a menu row is the mark the system + /// gives an item that is on, so the glyph that reads as "commit" in the toolbar would read as + /// "already saved" in the overflow list. func makeSaveChangesItem() -> NSToolbarItem { - menuOnlyItem( + let item = menuOnlyItem( id: Self.saveChanges, label: commitVerb, - symbol: "checkmark.circle.fill", + symbol: ToolbarSymbols.commit, action: #selector(performSaveChanges(_:)), shortcut: .saveChanges ) + item.menuFormRepresentation?.image = nil + return item } /// A row insert is a change to the data, so it belongs with the other data commands rather than diff --git a/TablePro/Core/Services/Infrastructure/Toolbar/ToolbarSymbols.swift b/TablePro/Core/Services/Infrastructure/Toolbar/ToolbarSymbols.swift new file mode 100644 index 0000000000..f12e17e0cb --- /dev/null +++ b/TablePro/Core/Services/Infrastructure/Toolbar/ToolbarSymbols.swift @@ -0,0 +1,48 @@ +// +// ToolbarSymbols.swift +// TablePro +// + +import Foundation + +/// The glyphs a window toolbar draws for the actions every toolbar shares. +/// +/// From macOS 26 a toolbar item sits in its own glass container, and the HIG asks for symbols +/// without borders there: the container is the border, so a circled glyph draws two. Earlier +/// releases give an item no container, and the circled form is the one their own toolbars use. +/// Each answer is a function of that one fact, so both forms can be tested on either release. +internal enum ToolbarSymbols { + internal static let commit = "checkmark" + + internal static var itemsHaveContainer: Bool { + if #available(macOS 26.0, *) { + return true + } + return false + } + + internal static func more(itemsHaveContainer: Bool = ToolbarSymbols.itemsHaveContainer) -> String { + itemsHaveContainer ? "ellipsis" : "ellipsis.circle" + } + + /// A More control inside a container draws no menu indicator, measured on Finder and Notes. + /// Every other pull-down keeps the system's. + internal static func moreShowsIndicator(itemsHaveContainer: Bool = ToolbarSymbols.itemsHaveContainer) -> Bool { + !itemsHaveContainer + } + + /// The filled disc is state, a filter narrowing the list, so it stays where the idle border goes. + internal static func filter( + isActive: Bool = false, + itemsHaveContainer: Bool = ToolbarSymbols.itemsHaveContainer + ) -> String { + if isActive { + return "line.3.horizontal.decrease.circle.fill" + } + return itemsHaveContainer ? "line.3.horizontal.decrease" : "line.3.horizontal.decrease.circle" + } + + internal static func disconnect(itemsHaveContainer: Bool = ToolbarSymbols.itemsHaveContainer) -> String { + itemsHaveContainer ? "xmark" : "xmark.circle" + } +} diff --git a/TablePro/Models/Connection/SafeModeLevel.swift b/TablePro/Models/Connection/SafeModeLevel.swift index 6648f0a949..ca9ca44714 100644 --- a/TablePro/Models/Connection/SafeModeLevel.swift +++ b/TablePro/Models/Connection/SafeModeLevel.swift @@ -81,14 +81,16 @@ internal extension SafeModeLevel { } } + /// Filled exactly when `appliesToAllQueries`, so the fill says one thing: this level gates + /// reads as well as writes. Silent and Read-Only differ by the padlock's shape instead. var iconName: String { switch self { - case .silent: return "lock.open.fill" + case .silent: return "lock.open" case .alert: return "exclamationmark.triangle" case .alertFull: return "exclamationmark.triangle.fill" case .safeMode: return "lock.shield" case .safeModeFull: return "lock.shield.fill" - case .readOnly: return "lock.fill" + case .readOnly: return "lock" } } diff --git a/TablePro/Models/Database/DatabaseMetadata.swift b/TablePro/Models/Database/DatabaseMetadata.swift index f658df6de9..6e7c4e750a 100644 --- a/TablePro/Models/Database/DatabaseMetadata.swift +++ b/TablePro/Models/Database/DatabaseMetadata.swift @@ -39,7 +39,7 @@ struct DatabaseMetadata: Identifiable, Equatable { sizeBytes: nil, lastAccessed: nil, isSystemDatabase: isSystem, - icon: isSystem ? "gearshape.fill" : "cylinder.fill" + icon: isSystem ? "gearshape" : "cylinder" ) } } diff --git a/TablePro/Models/Query/ResultStatusPresentation.swift b/TablePro/Models/Query/ResultStatusPresentation.swift index c702ed0ca3..54c3832824 100644 --- a/TablePro/Models/Query/ResultStatusPresentation.swift +++ b/TablePro/Models/Query/ResultStatusPresentation.swift @@ -16,7 +16,7 @@ import CoreGraphics /// tier whose ideal width fits, and `narrow` is measured to fit the narrowest pane the window /// allows, so no tier can over-size the column again. enum StatusBarTier: CaseIterable { - /// Every control at full width, titles beside icons. + /// Every control at full width, titles beside icons. Highlight Rules alone stays a glyph. case regular /// Titles drop to their icons and the page edges go. Every control is still on the bar. case compact @@ -37,7 +37,7 @@ enum StatusBarTier: CaseIterable { /// which controls a result offers, this one answers how they are drawn. Both are pure so the whole /// matrix is decidable without mounting a view. struct ResultStatusPresentation: Equatable { - /// Whether a control carries its title beside its icon. + /// Whether Columns and Filters carry their title beside their icon. let showsControlTitles: Bool /// A segmented control while the modes fit, and a pull-down naming the current one once they do /// not. `View > Result View` offers the same choice either way. diff --git a/TablePro/Views/DataFiles/DataFileWindowController.swift b/TablePro/Views/DataFiles/DataFileWindowController.swift index 9562765bfd..53cb57bc5a 100644 --- a/TablePro/Views/DataFiles/DataFileWindowController.swift +++ b/TablePro/Views/DataFiles/DataFileWindowController.swift @@ -105,7 +105,7 @@ final class DataFileWindowController: NSWindowController, NSWindowDelegate, NSTo return makeItem(itemIdentifier, label: String(localized: "Delete"), symbol: "minus", action: #selector(DataFileSplitViewController.dataFileDeleteSelectedRows(_:))) case .dataFileFilters: - return makeItem(itemIdentifier, label: String(localized: "Filters"), symbol: "line.3.horizontal.decrease.circle", + return makeItem(itemIdentifier, label: String(localized: "Filters"), symbol: ToolbarSymbols.filter(), action: #selector(DataFileSplitViewController.toggleFilterBar(_:))) case .dataFileInspector: return makeItem(itemIdentifier, label: String(localized: "Inspector"), symbol: "sidebar.trailing", diff --git a/TablePro/Views/Integrations/IntegrationsActivityLogPane.swift b/TablePro/Views/Integrations/IntegrationsActivityLogPane.swift index 0ec021725e..507fddf536 100644 --- a/TablePro/Views/Integrations/IntegrationsActivityLogPane.swift +++ b/TablePro/Views/Integrations/IntegrationsActivityLogPane.swift @@ -123,17 +123,14 @@ struct IntegrationsActivityLogPane: View { } } } label: { - Label(String(localized: "Filters"), systemImage: filterIcon) + Label(String(localized: "Filters"), systemImage: ToolbarSymbols.filter(isActive: hasActiveFilters)) } + /// Named here for the reason `WelcomeViewOptionsMenu` gives: a toolbar-hosted `Menu` + /// publishes "chevron.pulldown" from its label alone. + .accessibilityLabel(String(localized: "Filters")) .help(String(localized: "Filter activity")) } - private var filterIcon: String { - hasActiveFilters - ? "line.3.horizontal.decrease.circle.fill" - : "line.3.horizontal.decrease.circle" - } - private var exportButton: some View { Button(action: exportCSV) { Label(String(localized: "Export"), systemImage: "square.and.arrow.up") diff --git a/TablePro/Views/Integrations/IntegrationsConnectedClientsPane.swift b/TablePro/Views/Integrations/IntegrationsConnectedClientsPane.swift index f0504e22d1..be772fe6a0 100644 --- a/TablePro/Views/Integrations/IntegrationsConnectedClientsPane.swift +++ b/TablePro/Views/Integrations/IntegrationsConnectedClientsPane.swift @@ -55,7 +55,7 @@ struct IntegrationsConnectedClientsPane: View { disconnectCandidate = client } } label: { - Label(String(localized: "Disconnect"), systemImage: "xmark.circle") + Label(String(localized: "Disconnect"), systemImage: ToolbarSymbols.disconnect()) } .help(String(localized: "Disconnect the selected client")) .disabled(selection == nil) diff --git a/TablePro/Views/Results/ResultStatusBar.swift b/TablePro/Views/Results/ResultStatusBar.swift index 1dbe6cc7c9..6c79fdbf46 100644 --- a/TablePro/Views/Results/ResultStatusBar.swift +++ b/TablePro/Views/Results/ResultStatusBar.swift @@ -314,6 +314,7 @@ struct ResultStatusBar: View { Text("Columns") } icon: { Image(systemName: hasHiddenColumns ? "eye.slash" : "eye") + .statusBarControlIcon(besideTitle: presentation.showsControlTitles) } } .statusBarLabelStyle(showsTitle: presentation.showsControlTitles) @@ -351,8 +352,12 @@ struct ResultStatusBar: View { Text("Highlight Rules") } icon: { Image(systemName: "highlighter") + .statusBarControlIcon(besideTitle: false) } } + /// A glyph at every tier. Its title would add 88pt, and measured at the window's default + /// 1200pt with the sidebar open the regular tier then no longer fits, so drawing it would + /// take the titles off Columns and Filters at the size most windows are. .labelStyle(.iconOnly) .controlSize(.small) .disabled(highlightState.columns.isEmpty) @@ -388,6 +393,7 @@ struct ResultStatusBar: View { Image(systemName: filterState.hasAppliedFilters ? "line.3.horizontal.decrease.circle.fill" : "line.3.horizontal.decrease.circle") + .statusBarControlIcon(besideTitle: presentation.showsControlTitles) } } .statusBarLabelStyle(showsTitle: presentation.showsControlTitles) diff --git a/TablePro/Views/Results/StatusBarChrome.swift b/TablePro/Views/Results/StatusBarChrome.swift index 4114366945..1e3dd824ab 100644 --- a/TablePro/Views/Results/StatusBarChrome.swift +++ b/TablePro/Views/Results/StatusBarChrome.swift @@ -17,6 +17,15 @@ enum StatusBarChrome { static let height: CGFloat = 28 static let horizontalPadding: CGFloat = 10 static let clusterSpacing: CGFloat = 8 + + /// The layout height a control's glyph is given. A small bordered button takes its height from + /// its glyph, measured at 18pt for `eye`, 19pt for the circled filter and 20pt for `eye.slash` + /// when the glyph stands alone, and 20 or 21pt beside a title, so the controls in one bar stood + /// at three heights. One box makes each the height of a text-only control: 12pt beside a title, + /// whose own line sets the height, and 14pt alone. + static func controlIconHeight(besideTitle: Bool) -> CGFloat { + besideTitle ? 12 : 14 + } } struct StatusBarSeparator: View { @@ -60,4 +69,9 @@ extension View { func statusBarChrome() -> some View { modifier(StatusBarChromeModifier()) } + + /// Height only: the glyph keeps its size and its width, so no control gets wider or narrower. + func statusBarControlIcon(besideTitle: Bool) -> some View { + frame(height: StatusBarChrome.controlIconHeight(besideTitle: besideTitle)) + } } diff --git a/TablePro/Views/Settings/SettingsView.swift b/TablePro/Views/Settings/SettingsView.swift index f8c8f9b90d..c926e88299 100644 --- a/TablePro/Views/Settings/SettingsView.swift +++ b/TablePro/Views/Settings/SettingsView.swift @@ -40,7 +40,7 @@ enum SettingsPane: String, CaseIterable { case .mcp: "network" case .plugins: "puzzlepiece.extension" case .sync: "arrow.triangle.2.circlepath" - case .account: "key.fill" + case .account: "key" } } } diff --git a/TablePro/Views/Shared/ObjectSourceView.swift b/TablePro/Views/Shared/ObjectSourceView.swift index 8f42b1f744..4ee62cf894 100644 --- a/TablePro/Views/Shared/ObjectSourceView.swift +++ b/TablePro/Views/Shared/ObjectSourceView.swift @@ -64,7 +64,7 @@ struct ObjectSourceView: View { Button { Task { await export() } } label: { - Label("Export…", systemImage: "square.and.arrow.down") + Label("Export…", systemImage: "square.and.arrow.up") } .buttonStyle(.bordered) .disabled(!hasSource) diff --git a/TablePro/Views/Structure/TableStructureView+Schema.swift b/TablePro/Views/Structure/TableStructureView+Schema.swift index 048abcef48..b8e22088e3 100644 --- a/TablePro/Views/Structure/TableStructureView+Schema.swift +++ b/TablePro/Views/Structure/TableStructureView+Schema.swift @@ -112,7 +112,7 @@ extension TableStructureView { .buttonStyle(.bordered) Button(action: exportDDL) { - Label("Export", systemImage: "square.and.arrow.down") + Label("Export", systemImage: "square.and.arrow.up") } .buttonStyle(.bordered) } diff --git a/TablePro/Views/Welcome/WelcomeLibraryPane.swift b/TablePro/Views/Welcome/WelcomeLibraryPane.swift index 2697940173..ce76005a01 100644 --- a/TablePro/Views/Welcome/WelcomeLibraryPane.swift +++ b/TablePro/Views/Welcome/WelcomeLibraryPane.swift @@ -144,8 +144,12 @@ internal struct WelcomeViewOptionsMenu: View { } } } label: { - Label(String(localized: "View Options"), systemImage: "line.3.horizontal.decrease.circle") + Label(String(localized: "View Options"), systemImage: ToolbarSymbols.filter()) } + /// A toolbar-hosted `Menu` is named by this modifier and not by its label: built against + /// the macOS 26 SDK, the label alone publishes "chevron.pulldown". An in-view `Menu` is the + /// reverse, which is the rule `MenuDisclosureIndicatorTests` holds everywhere else. + .accessibilityLabel(String(localized: "View Options")) .help(String(localized: "Sort and filter connections")) .accessibilityIdentifier("welcome-toolbar-view-options") } diff --git a/TableProTests/Core/Plugins/PluginDriverAdapterSystemDatabaseTests.swift b/TableProTests/Core/Plugins/PluginDriverAdapterSystemDatabaseTests.swift index fe00511eaf..0004159228 100644 --- a/TableProTests/Core/Plugins/PluginDriverAdapterSystemDatabaseTests.swift +++ b/TableProTests/Core/Plugins/PluginDriverAdapterSystemDatabaseTests.swift @@ -68,8 +68,8 @@ struct PluginDriverAdapterSystemDatabaseTests { let result = try await adapter.fetchAllDatabaseMetadata() #expect(result.filter(\.isSystemDatabase).map(\.name) == ["master", "tempdb"]) - #expect(result.first { $0.name == "master" }?.icon == "gearshape.fill") - #expect(result.first { $0.name == "sales" }?.icon == "cylinder.fill") + #expect(result.first { $0.name == "master" }?.icon == "gearshape") + #expect(result.first { $0.name == "sales" }?.icon == "cylinder") } @Test("A database the driver flags stays a system database even when the type's list omits it") diff --git a/TableProTests/Core/Services/Infrastructure/ToolbarSymbolConventionTests.swift b/TableProTests/Core/Services/Infrastructure/ToolbarSymbolConventionTests.swift new file mode 100644 index 0000000000..4e286def91 --- /dev/null +++ b/TableProTests/Core/Services/Infrastructure/ToolbarSymbolConventionTests.swift @@ -0,0 +1,119 @@ +// +// ToolbarSymbolConventionTests.swift +// TableProTests +// +// A window toolbar draws outline glyphs. A fill is the HIG's variant for selection, so a filled +// glyph on a command reads as state that is not there, and from macOS 26 a circled one draws a +// border inside the border its item already has. Four shipped that way: Save was the filled, +// circled check the rest of the app uses for "succeeded", Actions and the data file's Filters +// were circled, and the License pane was the one filled glyph of twelve in Settings. +// +// A glyph whose form depends on the release comes from `ToolbarSymbols`, which is why a literal +// with a `circle` or `fill` component has no business in these files at all. The SwiftUI toolbars +// (welcome, Integrations) take theirs from the same owner but are not scanned: their files also +// draw content, where a circled or filled status glyph is the right one. +// + +import Foundation +import Testing + +struct ToolbarSymbolConventionTests { + private static let repositoryRoot: URL = { + var url = URL(fileURLWithPath: #filePath) + for _ in 0 ..< 5 { + url.deleteLastPathComponent() + } + return url + }() + + /// The Settings panes are toolbar items too, built from the tab view rather than by hand, so + /// the file never names `NSToolbarItem` and has to be listed. + private static let additionalFiles = ["TablePro/Views/Settings/SettingsView.swift"] + + private static let bannedComponents: Set = ["circle", "fill"] + + /// Comments are dropped before the scan, because a comment may name what was removed. + private static func code(of source: String) -> String { + source + .split(separator: "\n", omittingEmptySubsequences: false) + .filter { !$0.trimmingCharacters(in: .whitespaces).hasPrefix("//") } + .joined(separator: "\n") + } + + /// Every string literal shaped like a symbol name: lowercase words and digits joined by dots. + private static func symbolLiterals(in source: String) -> [String] { + guard let pattern = try? NSRegularExpression(pattern: "\"([a-z0-9]+(?:\\.[a-z0-9]+)+)\"") else { return [] } + let text = code(of: source) as NSString + return pattern + .matches(in: text as String, range: NSRange(location: 0, length: text.length)) + .map { text.substring(with: $0.range(at: 1)) } + } + + /// By component, not by substring: `arrow.triangle.2.circlepath` and `rectangle.inset.filled` + /// are outline glyphs whose names happen to contain the letters. + private static func isEnclosedOrFilled(_ name: String) -> Bool { + name.split(separator: ".").contains { bannedComponents.contains($0) } + } + + private static func buildsToolbarItems(_ source: String) -> Bool { + source.contains("NSToolbarItem") || source.contains("NSMenuToolbarItem") + } + + @Test("No file that builds toolbar items names a circled or filled symbol") + func toolbarFilesNameNoEnclosedOrFilledSymbol() throws { + let appRoot = Self.repositoryRoot.appendingPathComponent("TablePro") + let enumerator = try #require(FileManager.default.enumerator(at: appRoot, includingPropertiesForKeys: nil)) + + var sources: [(path: String, source: String)] = [] + for case let url as URL in enumerator where url.pathExtension == "swift" { + let source = try String(contentsOf: url, encoding: .utf8) + guard Self.buildsToolbarItems(source) else { continue } + sources.append((url.path.replacingOccurrences(of: Self.repositoryRoot.path + "/", with: ""), source)) + } + for path in Self.additionalFiles { + let url = Self.repositoryRoot.appendingPathComponent(path) + sources.append((path, try String(contentsOf: url, encoding: .utf8))) + } + + var inspected = 0 + var offenders: [String] = [] + for (path, source) in sources { + let literals = Self.symbolLiterals(in: source) + inspected += literals.count + offenders += literals.filter(Self.isEnclosedOrFilled).map { "\(path): \($0)" } + } + + /// Guards the scan itself: a root that stopped resolving reads as a clean run. + #expect(sources.count > 8, "Expected to find the toolbar sources, found \(sources.count)") + #expect(inspected > 15, "Expected to find symbol names to check, found \(inspected)") + #expect( + offenders.isEmpty, + "A toolbar glyph is outline. Take one whose form follows the release from ToolbarSymbols: \(offenders.sorted())" + ) + } + + /// A scan that stops matching anything passes forever. This pins both halves. + @Test("The scan catches a circled or filled name and leaves an outline one alone") + func scanTellsAVariantFromALookalike() { + let flagged = [ + " symbol: \"checkmark.circle.fill\",", + " item.image = NSImage(systemSymbolName: \"ellipsis.circle\", accessibilityDescription: label)", + " case .account: \"key.fill\"", + " symbolProvider: { \"line.3.horizontal.decrease.circle\" }", + ] + for line in flagged { + #expect(Self.symbolLiterals(in: line).contains(where: Self.isEnclosedOrFilled), "\(line)") + } + + let allowed = [ + " symbol: \"clock.arrow.circlepath\",", + " symbol: \"arrow.triangle.2.circlepath\",", + " ? \"rectangle.inset.filled\"", + " symbol: \"square.and.arrow.down.on.square\",", + " /// It used to draw \"checkmark.circle.fill\".", + ] + for line in allowed { + #expect(Self.symbolLiterals(in: line).contains(where: Self.isEnclosedOrFilled) == false, "\(line)") + } + } +} diff --git a/TableProTests/Core/Services/Infrastructure/ToolbarSymbolsTests.swift b/TableProTests/Core/Services/Infrastructure/ToolbarSymbolsTests.swift new file mode 100644 index 0000000000..2083d57877 --- /dev/null +++ b/TableProTests/Core/Services/Infrastructure/ToolbarSymbolsTests.swift @@ -0,0 +1,70 @@ +// +// ToolbarSymbolsTests.swift +// TableProTests +// + +import AppKit +@testable import TablePro +import Testing + +/// Both forms of each glyph are asserted here because a test run only ever takes one of them: CI +/// and every developer machine are on a release where a toolbar item has a container. +struct ToolbarSymbolsTests { + @Test("An item in a container draws the glyph without its circle") + func containedItemsDropTheCircle() { + #expect(ToolbarSymbols.more(itemsHaveContainer: true) == "ellipsis") + #expect(ToolbarSymbols.filter(isActive: false, itemsHaveContainer: true) == "line.3.horizontal.decrease") + #expect(ToolbarSymbols.disconnect(itemsHaveContainer: true) == "xmark") + } + + @Test("An item with no container keeps the circle") + func bareItemsKeepTheCircle() { + #expect(ToolbarSymbols.more(itemsHaveContainer: false) == "ellipsis.circle") + #expect( + ToolbarSymbols.filter(isActive: false, itemsHaveContainer: false) == "line.3.horizontal.decrease.circle" + ) + #expect(ToolbarSymbols.disconnect(itemsHaveContainer: false) == "xmark.circle") + } + + /// There is no filled form of the bare glyph, so the disc is the only way to say a filter is on. + @Test("An active filter is the filled disc on both", arguments: [true, false]) + func activeFilterIsFilled(itemsHaveContainer: Bool) { + #expect( + ToolbarSymbols.filter(isActive: true, itemsHaveContainer: itemsHaveContainer) + == "line.3.horizontal.decrease.circle.fill" + ) + } + + @Test("Only a More control without a container draws the menu indicator") + func moreIndicatorFollowsTheContainer() { + #expect(!ToolbarSymbols.moreShowsIndicator(itemsHaveContainer: true)) + #expect(ToolbarSymbols.moreShowsIndicator(itemsHaveContainer: false)) + } + + @Test("Commit is the bare check") + func commitIsTheBareCheck() { + #expect(ToolbarSymbols.commit == "checkmark") + } + + /// A name that is not a system symbol draws nothing: the toolbar item keeps an empty image and + /// a SwiftUI label lays its icon out at zero size. + @Test("Every name it can return is a system symbol") + func everyNameResolves() { + var names: Set = [ToolbarSymbols.commit] + for itemsHaveContainer in [true, false] { + names.insert(ToolbarSymbols.more(itemsHaveContainer: itemsHaveContainer)) + names.insert(ToolbarSymbols.disconnect(itemsHaveContainer: itemsHaveContainer)) + for isActive in [true, false] { + names.insert(ToolbarSymbols.filter(isActive: isActive, itemsHaveContainer: itemsHaveContainer)) + } + } + + #expect(names.count == 8) + for name in names { + #expect( + NSImage(systemSymbolName: name, accessibilityDescription: nil) != nil, + "\(name) is not a system symbol" + ) + } + } +} diff --git a/TableProTests/Models/SafeModeLevelTests.swift b/TableProTests/Models/SafeModeLevelTests.swift index 5e5f9871ce..fcff3a76bb 100644 --- a/TableProTests/Models/SafeModeLevelTests.swift +++ b/TableProTests/Models/SafeModeLevelTests.swift @@ -3,6 +3,7 @@ // TableProTests // +import AppKit import SwiftUI import TableProPluginKit import Testing @@ -122,12 +123,33 @@ struct SafeModeLevelTests { @Test("each case has the correct SF Symbol icon name") func iconNames() { - #expect(SafeModeLevel.silent.iconName == "lock.open.fill") + #expect(SafeModeLevel.silent.iconName == "lock.open") #expect(SafeModeLevel.alert.iconName == "exclamationmark.triangle") #expect(SafeModeLevel.alertFull.iconName == "exclamationmark.triangle.fill") #expect(SafeModeLevel.safeMode.iconName == "lock.shield") #expect(SafeModeLevel.safeModeFull.iconName == "lock.shield.fill") - #expect(SafeModeLevel.readOnly.iconName == "lock.fill") + #expect(SafeModeLevel.readOnly.iconName == "lock") + } + + /// The fill used to mark two of the four levels that gate reads and both of the two that do + /// not, so it said nothing. The weakest level drew the heaviest glyph in the toolbar. + @Test("An icon is filled exactly when its level applies to all queries", arguments: SafeModeLevel.allCases) + func fillMeansTheLevelAppliesToAllQueries(level: SafeModeLevel) { + #expect(level.iconName.hasSuffix(".fill") == level.appliesToAllQueries) + } + + /// The toolbar glyph is re-read only when the name changes, so two levels sharing one would + /// leave the previous level's glyph in place. + @Test("Every level draws its own symbol, and each one exists") + func iconNamesAreDistinctAndResolve() { + let names = SafeModeLevel.allCases.map(\.iconName) + #expect(Set(names).count == names.count) + for name in names { + #expect( + NSImage(systemSymbolName: name, accessibilityDescription: nil) != nil, + "\(name) is not a system symbol" + ) + } } // MARK: - badgeColor diff --git a/TableProTests/Services/MainWindowToolbarNativeContractTests.swift b/TableProTests/Services/MainWindowToolbarNativeContractTests.swift index 559f07282d..a17ce9a7f7 100644 --- a/TableProTests/Services/MainWindowToolbarNativeContractTests.swift +++ b/TableProTests/Services/MainWindowToolbarNativeContractTests.swift @@ -209,6 +209,47 @@ struct MainWindowToolbarNativeContractTests { #expect(!item.label.isEmpty) } + /// Finder's own Action pull-down, which is a bare ellipsis with no indicator where the item + /// has a glass container and a circled one with a chevron where it has none. `ToolbarSymbols` + /// owns both answers, so the glyph and the indicator cannot be taken from different releases. + @Test("The Actions item draws its indicator only where the More glyph is circled") + func actionsIndicatorFollowsTheGlyph() throws { + let owner = MainWindowToolbar() + let item = try #require( + owner.toolbar( + owner.managedToolbar, + itemForItemIdentifier: MainWindowToolbar.actions, + willBeInsertedIntoToolbar: true + ) as? NSMenuToolbarItem + ) + + #expect(item.image != nil) + #expect(item.showsIndicator == ToolbarSymbols.moreShowsIndicator()) + } + + /// Save drew the filled, circled check the rest of the app uses for "succeeded", the one + /// filled glyph in a toolbar of outline ones. The bare check is the HIG's glyph for Save, and + /// it stays out of the overflow entry: a check in a menu row is the mark of an item that is on. + @Test("The commit control draws the bare check, and its overflow entry draws no image") + func commitGlyphStaysOutOfTheOverflowEntry() throws { + let owner = MainWindowToolbar() + let item = try #require( + owner.toolbar( + owner.managedToolbar, + itemForItemIdentifier: MainWindowToolbar.saveChanges, + willBeInsertedIntoToolbar: true + ) as? StatefulToolbarItem + ) + let provider = try #require(item.symbolProvider) + + #expect(provider() == ToolbarSymbols.commit) + #expect(item.image != nil) + let overflow = try #require(item.menuFormRepresentation) + #expect(overflow.image == nil) + #expect(overflow.title == item.label) + #expect(overflow.action == item.action) + } + /// The commit control says what its tab commits. The palette, the overflow entry and the /// tooltip all read the label, so a Create Table tab offering to Save Changes is the defect. /// diff --git a/TableProTests/Views/Main/StatusBarControlIconTests.swift b/TableProTests/Views/Main/StatusBarControlIconTests.swift new file mode 100644 index 0000000000..eba51fb816 --- /dev/null +++ b/TableProTests/Views/Main/StatusBarControlIconTests.swift @@ -0,0 +1,55 @@ +// +// StatusBarControlIconTests.swift +// TableProTests +// + +import AppKit +import SwiftUI +import Testing + +@testable import TablePro + +/// File scope rather than a member: `@Test(arguments:)` reads its arguments from a nonisolated +/// context, which cannot touch a static on a `@MainActor` suite. +private let statusBarLabelStyles = [true, false] + +/// A small bordered button takes its height from its glyph, so the status bar's controls stood at +/// three heights: measured, `eye` alone made an 18pt button, the circled filter 19pt and +/// `eye.slash` 20pt, and Columns grew a point the moment a column was hidden. +@MainActor +struct StatusBarControlIconTests { + /// Every glyph the result status bar draws in a bordered control, in each of its states. + private static let glyphs = [ + "eye", + "eye.slash", + "highlighter", + "line.3.horizontal.decrease.circle", + "line.3.horizontal.decrease.circle.fill", + ] + + private func controlHeight(glyph: String, showsTitle: Bool) -> CGFloat { + let control = Button {} label: { + Label { + Text(verbatim: "Filters") + } icon: { + Image(systemName: glyph) + .statusBarControlIcon(besideTitle: showsTitle) + } + } + .controlSize(.small) + + let host = showsTitle + ? NSHostingView(rootView: AnyView(control.labelStyle(.titleAndIcon))) + : NSHostingView(rootView: AnyView(control.labelStyle(.iconOnly))) + host.layoutSubtreeIfNeeded() + return host.fittingSize.height + } + + @Test("Every glyph gives its control the same height", arguments: statusBarLabelStyles) + func glyphDoesNotDecideTheControlHeight(showsTitle: Bool) { + let heights = Self.glyphs.map { controlHeight(glyph: $0, showsTitle: showsTitle) } + + #expect(heights.allSatisfy { $0 > 0 }, "Expected every control to lay out, measured \(heights)") + #expect(Set(heights).count == 1, "Measured \(heights) for \(Self.glyphs)") + } +} diff --git a/TableProTests/Views/MenuDisclosureIndicatorTests.swift b/TableProTests/Views/MenuDisclosureIndicatorTests.swift index 7b40582ae6..6b4491c5d6 100644 --- a/TableProTests/Views/MenuDisclosureIndicatorTests.swift +++ b/TableProTests/Views/MenuDisclosureIndicatorTests.swift @@ -30,6 +30,17 @@ struct MenuDisclosureIndicatorTests { return url }() + /// A `Menu` hosted by a toolbar is the reverse of the rule the first test below holds. Measured + /// through the accessibility API on a build linked against the macOS 26 SDK: the label alone + /// publishes `AXTitle` "chevron.pulldown", in every label form that test allows, and + /// `.accessibilityLabel` on the `Menu` is the one construction that names it. Both menus shipped + /// unnamed. Keyed by file, each of which holds a single `Menu`, because a line number moves with + /// every edit above it. + private static let toolbarHostedMenus: Set = [ + "TablePro/Views/Integrations/IntegrationsActivityLogPane.swift", + "TablePro/Views/Welcome/WelcomeLibraryPane.swift", + ] + /// `.accessibilityLabel` on a `Menu` is not additive. Measured with System Events against a /// standalone SwiftUI app: a menu labelled `Text("Add tags")` publishes `AXTitle` "Add tags", /// and the same menu with `.accessibilityLabel(Text("Add tags"))` on it publishes an empty name. @@ -55,6 +66,7 @@ struct MenuDisclosureIndicatorTests { let relativePath = url.path.replacingOccurrences(of: Self.repositoryRoot.path + "/", with: "") let result = Self.scanForMisplacedNames(source, relativePath: relativePath) inspected += result.inspected + guard !Self.toolbarHostedMenus.contains(relativePath) else { continue } offenders += result.offenders } @@ -65,6 +77,21 @@ struct MenuDisclosureIndicatorTests { ) } + @Test("A toolbar-hosted menu names itself with accessibilityLabel") + func toolbarHostedMenusCarryTheirNameOnTheMenu() throws { + for relativePath in Self.toolbarHostedMenus.sorted() { + let url = Self.repositoryRoot.appendingPathComponent(relativePath) + let source = try String(contentsOf: url, encoding: .utf8) + let result = Self.scanForMisplacedNames(source, relativePath: relativePath) + + #expect(result.inspected == 1, "Expected one menu in \(relativePath), found \(result.inspected)") + #expect( + result.offenders.count == 1, + "The toolbar menu in \(relativePath) needs .accessibilityLabel on the Menu itself, or VoiceOver reads it as \"chevron.pulldown\"" + ) + } + } + @Test("A label on the menu itself is flagged, a label on a container wrapping it is not") func scannerTellsTheMenuFromItsContainer() { let namedMenu = """ diff --git a/docs/features/connection-window.mdx b/docs/features/connection-window.mdx index fa137208ad..75740d9d9c 100644 --- a/docs/features/connection-window.mdx +++ b/docs/features/connection-window.mdx @@ -17,7 +17,7 @@ Eight controls, laid out by pane: the sidebar toggle over the sidebar, the conne | Refresh | Reloads what the tab is showing (`Cmd+R`) | | Save Changes | Commits the tab's staged changes (`Cmd+S`) | | Actions | The rest of the commands for this tab and this connection | -| Safe Mode | A padlock carrying the current [level](/features/safe-mode), open for **Silent** and closed for the rest | +| Safe Mode | A padlock carrying the current [level](/features/safe-mode), open for **Silent** and closed for **Read-Only** | | Inspector | Opens and closes the [trailing pane](#the-trailing-pane) (`Cmd+Option+I`) | Two of them change their name. The database control follows the engine's own word: **Schema** on [Oracle](/databases/oracle), **Catalog** on [Trino](/databases/trino), **Keyspace** on [Cassandra](/databases/cassandra). The commit control follows the tab: **Create Table** on a new table's tab, **Apply Changes** on [Users & Roles](/features/users-roles), **Save Changes** everywhere else. diff --git a/docs/features/safe-mode.mdx b/docs/features/safe-mode.mdx index 9a4898534d..a3c45b5f85 100644 --- a/docs/features/safe-mode.mdx +++ b/docs/features/safe-mode.mdx @@ -60,7 +60,7 @@ Where a string, a quoted name or a comment ends depends on the engine. A backsla ## Changing the level while connected -A padlock in the toolbar carries the current level: open for **Silent**, closed for the rest. Click it for the levels this connection may run at, with the current one ticked. **Database > Safe Mode Level** is the same list. +A padlock in the toolbar carries the current level: open for **Silent**, closed for **Read-Only**, a warning triangle for the two Alert levels and a shield for the two Safe Mode levels. The icon is filled at the two **Full** levels, the ones that also ask before a read. Click it for the levels this connection may run at, with the current one ticked. **Database > Safe Mode Level** is the same list. Four things can hold the level above the one you chose: a read-only engine, a [remote database file](/connections/remote-database-files), a [managed policy](#managed-by-an-organization), and [Agent mode](/features/agent-mode). The weaker levels are then left out of both lists, the reason sits under them, and the padlock's tooltip carries the same sentence. Picking the level already in force writes nothing. diff --git a/docs/images/welcome-screen-dark.png b/docs/images/welcome-screen-dark.png index 719b74b7f0..5044c78ce2 100644 Binary files a/docs/images/welcome-screen-dark.png and b/docs/images/welcome-screen-dark.png differ diff --git a/docs/images/welcome-screen.png b/docs/images/welcome-screen.png index 7c054fec14..a59a2998fc 100644 Binary files a/docs/images/welcome-screen.png and b/docs/images/welcome-screen.png differ