Skip to content

Commit dcf4acf

Browse files
author
yan
committed
fix(ui): three defects found in visual review
Every tab showed the same file. SourceEditor pushes text into its text view exactly once, in makeNSViewController; updateNSViewController diffs language, configuration and highlight providers, never the text behind the Binding<String>. Switching tabs therefore kept the previous document on screen. DocumentTextCoordinator captures the controller in prepareCoordinator — the one hook that hands it out — and CodeEditorView reloads it when the document changes. Verified in the running app: switching tabs now changes the editor from README content to index.html content. A folder's chevron stopped working once the folder was selected. Expansion hangs off the selection change, and clicking an already selected row changes nothing, so no toggle fired. The chevron is a real Button again; it selects the row first so the two paths cannot cancel. The line-number gutter bled onto the tab labels while rubber-band scrolling. The strip sits flush against the editor and a translucent material lets an overscroll NSScrollView draw through it. Both bars now carry an opaque canvas base under the material. Also: the tab label was rendering both a dirty dot and the " *" that Document.title appends. One state, one indicator.
1 parent 1cb974a commit dcf4acf

5 files changed

Lines changed: 70 additions & 14 deletions

File tree

‎Sources/CodeEditorUI/Sources/Chrome/StatusBarView.swift‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,13 @@ public struct StatusBarView: View {
4747
}
4848
}
4949

50+
/// Opaque base first, then material: the status bar also sits flush against a scroll view,
51+
/// and a translucent fill lets overscrolled content bleed through. See `TabBarView.stripFill`.
5052
@ViewBuilder
5153
private var backgroundFill: some View {
54+
Rectangle().fill(appState.theme.background.color)
5255
if appState.theme.chrome.usesMaterials {
5356
Rectangle().fill(.thinMaterial)
54-
} else {
55-
Rectangle().fill(appState.theme.chrome.barBackground.color)
5657
}
5758
}
5859

‎Sources/CodeEditorUI/Sources/Chrome/TabBarView.swift‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,18 @@ public struct TabBarView: View {
6363

6464
// MARK: - Strip
6565

66-
/// Materials when the palette allows them; the token fill otherwise.
66+
/// An opaque canvas-coloured base under the material.
67+
///
68+
/// The strip sits directly above the editor with no separator, and an `NSScrollView`
69+
/// draws its content outside its bounds while rubber-band scrolling. A translucent
70+
/// material lets that overscrolled content — including the gutter — show through and
71+
/// appear to overlap the tab labels. The base kills the bleed-through while keeping the
72+
/// material's tint and the window-focus response.
6773
@ViewBuilder
6874
private var stripFill: some View {
75+
Rectangle().fill(theme.background.color)
6976
if theme.chrome.usesMaterials {
7077
Rectangle().fill(.thinMaterial)
71-
} else {
72-
Rectangle().fill(theme.chrome.barBackground.color)
7378
}
7479
}
7580

@@ -105,7 +110,9 @@ public struct TabBarView: View {
105110
.help("Unsaved changes")
106111
}
107112

108-
Text(document.title)
113+
// `name`, not `title`: title appends " *" when modified, and the dot
114+
// beside the label already says that. Two indicators for one state.
115+
Text(document.name)
109116
.font(Typography.tabLabel)
110117
.lineLimit(1)
111118
.truncationMode(.middle)

‎Sources/CodeEditorUI/Sources/Editor/CodeEditorView.swift‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ public struct CodeEditorView: View {
1313
let editorState: EditorState
1414

1515
@State private var sourceEditorState = SourceEditorState()
16+
@State private var textCoordinator = DocumentTextCoordinator()
1617

1718
public init(document: CodeEditorCore.Document, theme: Theme, editorState: EditorState) {
1819
self.document = document
@@ -26,13 +27,18 @@ public struct CodeEditorView: View {
2627
language: document.language.codeLanguage,
2728
configuration: configuration,
2829
state: $sourceEditorState,
29-
undoManager: undoManager
30+
undoManager: undoManager,
31+
coordinators: [textCoordinator]
3032
)
3133
.background(theme.background.color)
3234
.onChange(of: sourceEditorState.cursorPositions) { _, positions in
3335
syncSelection(with: positions)
3436
}
3537
.onChange(of: document.id) { _, _ in
38+
// SourceEditor does not diff text on update — only language, configuration
39+
// and highlight providers. Without this, switching tabs keeps the previous
40+
// document on screen.
41+
textCoordinator.reload(text: document.content)
3642
editorState.syncSelection(of: document)
3743
}
3844
.task {
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import CodeEditSourceEditor
2+
3+
/// Keeps the text view's contents in step with the document it is showing.
4+
///
5+
/// `SourceEditor` pushes text into its text view exactly once, in
6+
/// `makeNSViewController`. `updateNSViewController` diffs language, configuration and
7+
/// highlight providers — **it never diffs the text behind the `Binding<String>`**. Switching
8+
/// tabs therefore leaves the previously opened document on screen, and every tab shows the
9+
/// same file.
10+
///
11+
/// `prepareCoordinator` is the one hook that hands out the live controller, and it runs once
12+
/// per controller, so this holds it weakly and `CodeEditorView` calls `reload(text:)` when
13+
/// the document changes.
14+
///
15+
/// This is a workaround for upstream, not a design preference. If a future version diffs the
16+
/// text on update, this type should go away — see [MEMORY.md](../../../docs/MEMORY.md).
17+
@MainActor
18+
final class DocumentTextCoordinator: TextViewCoordinator {
19+
private weak var controller: TextViewController?
20+
21+
func prepareCoordinator(controller: TextViewController) {
22+
self.controller = controller
23+
}
24+
25+
/// Replace the editor's contents, unless it is already showing them.
26+
///
27+
/// Safe either way: the change flows back through the binding, and `Document.setContent`
28+
/// guards on equality, so reloading the same text records no edit and no undo entry.
29+
func reload(text: String) {
30+
guard let controller, controller.textView.string != text else { return }
31+
controller.setText(text)
32+
}
33+
}

‎Sources/CodeEditorUI/Sources/Sidebar/FileExplorerView.swift‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -187,16 +187,25 @@ public struct FileExplorerView: View {
187187
}
188188
}
189189

190-
/// The disclosure chevron. Decorative, not a control: selecting the row is what
191-
/// expands a folder, so a second tap target here would toggle it twice.
190+
/// The disclosure chevron. A real control: expansion hangs off the selection change,
191+
/// so a folder that is *already* selected produces no change and nothing would toggle.
192+
/// Selecting it first also makes the row selection agree with the chevron, which is
193+
/// what stops the two from cancelling out.
192194
@ViewBuilder
193195
private func disclosureControl(for item: FileSystemItem) -> some View {
194196
if item.isDirectory {
195-
Image(systemName: fileSystemManager.isExpanded(item.url) ? "chevron.down" : "chevron.right")
196-
.font(.system(size: 9, weight: .bold))
197-
.foregroundStyle(.tertiary)
198-
.frame(width: 12, height: 12)
199-
.accessibilityHidden(true)
197+
Button {
198+
selection = item
199+
fileSystemManager.toggleExpansion(of: item)
200+
} label: {
201+
Image(systemName: fileSystemManager.isExpanded(item.url) ? "chevron.down" : "chevron.right")
202+
.font(.system(size: 9, weight: .bold))
203+
.foregroundStyle(.tertiary)
204+
.frame(width: 12, height: 12)
205+
.contentShape(Rectangle())
206+
}
207+
.buttonStyle(.plain)
208+
.help(fileSystemManager.isExpanded(item.url) ? "Collapse" : "Expand")
200209
} else {
201210
Color.clear.frame(width: 12, height: 12)
202211
}

0 commit comments

Comments
 (0)