Tree-Style Tabs & Split Views #152

ImTheSquid · 2025-10-22T14:46:16Z

Working on adding tree-style tabs. I'll add more checklist items as I discover what needs to be done. Closes #135, closes #167, closes #174.

ImTheSquid · 2025-10-22T18:50:11Z

Initial implementation done, looking for feedback for what to improve!

Copilot

Pull request overview

This PR implements tree-style tabs and split view functionality for the Ora browser, addressing issues #135 (tree-style tabs feature request), #167 (split tabs view), and #174 (tab dragging improvements). The implementation adds parent-child tab relationships, tileset management for split views, and consolidates tab list components.

Key Changes

Added hierarchical tab structure with parent-child relationships and tileset support for split views
Implemented drag-and-drop improvements with new delegates and visual indicators for better UX
Unified tab list components (PinnedTabsList, NormalTabsList) into a single TabsList component with tree rendering support

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 23 comments.

Show a summary per file

File	Description
ora/Models/Tab.swift	Added parent/child relationships, tileset association, and helper methods for hierarchy management
ora/Models/TabContainer.swift	Added tileset management, enhanced reordering logic with reparenting behavior, and tree-aware tab ordering
ora/Models/TabTileset.swift	New model representing a collection of tabs displayed in a split view
ora/Services/TabManager.swift	Enhanced tab activation to handle splits, added split detection, and parent-aware tab creation
ora/Services/TabDropDelegate.swift	Refactored drop delegates with new target types and split view support (contains duplicated code from GeneralDropDelegate.swift)
ora/Services/GeneralDropDelegate.swift	New file with consolidated drop handling logic (duplicates structures from TabDropDelegate.swift)
ora/Services/WebViewNavigationDelegate.swift	Added automatic parenting for new tabs opened from existing tabs
ora/Modules/Sidebar/TabList/TabsList.swift	Unified tab list component with tree rendering via tabsSortedByParent function
ora/Modules/Sidebar/TabList/FavTabsList.swift	Updated to support tilesets in favorite tabs grid
ora/Modules/Browser/BrowserSplitView.swift	Modified rendering to display split view tabs side-by-side
ora/Modules/Browser/BrowserView.swift	Added handler to flatten all tabs when tree-style tabs are disabled
ora/Modules/Settings/Sections/GeneralSettingsView.swift	Added toggle for tree-style tabs feature
ora/Common/Utils/SettingsStore.swift	Added treeTabsEnabled setting with persistence
ora/UI/TabItem.swift	Added new bindings for drag target state tracking
ora/UI/FavTabItem.swift	Changed to accept array of tabs for tileset rendering
ora/UI/DropCapsule.swift	New drop indicator component for visual feedback
ora/UI/DragTarget.swift	New overlay component showing drop targets for parenting/sibling operations
ora/Modules/Sidebar/ContainerView.swift	Updated to use unified TabsList and improved drag cleanup
ora/Common/Utils/TabUtils.swift	Deleted - utility functions moved or replaced
ora/Services/SectionDropDelegate.swift	Deleted - replaced by GeneralDropDelegate
ora/Modules/Sidebar/TabList/PinnedTabsList.swift	Deleted - replaced by unified TabsList
ora/Modules/Sidebar/TabList/NormalTabsList.swift	Deleted - replaced by unified TabsList

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-28T15:31:08Z

ora/Modules/Sidebar/TabList/TabsList.swift

+                IndentedTab(
+                    tab: root,
+                    indentationLevel: indentation,
+                    tabs: toAppend.reversed()


The tabs array is being reversed when adding to IndentedTab (line 39), but it's not clear why this reversal is necessary. This could lead to tabs appearing in an unexpected order in tilesets. Consider documenting why the reversal is needed, or verify this is the intended behavior.

Suggested change

tabs: toAppend.reversed()

// NOTE: We intentionally reverse `toAppend` so that tabs belonging to a

// tileset are stored in the order they are rendered in the UI. Changing

// this may alter the visible ordering of tabs within tilesets.

tabs: Array(toAppend.reversed())

Copilot · 2025-12-28T15:31:08Z

ora/Modules/Sidebar/TabList/TabsList.swift

+            }
+            let foundTiles = Set(tileset.tabs.map(\.id))
+            toAppend = tabs.filter { foundTiles.contains($0.id) }
+            assert(!toAppend.isEmpty)


The assert statement will crash in release builds if toAppend is empty. While this might indicate a programming error, it could also happen with data corruption or race conditions. Consider using a proper error handling mechanism instead of assert, or at minimum add a guard statement that handles this case gracefully.

Suggested change

assert(!toAppend.isEmpty)

guard !toAppend.isEmpty else {

// If the tileset does not contain any matching tabs, skip processing this root

continue

}

Copilot · 2025-12-28T15:31:08Z

ora/Modules/Sidebar/TabList/TabsList.swift

+                targetedDropItem: $targetedDropItem,
+                draggedItem: $draggedItem,
+                delegate: TopDropDelegate(
+                    container: tabManager.activeContainer ?? containers.first!,


Force unwrapping containers.first! will crash if there are no containers. While this might be unlikely, it's safer to provide a default container or handle this case gracefully. Consider using ?? TabContainer() or checking for nil and providing appropriate fallback behavior.

Copilot · 2025-12-28T15:31:08Z

ora/Modules/Sidebar/TabList/FavTabsList.swift

            } else {
-                ForEach(tabs) { tab in
+                ForEach(tabsSortedByParent(tabs)) { iTab in
+                    let tab = iTab.tabs.first!


Force unwrapping iTab.tabs.first! assumes that tabs array is never empty. However, if there's an issue with the tabsSortedByParent function or data corruption, this could crash. Consider using optional binding or providing a guard statement to handle the empty case.

Copilot · 2025-12-28T15:31:09Z

ora/Modules/Sidebar/TabList/TabsList.swift

+                    VStack(spacing: 3) {
+                        HStack {
+                            ForEach(iTab.tabs) { tab in
+                                TabItem(
+                                    tab: tab,
+                                    isSelected: iTab.tabs
+                                        .contains(
+                                            where: { t in tabManager.isActive(t)
+                                            }),
+                                    isDragging: draggedItem == tab.id,
+                                    isDragTarget: targetedDropItem?
+                                        .imTargeted(
+                                            withMyIdBeing: tab.id,
+                                            andType: .tab(tabset: true)
+                                        ) ?? false,
+                                    onTap: { onSelect(tab) },
+                                    onPinToggle: { onPinToggle(tab) },
+                                    onFavoriteToggle: { onFavoriteToggle(tab) },
+                                    onClose: { onClose(tab) },
+                                    onDuplicate: { onDuplicate(tab) },
+                                    onMoveToContainer: { onMoveToContainer(tab, $0) },
+                                    availableContainers: containers,
+                                    draggedItem: $draggedItem,
+                                    targetedDropItem: $targetedDropItem
+                                )
+                                .onDrag { onDrag(tab.id) }
+                                .onDrop(
+                                    of: [.text],
+                                    delegate: GeneralDropDelegate(
+                                        item: .tab(tab),
+                                        representative: .tab(tabset: false), draggedItem: $draggedItem,
+                                        targetedItem: $targetedDropItem,
+                                        targetSection: isPinned ? .pinned : .normal
+                                    )
+                                )
+                                .transition(.asymmetric(
+                                    insertion: .opacity.combined(with: .move(edge: .bottom)),
+                                    removal: .opacity.combined(with: .move(edge: .top))
+                                ))
+                                .animation(.spring(response: 0.3, dampingFraction: 0.8), value: shouldAnimate(tab))
+                            }
+                        }
+                        .overlay(
+                            iTab.tabs.contains(where: { targetedDropItem?.imTargeted(
+                                withMyIdBeing: $0.id,
+                                andType: .tab(tabset: false)
+                            ) ?? false }) ? DragTarget(
+                                tab: iTab.tabs.first!,
+                                draggedItem: $draggedItem,
+                                targetedDropItem: $targetedDropItem,
+                                showTree: settings.treeTabsEnabled
+                            ) : nil
+                        )
+
+                        DropCapsule(
+                            id: iTab.tabs.first!.id,
+                            targetedDropItem: $targetedDropItem,
+                            draggedItem: $draggedItem,
+                            delegate: GeneralDropDelegate(
+                                item: .tab(iTab.tabs.first!),
+                                representative: .divider,
+                                draggedItem: $draggedItem,
+                                targetedItem: $targetedDropItem,
+                                targetSection: isPinned ? .pinned : .normal
+                            )
+                        )
+                    }
+                    .padding(
+                        .leading,
+                        CGFloat(
+                            integerLiteral: settings.treeTabsEnabled ? iTab.indentationLevel * 8 : 0
+                        )
+                    )


Multiple force unwraps of iTab.tabs.first! (lines 181, 189, 193) assume that the tabs array is never empty. However, if there's an issue with the tabsSortedByParent function or data corruption, these will crash. Consider using optional binding or providing guard statements to handle the empty case.

Suggested change

VStack(spacing: 3) {

HStack {

ForEach(iTab.tabs) { tab in

TabItem(

tab: tab,

isSelected: iTab.tabs

.contains(

where: { t in tabManager.isActive(t)

}),

isDragging: draggedItem == tab.id,

isDragTarget: targetedDropItem?

.imTargeted(

withMyIdBeing: tab.id,

andType: .tab(tabset: true)

) ?? false,

onTap: { onSelect(tab) },

onPinToggle: { onPinToggle(tab) },

onFavoriteToggle: { onFavoriteToggle(tab) },

onClose: { onClose(tab) },

onDuplicate: { onDuplicate(tab) },

onMoveToContainer: { onMoveToContainer(tab, $0) },

availableContainers: containers,

draggedItem: $draggedItem,

targetedDropItem: $targetedDropItem

)

.onDrag { onDrag(tab.id) }

.onDrop(

of: [.text],

delegate: GeneralDropDelegate(

item: .tab(tab),

representative: .tab(tabset: false), draggedItem: $draggedItem,

targetedItem: $targetedDropItem,

targetSection: isPinned ? .pinned : .normal

)

)

.transition(.asymmetric(

insertion: .opacity.combined(with: .move(edge: .bottom)),

removal: .opacity.combined(with: .move(edge: .top))

))

.animation(.spring(response: 0.3, dampingFraction: 0.8), value: shouldAnimate(tab))

}

}

.overlay(

iTab.tabs.contains(where: { targetedDropItem?.imTargeted(

withMyIdBeing: $0.id,

andType: .tab(tabset: false)

) ?? false }) ? DragTarget(

tab: iTab.tabs.first!,

draggedItem: $draggedItem,

targetedDropItem: $targetedDropItem,

showTree: settings.treeTabsEnabled

) : nil

)

DropCapsule(

id: iTab.tabs.first!.id,

targetedDropItem: $targetedDropItem,

draggedItem: $draggedItem,

delegate: GeneralDropDelegate(

item: .tab(iTab.tabs.first!),

representative: .divider,

draggedItem: $draggedItem,

targetedItem: $targetedDropItem,

targetSection: isPinned ? .pinned : .normal

)

)

}

.padding(

.leading,

CGFloat(

integerLiteral: settings.treeTabsEnabled ? iTab.indentationLevel * 8 : 0

)

)

if let firstTab = iTab.tabs.first {

VStack(spacing: 3) {

HStack {

ForEach(iTab.tabs) { tab in

TabItem(

tab: tab,

isSelected: iTab.tabs

.contains(

where: { t in tabManager.isActive(t)

}),

isDragging: draggedItem == tab.id,

isDragTarget: targetedDropItem?

.imTargeted(

withMyIdBeing: tab.id,

andType: .tab(tabset: true)

) ?? false,

onTap: { onSelect(tab) },

onPinToggle: { onPinToggle(tab) },

onFavoriteToggle: { onFavoriteToggle(tab) },

onClose: { onClose(tab) },

onDuplicate: { onDuplicate(tab) },

onMoveToContainer: { onMoveToContainer(tab, $0) },

availableContainers: containers,

draggedItem: $draggedItem,

targetedDropItem: $targetedDropItem

)

.onDrag { onDrag(tab.id) }

.onDrop(

of: [.text],

delegate: GeneralDropDelegate(

item: .tab(tab),

representative: .tab(tabset: false), draggedItem: $draggedItem,

targetedItem: $targetedDropItem,

targetSection: isPinned ? .pinned : .normal

)

)

.transition(.asymmetric(

insertion: .opacity.combined(with: .move(edge: .bottom)),

removal: .opacity.combined(with: .move(edge: .top))

))

.animation(.spring(response: 0.3, dampingFraction: 0.8), value: shouldAnimate(tab))

}

}

.overlay(

iTab.tabs.contains(where: { targetedDropItem?.imTargeted(

withMyIdBeing: $0.id,

andType: .tab(tabset: false)

) ?? false }) ? DragTarget(

tab: firstTab,

draggedItem: $draggedItem,

targetedDropItem: $targetedDropItem,

showTree: settings.treeTabsEnabled

) : nil

)

DropCapsule(

id: firstTab.id,

targetedDropItem: $targetedDropItem,

draggedItem: $draggedItem,

delegate: GeneralDropDelegate(

item: .tab(firstTab),

representative: .divider,

draggedItem: $draggedItem,

targetedItem: $targetedDropItem,

targetSection: isPinned ? .pinned : .normal

)

)

}

.padding(

.leading,

CGFloat(

integerLiteral: settings.treeTabsEnabled ? iTab.indentationLevel * 8 : 0

)

)

}

Copilot · 2025-12-28T15:31:12Z

ora/Services/TabManager.swift

+        var tabsToActivate = Set([tab])
+        if let activeContainer, let activeTab {
+            addSplitMembers(to: &tabsToActivate, fromContainer: activeContainer)
+            return tabsToActivate.contains(activeTab)
        }
        return false


The isActive method logic appears reversed. It adds the passed tab to a set, then adds split members based on the activeTab, and checks if the activeTab is in that set. This seems backward - it should be checking if tab is in the set of active tabs that includes the activeTab and its split members. The correct logic would be to start with activeTab in the set, add its split members, then check if tab is in that set.

Suggested change

var tabsToActivate = Set([tab])

if let activeContainer, let activeTab {

addSplitMembers(to: &tabsToActivate, fromContainer: activeContainer)

return tabsToActivate.contains(activeTab)

}

return false

guard let activeContainer, let activeTab else {

return false

}

var activeTabs = Set([activeTab])

addSplitMembers(to: &activeTabs, fromContainer: activeContainer)

return activeTabs.contains(tab)

Copilot · 2025-12-28T15:31:12Z

ora/Services/TabManager.swift


                let cleanHost = host.hasPrefix("www.") ? String(host.dropFirst(4)) : host

+                let orderBase = (parent != nil ? parent!.children : container.tabs).map(\.order).max() ?? -1


Force unwrapping parent!.children on line 263 when parent != nil is unsafe. If parent somehow becomes nil between the check and the unwrap (though unlikely in this synchronous code), or if the check itself is bypassed, this will crash. Use optional chaining instead: parent?.children ?? container.tabs.

Suggested change

let orderBase = (parent != nil ? parent!.children : container.tabs).map(\.order).max() ?? -1

let orderBase = (parent?.children ?? container.tabs).map(\.order).max() ?? -1

Copilot · 2025-12-28T15:31:12Z

ora/Modules/Sidebar/TabList/FavTabsList.swift

+            delegate: GeneralDropDelegate(
+                item:
+                .container(
+                    tabManager.activeContainer!),


Force unwrapping tabManager.activeContainer! will crash if activeContainer is nil. This can happen during initialization or in edge cases. Consider using optional chaining or a guard statement to handle the nil case gracefully.

Copilot · 2025-12-28T15:31:13Z

ora/Modules/Sidebar/TabList/TabsList.swift

+    withIndentation indentation: Int,
+    withTilesetUseSet usedTilesets: inout Set<UUID>
+) -> [IndentedTab] {
+    // Start by finding only parent tags, then recurse down through


The comment on line 18 says "parent tags" but should say "parent tabs" since this function is dealing with tabs, not tags.

Suggested change

// Start by finding only parent tags, then recurse down through

// Start by finding only parent tabs, then recurse down through

Copilot · 2025-12-28T15:31:13Z

ora/Modules/Sidebar/ContainerView.swift

        let provider = TabItemProvider(object: tabId.uuidString as NSString)
        provider.didEnd = {
-            draggedItem = nil
+            Task { @MainActor in


Wrapping the draggedItem assignment in a Task with @mainactor is good for ensuring it runs on the main thread. However, the didEnd callback might be called after the drag operation completes, and wrapping it in an async Task could introduce a slight delay. If immediate cleanup is required, consider using DispatchQueue.main.async or ensure this delay doesn't cause UI inconsistencies.

Suggested change

Task { @MainActor in

DispatchQueue.main.async {

ImTheSquid added 4 commits October 22, 2025 10:42

Parenting implmenetation

cd13107

Added better outlining for how tabs will be reodered

5ef3f61

Fix ordering issues

9a9ade3

Other various fixes

1560862

ImTheSquid marked this pull request as ready for review October 22, 2025 18:49

ImTheSquid added 5 commits October 22, 2025 18:01

Remove empty function

8170b59

Merge branch 'main' into feature/tree-tabs

01e3a0c

Merge branch 'main' into feature/tree-tabs

0ef9e1f

Start working on side by side

b4244d9

Show all tabs in sbs as selected

40a7383

ImTheSquid marked this pull request as draft October 29, 2025 15:24

ImTheSquid changed the title ~~Tree-Style Tabs~~ Tree-Style Tabs & Split Views Oct 29, 2025

ImTheSquid added 5 commits October 29, 2025 12:25

Significant progress on split views

d982979

Tab activation

8c16881

Code cleanup

9c7d387

Favorites support

18e0f6f

Merge branch 'main' into feature/tree-tabs

0c33ece

ImTheSquid marked this pull request as ready for review October 30, 2025 01:17

ImTheSquid requested review from kenenisa and yonaries as code owners October 30, 2025 01:17

ImTheSquid added 2 commits October 29, 2025 21:45

Remove debug data

049ce55

Merge branch 'main' into feature/tree-tabs

b4c7aed

Copilot AI review requested due to automatic review settings December 28, 2025 15:23

Copilot started reviewing on behalf of ImTheSquid December 28, 2025 15:24 View session

Merge branch 'main' into feature/tree-tabs

777b47f

Copilot AI reviewed Dec 28, 2025

View reviewed changes

-                    tabs: toAppend.reversed()
+                    // NOTE: We intentionally reverse `toAppend` so that tabs belonging to a
+                    // tileset are stored in the order they are rendered in the UI. Changing
+                    // this may alter the visible ordering of tabs within tilesets.
+                    tabs: Array(toAppend.reversed())

-            assert(!toAppend.isEmpty)
+            guard !toAppend.isEmpty else {
+                // If the tileset does not contain any matching tabs, skip processing this root
+                continue
+            }


		let cleanHost = host.hasPrefix("www.") ? String(host.dropFirst(4)) : host

		let orderBase = (parent != nil ? parent!.children : container.tabs).map(\.order).max() ?? -1

	let orderBase = (parent != nil ? parent!.children : container.tabs).map(\.order).max() ?? -1
	let orderBase = (parent?.children ?? container.tabs).map(\.order).max() ?? -1

	// Start by finding only parent tags, then recurse down through
	// Start by finding only parent tabs, then recurse down through

Tree-Style Tabs & Split Views #152

Are you sure you want to change the base?

Tree-Style Tabs & Split Views #152

Uh oh!

Conversation

ImTheSquid commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ImTheSquid commented Oct 22, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ImTheSquid commented Oct 22, 2025 •

edited

Loading