diff --git a/CHANGELOG.md b/CHANGELOG.md index 474044a8..8e498b80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 other non-browser) tabs keep their titled layout. ### Fixed +- A tab title change no longer re-renders the whole tab bar. `TabItem` is now a + reference type, so writing one tab's title touches that object instead of the + `@Observable` `PaneState.tabs` array, and observation delivers the change to + the one `TabItemView` that reads the title. Inserting, removing, and moving + tabs still publish through `tabs`. Equality and hashing were already by `id`, + so identity comparisons are unaffected. - Split-button icons no longer re-resolve their SF Symbol on every tab bar body evaluation. `TabBarStyling.splitActionSystemImage(for:)` tested whether a name was a real symbol by allocating an `NSImage` and discarding it, which cost diff --git a/Sources/Bonsplit/Internal/Models/TabItem.swift b/Sources/Bonsplit/Internal/Models/TabItem.swift index be7f011e..fe5a8039 100644 --- a/Sources/Bonsplit/Internal/Models/TabItem.swift +++ b/Sources/Bonsplit/Internal/Models/TabItem.swift @@ -14,7 +14,19 @@ extension UTType { } /// Represents a single tab in a pane's tab bar (internal representation) -struct TabItem: Identifiable, Hashable, Codable { +/// +/// A reference type on purpose. `PaneState.tabs` is an `@Observable` property, +/// so when tabs were values, writing one tab's title wrote the whole array and +/// invalidated every view that had read it: all of `TabBarView`, not just the +/// one tab that changed. As references, a per-tab write touches only this +/// object, and observation delivers it to the `TabItemView` that read the +/// property. `PaneState.tabs` still changes on insert, remove, and move, which +/// is what `TabBarView` actually needs to see. +/// +/// Equality and hashing are by `id`, as they were when this was a struct, so +/// identity comparisons behave the same. +@Observable +final class TabItem: Identifiable, Hashable, Codable { let id: UUID var title: String var hasCustomTitle: Bool @@ -93,7 +105,7 @@ struct TabItem: Identifiable, Hashable, Codable { case presence } - init(from decoder: Decoder) throws { + required init(from decoder: Decoder) throws { let c = try decoder.container(keyedBy: CodingKeys.self) self.id = try c.decode(UUID.self, forKey: .id) self.title = try c.decode(String.self, forKey: .title) diff --git a/Sources/Bonsplit/Public/BonsplitController.swift b/Sources/Bonsplit/Public/BonsplitController.swift index 08452eb0..51ae6e12 100644 --- a/Sources/Bonsplit/Public/BonsplitController.swift +++ b/Sources/Bonsplit/Public/BonsplitController.swift @@ -365,43 +365,43 @@ public final class BonsplitController { guard didChange else { return } if let title = title { - pane.tabs[tabIndex].title = title + currentTab.title = title } if let icon = icon { - pane.tabs[tabIndex].icon = icon + currentTab.icon = icon } if let iconImageData = iconImageData { - pane.tabs[tabIndex].iconImageData = iconImageData + currentTab.iconImageData = iconImageData } if let iconAsset = iconAsset { - pane.tabs[tabIndex].iconAsset = iconAsset + currentTab.iconAsset = iconAsset } if let kind = kind { - pane.tabs[tabIndex].kind = kind + currentTab.kind = kind } if let hasCustomTitle = hasCustomTitle { - pane.tabs[tabIndex].hasCustomTitle = hasCustomTitle + currentTab.hasCustomTitle = hasCustomTitle } if let isDirty = isDirty { - pane.tabs[tabIndex].isDirty = isDirty + currentTab.isDirty = isDirty } if let showsNotificationBadge = showsNotificationBadge { - pane.tabs[tabIndex].showsNotificationBadge = showsNotificationBadge + currentTab.showsNotificationBadge = showsNotificationBadge } if let isLoading = isLoading { - pane.tabs[tabIndex].isLoading = isLoading + currentTab.isLoading = isLoading } if let isAudioMuted = isAudioMuted { - pane.tabs[tabIndex].isAudioMuted = isAudioMuted + currentTab.isAudioMuted = isAudioMuted } if let isAudioPlaying = isAudioPlaying { - pane.tabs[tabIndex].isAudioPlaying = isAudioPlaying + currentTab.isAudioPlaying = isAudioPlaying } if let isPinned = isPinned { - pane.tabs[tabIndex].isPinned = isPinned + currentTab.isPinned = isPinned } if let showsRemoteIndicator = showsRemoteIndicator { - pane.tabs[tabIndex].showsRemoteIndicator = showsRemoteIndicator + currentTab.showsRemoteIndicator = showsRemoteIndicator } if let presence = presence { pane.tabs[tabIndex].presence = presence diff --git a/Tests/BonsplitTests/TabObservationScopeTests.swift b/Tests/BonsplitTests/TabObservationScopeTests.swift new file mode 100644 index 00000000..428ca910 --- /dev/null +++ b/Tests/BonsplitTests/TabObservationScopeTests.swift @@ -0,0 +1,140 @@ +import Observation +import XCTest +@testable import Bonsplit + +/// `PaneState.tabs` is an `@Observable` property. `TabBarView.body` reads it to +/// build its `ForEach`, and `TabItemView.body` reads a single tab's `title`. +/// +/// When `TabItem` was a struct, `tabs[i].title = x` wrote the whole array, so a +/// title change invalidated everything that had read `tabs`: every tab item, the +/// scroll content, and the split button row. A host whose tab titles animate +/// drove that about 20 times a second. +/// +/// These tests pin who gets woken. +final class TabObservationScopeTests: XCTestCase { + @MainActor + func testChangingOneTabTitleDoesNotInvalidateTheTabsArray() throws { + let controller = BonsplitController() + let paneId = try XCTUnwrap(controller.focusedPaneId) + let pane = try XCTUnwrap(controller.internalController.paneState(for: paneId)) + let tabId = try XCTUnwrap(controller.createTab(title: "First")) + + // Stand in for TabBarView.body, which reads the array to lay out tabs. + var tabsInvalidated = false + withObservationTracking { + _ = pane.tabs + } onChange: { + tabsInvalidated = true + } + + controller.updateTab(tabId, title: "First (running)") + + XCTAssertFalse( + tabsInvalidated, + "A title change must not invalidate readers of the tabs array" + ) + } + + @MainActor + func testChangingOneTabTitleInvalidatesThatTab() throws { + let controller = BonsplitController() + let paneId = try XCTUnwrap(controller.focusedPaneId) + let pane = try XCTUnwrap(controller.internalController.paneState(for: paneId)) + let tabId = try XCTUnwrap(controller.createTab(title: "First")) + let tab = try XCTUnwrap(pane.tabs.first { $0.id == tabId.id }) + + // Stand in for TabItemView.body, which renders the title. + var titleInvalidated = false + withObservationTracking { + _ = tab.title + } onChange: { + titleInvalidated = true + } + + controller.updateTab(tabId, title: "First (running)") + + XCTAssertTrue(titleInvalidated, "The tab whose title changed must still redraw") + XCTAssertEqual(tab.title, "First (running)") + } + + @MainActor + func testChangingOneTabTitleDoesNotInvalidateItsSiblings() throws { + let controller = BonsplitController() + let paneId = try XCTUnwrap(controller.focusedPaneId) + let pane = try XCTUnwrap(controller.internalController.paneState(for: paneId)) + let firstId = try XCTUnwrap(controller.createTab(title: "First")) + let secondId = try XCTUnwrap(controller.createTab(title: "Second")) + let second = try XCTUnwrap(pane.tabs.first { $0.id == secondId.id }) + + var siblingInvalidated = false + withObservationTracking { + _ = second.title + } onChange: { + siblingInvalidated = true + } + + controller.updateTab(firstId, title: "First (running)") + + XCTAssertFalse(siblingInvalidated, "A sibling tab must not redraw for another tab's title") + } + + /// The array must still publish the changes it is the right home for. + @MainActor + func testAddingATabStillInvalidatesTheTabsArray() throws { + let controller = BonsplitController() + let paneId = try XCTUnwrap(controller.focusedPaneId) + let pane = try XCTUnwrap(controller.internalController.paneState(for: paneId)) + + var tabsInvalidated = false + withObservationTracking { + _ = pane.tabs + } onChange: { + tabsInvalidated = true + } + + _ = controller.createTab(title: "Second") + + XCTAssertTrue(tabsInvalidated, "Adding a tab changes the layout and must invalidate the array") + } + + @MainActor + func testRemovingATabStillInvalidatesTheTabsArray() throws { + let controller = BonsplitController() + let paneId = try XCTUnwrap(controller.focusedPaneId) + let pane = try XCTUnwrap(controller.internalController.paneState(for: paneId)) + let tabId = try XCTUnwrap(controller.createTab(title: "Second")) + + var tabsInvalidated = false + withObservationTracking { + _ = pane.tabs + } onChange: { + tabsInvalidated = true + } + + _ = controller.closeTab(tabId) + + XCTAssertTrue(tabsInvalidated, "Removing a tab changes the layout and must invalidate the array") + } + + /// A no-op write stays a no-op. `updateTab` guards on `didChange`, and that + /// guard is what keeps a terminal re-emitting an unchanged title free. + @MainActor + func testWritingTheSameTitleWakesNobody() throws { + let controller = BonsplitController() + let paneId = try XCTUnwrap(controller.focusedPaneId) + let pane = try XCTUnwrap(controller.internalController.paneState(for: paneId)) + let tabId = try XCTUnwrap(controller.createTab(title: "First")) + let tab = try XCTUnwrap(pane.tabs.first { $0.id == tabId.id }) + + var titleInvalidated = false + withObservationTracking { + _ = tab.title + } onChange: { + titleInvalidated = true + } + + controller.updateTab(tabId, title: "First") + + XCTAssertFalse(titleInvalidated) + } +}