Merge pull request #92 from bnema/fix/stacked-pane-focus-and-close
fix: stacked pane focus and vertical split improvements
16 files · +600 -226
M CHANGELOG.md +5 -0
| @@ -7,8 +7,13 @@ All notable changes to this project will be documented in this file. | |||
| 7 | 7 | ### Added | |
| 8 | 8 | ||
| 9 | 9 | ### Changed | |
| 10 | + | - **Flatpak runtime**: Bumped to GNOME 49 runtime. | |
| 10 | 11 | ||
| 11 | 12 | ### Fixed | |
| 13 | + | - **Flatpak locale crash**: Attempted fix for crash on systems with certain locale configurations by adding proper locale data to the Flatpak bundle. | |
| 14 | + | - **Stacked pane vertical splits**: Vertical splits from stacked panes now correctly split around the entire stack instead of just the active pane. | |
| 15 | + | - **Pane border focus**: Fixed stale border remaining on previously hovered pane when using keyboard navigation to switch focus. | |
| 16 | + | - **Geometric focus navigation**: Fixed focus jumping to wrong pane when navigating left/right in complex layouts. Navigation now prioritizes panes at the same vertical level (for left/right) or horizontal level (for up/down). | |
| 12 | 17 | ||
| 13 | 18 | ## [0.23.2] - 2025-12-31 | |
| 14 | 19 | ||
| @@ -7,8 +7,13 @@ All notable changes to this project will be documented in this file. | |||||
| 7 | ### Added | 7 | ### Added | ||
| 8 | 8 | ||||
| 9 | ### Changed | 9 | ### Changed | ||
| 10 | + | - **Flatpak runtime**: Bumped to GNOME 49 runtime. | |||
| 10 | 11 | ||||
| 11 | ### Fixed | 12 | ### Fixed | ||
| 13 | + | - **Flatpak locale crash**: Attempted fix for crash on systems with certain locale configurations by adding proper locale data to the Flatpak bundle. | |||
| 14 | + | - **Stacked pane vertical splits**: Vertical splits from stacked panes now correctly split around the entire stack instead of just the active pane. | |||
| 15 | + | - **Pane border focus**: Fixed stale border remaining on previously hovered pane when using keyboard navigation to switch focus. | |||
| 16 | + | - **Geometric focus navigation**: Fixed focus jumping to wrong pane when navigating left/right in complex layouts. Navigation now prioritizes panes at the same vertical level (for left/right) or horizontal level (for up/down). | |||
| 12 | 17 | ||||
| 13 | ## [0.23.2] - 2025-12-31 | 18 | ## [0.23.2] - 2025-12-31 | ||
| 14 | 19 | ||||
M internal/application/usecase/manage_panes.go +123 -54
| @@ -112,15 +112,14 @@ func (uc *ManagePanesUseCase) Split(ctx context.Context, input SplitPaneInput) ( | |||
| 112 | 112 | return nil, fmt.Errorf("target pane is required") | |
| 113 | 113 | } | |
| 114 | 114 | ||
| 115 | - | // If target is inside a stack and splitting left/right, split around the entire stack | |
| 115 | + | // If target is inside a stack, split around the entire stack | |
| 116 | 116 | // (i.e., create the new pane as a sibling to the stack, not inside it) | |
| 117 | + | // This applies to all directions - we don't want to nest split containers inside stacks. | |
| 117 | 118 | targetNode := input.TargetPane | |
| 118 | - | if input.Direction == SplitLeft || input.Direction == SplitRight { | |
| 119 | - | if targetNode.Parent != nil && targetNode.Parent.IsStacked { | |
| 120 | - | // Promote to the stack container so split happens outside the stack | |
| 121 | - | targetNode = targetNode.Parent | |
| 122 | - | log.Debug().Msg("target is inside a stack, splitting around stack container") | |
| 123 | - | } | |
| 119 | + | if targetNode.Parent != nil && targetNode.Parent.IsStacked { | |
| 120 | + | // Promote to the stack container so split happens outside the stack | |
| 121 | + | targetNode = targetNode.Parent | |
| 122 | + | log.Debug().Msg("target is inside a stack, splitting around stack container") | |
| 124 | 123 | } | |
| 125 | 124 | ||
| 126 | 125 | // Create new pane | |
| @@ -619,12 +618,61 @@ type GeometricNavigationOutput struct { | |||
| 619 | 618 | Found bool | |
| 620 | 619 | } | |
| 621 | 620 | ||
| 621 | + | // ApplyFocusChange sets the active pane and updates the stack index if the target pane is in a stack. | |
| 622 | + | // This is the canonical way to change focus to a pane, ensuring stack state is kept in sync. | |
| 623 | + | func (uc *ManagePanesUseCase) ApplyFocusChange( | |
| 624 | + | ctx context.Context, | |
| 625 | + | ws *entity.Workspace, | |
| 626 | + | targetPaneID entity.PaneID, | |
| 627 | + | ) (*entity.PaneNode, error) { | |
| 628 | + | log := logging.FromContext(ctx) | |
| 629 | + | if uc == nil { | |
| 630 | + | return nil, fmt.Errorf("manage panes use case is nil") | |
| 631 | + | } | |
| 632 | + | if ws == nil { | |
| 633 | + | return nil, fmt.Errorf("workspace is required") | |
| 634 | + | } | |
| 635 | + | ||
| 636 | + | targetNode := ws.FindPane(targetPaneID) | |
| 637 | + | if targetNode == nil { | |
| 638 | + | return nil, fmt.Errorf("pane not found: %s", targetPaneID) | |
| 639 | + | } | |
| 640 | + | ||
| 641 | + | oldActive := ws.ActivePaneID | |
| 642 | + | ws.ActivePaneID = targetPaneID | |
| 643 | + | ||
| 644 | + | // Update stack index if target is in a stack | |
| 645 | + | if targetNode.Parent != nil && targetNode.Parent.IsStacked { | |
| 646 | + | oldIndex := targetNode.Parent.ActiveStackIndex | |
| 647 | + | for i, child := range targetNode.Parent.Children { | |
| 648 | + | if child == targetNode { | |
| 649 | + | targetNode.Parent.ActiveStackIndex = i | |
| 650 | + | log.Debug(). | |
| 651 | + | Str("target_pane_id", string(targetPaneID)). | |
| 652 | + | Int("old_stack_index", oldIndex). | |
| 653 | + | Int("new_stack_index", i). | |
| 654 | + | Int("num_children", len(targetNode.Parent.Children)). | |
| 655 | + | Msg("updated stack index for focus change") | |
| 656 | + | break | |
| 657 | + | } | |
| 658 | + | } | |
| 659 | + | } | |
| 660 | + | ||
| 661 | + | log.Info(). | |
| 662 | + | Str("from", string(oldActive)). | |
| 663 | + | Str("to", string(targetPaneID)). | |
| 664 | + | Msg("focus changed") | |
| 665 | + | ||
| 666 | + | return targetNode, nil | |
| 667 | + | } | |
| 668 | + | ||
| 622 | 669 | // NavigateFocusGeometric finds the nearest pane in the given direction using geometry. | |
| 623 | 670 | // Algorithm: | |
| 624 | - | // 1. Get center of active pane | |
| 671 | + | // 1. Get active pane rectangle | |
| 625 | 672 | // 2. Filter candidates that are in the direction (dx < 0 for Left, etc.) | |
| 626 | - | // 3. Score by: primary_distance * 1000 + perpendicular_distance | |
| 627 | - | // 4. Return lowest scoring candidate | |
| 673 | + | // 3. Prioritize panes with perpendicular overlap (same row for left/right, same column for up/down) | |
| 674 | + | // 4. Score by: overlap_penalty + primary_distance * 1000 + perpendicular_distance | |
| 675 | + | // 5. Return lowest scoring candidate | |
| 628 | 676 | func (uc *ManagePanesUseCase) NavigateFocusGeometric( | |
| 629 | 677 | ctx context.Context, | |
| 630 | 678 | input GeometricNavigationInput, | |
| @@ -652,50 +700,7 @@ func (uc *ManagePanesUseCase) NavigateFocusGeometric( | |||
| 652 | 700 | return &GeometricNavigationOutput{Found: false}, nil | |
| 653 | 701 | } | |
| 654 | 702 | ||
| 655 | - | acx, acy := activeRect.Center() | |
| 656 | - | ||
| 657 | - | type candidate struct { | |
| 658 | - | paneID entity.PaneID | |
| 659 | - | score int | |
| 660 | - | } | |
| 661 | - | var candidates []candidate | |
| 662 | - | ||
| 663 | - | for _, rect := range input.PaneRects { | |
| 664 | - | if rect.PaneID == input.ActivePaneID { | |
| 665 | - | continue | |
| 666 | - | } | |
| 667 | - | ||
| 668 | - | cx, cy := rect.Center() | |
| 669 | - | dx := cx - acx | |
| 670 | - | dy := cy - acy | |
| 671 | - | ||
| 672 | - | var inDirection bool | |
| 673 | - | var primaryDist, perpDist int | |
| 674 | - | ||
| 675 | - | switch input.Direction { | |
| 676 | - | case NavLeft: | |
| 677 | - | inDirection = dx < 0 | |
| 678 | - | primaryDist = abs(dx) | |
| 679 | - | perpDist = abs(dy) | |
| 680 | - | case NavRight: | |
| 681 | - | inDirection = dx > 0 | |
| 682 | - | primaryDist = abs(dx) | |
| 683 | - | perpDist = abs(dy) | |
| 684 | - | case NavUp: | |
| 685 | - | inDirection = dy < 0 | |
| 686 | - | primaryDist = abs(dy) | |
| 687 | - | perpDist = abs(dx) | |
| 688 | - | case NavDown: | |
| 689 | - | inDirection = dy > 0 | |
| 690 | - | primaryDist = abs(dy) | |
| 691 | - | perpDist = abs(dx) | |
| 692 | - | } | |
| 693 | - | ||
| 694 | - | if inDirection { | |
| 695 | - | score := primaryDist*1000 + perpDist | |
| 696 | - | candidates = append(candidates, candidate{rect.PaneID, score}) | |
| 697 | - | } | |
| 698 | - | } | |
| 703 | + | candidates := scoreNavigationCandidates(*activeRect, input.PaneRects, input.Direction) | |
| 699 | 704 | ||
| 700 | 705 | if len(candidates) == 0 { | |
| 701 | 706 | log.Debug().Msg("no candidates in direction") | |
| @@ -718,6 +723,70 @@ func (uc *ManagePanesUseCase) NavigateFocusGeometric( | |||
| 718 | 723 | }, nil | |
| 719 | 724 | } | |
| 720 | 725 | ||
| 726 | + | // navCandidate represents a pane candidate for navigation with its score. | |
| 727 | + | type navCandidate struct { | |
| 728 | + | paneID entity.PaneID | |
| 729 | + | score int | |
| 730 | + | } | |
| 731 | + | ||
| 732 | + | // scoreNavigationCandidates scores all panes in the given direction from activeRect. | |
| 733 | + | // Panes with perpendicular overlap are heavily preferred (same row for left/right, same column for up/down). | |
| 734 | + | func scoreNavigationCandidates( | |
| 735 | + | activeRect entity.PaneRect, | |
| 736 | + | paneRects []entity.PaneRect, | |
| 737 | + | direction NavigateDirection, | |
| 738 | + | ) []navCandidate { | |
| 739 | + | // Large penalty for panes without perpendicular overlap. | |
| 740 | + | // This ensures panes at the same level are always preferred. | |
| 741 | + | const noOverlapPenalty = 10_000_000 | |
| 742 | + | ||
| 743 | + | acx, acy := activeRect.Center() | |
| 744 | + | var candidates []navCandidate | |
| 745 | + | ||
| 746 | + | for _, rect := range paneRects { | |
| 747 | + | if rect.PaneID == activeRect.PaneID { | |
| 748 | + | continue | |
| 749 | + | } | |
| 750 | + | ||
| 751 | + | cx, cy := rect.Center() | |
| 752 | + | dx := cx - acx | |
| 753 | + | dy := cy - acy | |
| 754 | + | ||
| 755 | + | inDirection, primaryDist, perpDist, hasOverlap := evalDirection(activeRect, rect, dx, dy, direction) | |
| 756 | + | ||
| 757 | + | if inDirection { | |
| 758 | + | score := primaryDist*1000 + perpDist | |
| 759 | + | if !hasOverlap { | |
| 760 | + | score += noOverlapPenalty | |
| 761 | + | } | |
| 762 | + | candidates = append(candidates, navCandidate{rect.PaneID, score}) | |
| 763 | + | } | |
| 764 | + | } | |
| 765 | + | ||
| 766 | + | return candidates | |
| 767 | + | } | |
| 768 | + | ||
| 769 | + | // evalDirection determines if a candidate rect is in the given direction from activeRect. | |
| 770 | + | // Returns: inDirection, primaryDist, perpDist, hasOverlap | |
| 771 | + | func evalDirection( | |
| 772 | + | activeRect, rect entity.PaneRect, | |
| 773 | + | dx, dy int, | |
| 774 | + | direction NavigateDirection, | |
| 775 | + | ) (inDirection bool, primaryDist, perpDist int, hasOverlap bool) { | |
| 776 | + | switch direction { | |
| 777 | + | case NavLeft: | |
| 778 | + | return dx < 0, abs(dx), abs(dy), activeRect.OverlapsVertically(rect) | |
| 779 | + | case NavRight: | |
| 780 | + | return dx > 0, abs(dx), abs(dy), activeRect.OverlapsVertically(rect) | |
| 781 | + | case NavUp: | |
| 782 | + | return dy < 0, abs(dy), abs(dx), activeRect.OverlapsHorizontally(rect) | |
| 783 | + | case NavDown: | |
| 784 | + | return dy > 0, abs(dy), abs(dx), activeRect.OverlapsHorizontally(rect) | |
| 785 | + | default: | |
| 786 | + | return false, 0, 0, false | |
| 787 | + | } | |
| 788 | + | } | |
| 789 | + | ||
| 721 | 790 | // abs returns the absolute value of an integer. | |
| 722 | 791 | func abs(x int) int { | |
| 723 | 792 | if x < 0 { | |
| @@ -112,15 +112,14 @@ func (uc *ManagePanesUseCase) Split(ctx context.Context, input SplitPaneInput) ( | |||||
| 112 | return nil, fmt.Errorf("target pane is required") | 112 | return nil, fmt.Errorf("target pane is required") | ||
| 113 | } | 113 | } | ||
| 114 | 114 | ||||
| 115 | - | // If target is inside a stack and splitting left/right, split around the entire stack | 115 | + | // If target is inside a stack, split around the entire stack |
| 116 | // (i.e., create the new pane as a sibling to the stack, not inside it) | 116 | // (i.e., create the new pane as a sibling to the stack, not inside it) | ||
| 117 | + | // This applies to all directions - we don't want to nest split containers inside stacks. | |||
| 117 | targetNode := input.TargetPane | 118 | targetNode := input.TargetPane | ||
| 118 | - | if input.Direction == SplitLeft || input.Direction == SplitRight { | 119 | + | if targetNode.Parent != nil && targetNode.Parent.IsStacked { |
| 119 | - | if targetNode.Parent != nil && targetNode.Parent.IsStacked { | 120 | + | // Promote to the stack container so split happens outside the stack |
| 120 | - | // Promote to the stack container so split happens outside the stack | 121 | + | targetNode = targetNode.Parent |
| 121 | - | targetNode = targetNode.Parent | 122 | + | log.Debug().Msg("target is inside a stack, splitting around stack container") |
| 122 | - | log.Debug().Msg("target is inside a stack, splitting around stack container") | |||
| 123 | - | } | |||
| 124 | } | 123 | } | ||
| 125 | 124 | ||||
| 126 | // Create new pane | 125 | // Create new pane | ||
| @@ -619,12 +618,61 @@ type GeometricNavigationOutput struct { | |||||
| 619 | Found bool | 618 | Found bool | ||
| 620 | } | 619 | } | ||
| 621 | 620 | ||||
| 621 | + | // ApplyFocusChange sets the active pane and updates the stack index if the target pane is in a stack. | |||
| 622 | + | // This is the canonical way to change focus to a pane, ensuring stack state is kept in sync. | |||
| 623 | + | func (uc *ManagePanesUseCase) ApplyFocusChange( | |||
| 624 | + | ctx context.Context, | |||
| 625 | + | ws *entity.Workspace, | |||
| 626 | + | targetPaneID entity.PaneID, | |||
| 627 | + | ) (*entity.PaneNode, error) { | |||
| 628 | + | log := logging.FromContext(ctx) | |||
| 629 | + | if uc == nil { | |||
| 630 | + | return nil, fmt.Errorf("manage panes use case is nil") | |||
| 631 | + | } | |||
| 632 | + | if ws == nil { | |||
| 633 | + | return nil, fmt.Errorf("workspace is required") | |||
| 634 | + | } | |||
| 635 | + | ||||
| 636 | + | targetNode := ws.FindPane(targetPaneID) | |||
| 637 | + | if targetNode == nil { | |||
| 638 | + | return nil, fmt.Errorf("pane not found: %s", targetPaneID) | |||
| 639 | + | } | |||
| 640 | + | ||||
| 641 | + | oldActive := ws.ActivePaneID | |||
| 642 | + | ws.ActivePaneID = targetPaneID | |||
| 643 | + | ||||
| 644 | + | // Update stack index if target is in a stack | |||
| 645 | + | if targetNode.Parent != nil && targetNode.Parent.IsStacked { | |||
| 646 | + | oldIndex := targetNode.Parent.ActiveStackIndex | |||
| 647 | + | for i, child := range targetNode.Parent.Children { | |||
| 648 | + | if child == targetNode { | |||
| 649 | + | targetNode.Parent.ActiveStackIndex = i | |||
| 650 | + | log.Debug(). | |||
| 651 | + | Str("target_pane_id", string(targetPaneID)). | |||
| 652 | + | Int("old_stack_index", oldIndex). | |||
| 653 | + | Int("new_stack_index", i). | |||
| 654 | + | Int("num_children", len(targetNode.Parent.Children)). | |||
| 655 | + | Msg("updated stack index for focus change") | |||
| 656 | + | break | |||
| 657 | + | } | |||
| 658 | + | } | |||
| 659 | + | } | |||
| 660 | + | ||||
| 661 | + | log.Info(). | |||
| 662 | + | Str("from", string(oldActive)). | |||
| 663 | + | Str("to", string(targetPaneID)). | |||
| 664 | + | Msg("focus changed") | |||
| 665 | + | ||||
| 666 | + | return targetNode, nil | |||
| 667 | + | } | |||
| 668 | + | ||||
| 622 | // NavigateFocusGeometric finds the nearest pane in the given direction using geometry. | 669 | // NavigateFocusGeometric finds the nearest pane in the given direction using geometry. | ||
| 623 | // Algorithm: | 670 | // Algorithm: | ||
| 624 | - | // 1. Get center of active pane | 671 | + | // 1. Get active pane rectangle |
| 625 | // 2. Filter candidates that are in the direction (dx < 0 for Left, etc.) | 672 | // 2. Filter candidates that are in the direction (dx < 0 for Left, etc.) | ||
| 626 | - | // 3. Score by: primary_distance * 1000 + perpendicular_distance | 673 | + | // 3. Prioritize panes with perpendicular overlap (same row for left/right, same column for up/down) |
| 627 | - | // 4. Return lowest scoring candidate | 674 | + | // 4. Score by: overlap_penalty + primary_distance * 1000 + perpendicular_distance |
| 675 | + | // 5. Return lowest scoring candidate | |||
| 628 | func (uc *ManagePanesUseCase) NavigateFocusGeometric( | 676 | func (uc *ManagePanesUseCase) NavigateFocusGeometric( | ||
| 629 | ctx context.Context, | 677 | ctx context.Context, | ||
| 630 | input GeometricNavigationInput, | 678 | input GeometricNavigationInput, | ||
| @@ -652,50 +700,7 @@ func (uc *ManagePanesUseCase) NavigateFocusGeometric( | |||||
| 652 | return &GeometricNavigationOutput{Found: false}, nil | 700 | return &GeometricNavigationOutput{Found: false}, nil | ||
| 653 | } | 701 | } | ||
| 654 | 702 | ||||
| 655 | - | acx, acy := activeRect.Center() | 703 | + | candidates := scoreNavigationCandidates(*activeRect, input.PaneRects, input.Direction) |
| 656 | - | ||||
| 657 | - | type candidate struct { | |||
| 658 | - | paneID entity.PaneID | |||
| 659 | - | score int | |||
| 660 | - | } | |||
| 661 | - | var candidates []candidate | |||
| 662 | - | ||||
| 663 | - | for _, rect := range input.PaneRects { | |||
| 664 | - | if rect.PaneID == input.ActivePaneID { | |||
| 665 | - | continue | |||
| 666 | - | } | |||
| 667 | - | ||||
| 668 | - | cx, cy := rect.Center() | |||
| 669 | - | dx := cx - acx | |||
| 670 | - | dy := cy - acy | |||
| 671 | - | ||||
| 672 | - | var inDirection bool | |||
| 673 | - | var primaryDist, perpDist int | |||
| 674 | - | ||||
| 675 | - | switch input.Direction { | |||
| 676 | - | case NavLeft: | |||
| 677 | - | inDirection = dx < 0 | |||
| 678 | - | primaryDist = abs(dx) | |||
| 679 | - | perpDist = abs(dy) | |||
| 680 | - | case NavRight: | |||
| 681 | - | inDirection = dx > 0 | |||
| 682 | - | primaryDist = abs(dx) | |||
| 683 | - | perpDist = abs(dy) | |||
| 684 | - | case NavUp: | |||
| 685 | - | inDirection = dy < 0 | |||
| 686 | - | primaryDist = abs(dy) | |||
| 687 | - | perpDist = abs(dx) | |||
| 688 | - | case NavDown: | |||
| 689 | - | inDirection = dy > 0 | |||
| 690 | - | primaryDist = abs(dy) | |||
| 691 | - | perpDist = abs(dx) | |||
| 692 | - | } | |||
| 693 | - | ||||
| 694 | - | if inDirection { | |||
| 695 | - | score := primaryDist*1000 + perpDist | |||
| 696 | - | candidates = append(candidates, candidate{rect.PaneID, score}) | |||
| 697 | - | } | |||
| 698 | - | } | |||
| 699 | 704 | ||||
| 700 | if len(candidates) == 0 { | 705 | if len(candidates) == 0 { | ||
| 701 | log.Debug().Msg("no candidates in direction") | 706 | log.Debug().Msg("no candidates in direction") | ||
| @@ -718,6 +723,70 @@ func (uc *ManagePanesUseCase) NavigateFocusGeometric( | |||||
| 718 | }, nil | 723 | }, nil | ||
| 719 | } | 724 | } | ||
| 720 | 725 | ||||
| 726 | + | // navCandidate represents a pane candidate for navigation with its score. | |||
| 727 | + | type navCandidate struct { | |||
| 728 | + | paneID entity.PaneID | |||
| 729 | + | score int | |||
| 730 | + | } | |||
| 731 | + | ||||
| 732 | + | // scoreNavigationCandidates scores all panes in the given direction from activeRect. | |||
| 733 | + | // Panes with perpendicular overlap are heavily preferred (same row for left/right, same column for up/down). | |||
| 734 | + | func scoreNavigationCandidates( | |||
| 735 | + | activeRect entity.PaneRect, | |||
| 736 | + | paneRects []entity.PaneRect, | |||
| 737 | + | direction NavigateDirection, | |||
| 738 | + | ) []navCandidate { | |||
| 739 | + | // Large penalty for panes without perpendicular overlap. | |||
| 740 | + | // This ensures panes at the same level are always preferred. | |||
| 741 | + | const noOverlapPenalty = 10_000_000 | |||
| 742 | + | ||||
| 743 | + | acx, acy := activeRect.Center() | |||
| 744 | + | var candidates []navCandidate | |||
| 745 | + | ||||
| 746 | + | for _, rect := range paneRects { | |||
| 747 | + | if rect.PaneID == activeRect.PaneID { | |||
| 748 | + | continue | |||
| 749 | + | } | |||
| 750 | + | ||||
| 751 | + | cx, cy := rect.Center() | |||
| 752 | + | dx := cx - acx | |||
| 753 | + | dy := cy - acy | |||
| 754 | + | ||||
| 755 | + | inDirection, primaryDist, perpDist, hasOverlap := evalDirection(activeRect, rect, dx, dy, direction) | |||
| 756 | + | ||||
| 757 | + | if inDirection { | |||
| 758 | + | score := primaryDist*1000 + perpDist | |||
| 759 | + | if !hasOverlap { | |||
| 760 | + | score += noOverlapPenalty | |||
| 761 | + | } | |||
| 762 | + | candidates = append(candidates, navCandidate{rect.PaneID, score}) | |||
| 763 | + | } | |||
| 764 | + | } | |||
| 765 | + | ||||
| 766 | + | return candidates | |||
| 767 | + | } | |||
| 768 | + | ||||
| 769 | + | // evalDirection determines if a candidate rect is in the given direction from activeRect. | |||
| 770 | + | // Returns: inDirection, primaryDist, perpDist, hasOverlap | |||
| 771 | + | func evalDirection( | |||
| 772 | + | activeRect, rect entity.PaneRect, | |||
| 773 | + | dx, dy int, | |||
| 774 | + | direction NavigateDirection, | |||
| 775 | + | ) (inDirection bool, primaryDist, perpDist int, hasOverlap bool) { | |||
| 776 | + | switch direction { | |||
| 777 | + | case NavLeft: | |||
| 778 | + | return dx < 0, abs(dx), abs(dy), activeRect.OverlapsVertically(rect) | |||
| 779 | + | case NavRight: | |||
| 780 | + | return dx > 0, abs(dx), abs(dy), activeRect.OverlapsVertically(rect) | |||
| 781 | + | case NavUp: | |||
| 782 | + | return dy < 0, abs(dy), abs(dx), activeRect.OverlapsHorizontally(rect) | |||
| 783 | + | case NavDown: | |||
| 784 | + | return dy > 0, abs(dy), abs(dx), activeRect.OverlapsHorizontally(rect) | |||
| 785 | + | default: | |||
| 786 | + | return false, 0, 0, false | |||
| 787 | + | } | |||
| 788 | + | } | |||
| 789 | + | ||||
| 721 | // abs returns the absolute value of an integer. | 790 | // abs returns the absolute value of an integer. | ||
| 722 | func abs(x int) int { | 791 | func abs(x int) int { | ||
| 723 | if x < 0 { | 792 | if x < 0 { | ||
M internal/domain/entity/geometry.go +14 -0
| @@ -13,3 +13,17 @@ type PaneRect struct { | |||
| 13 | 13 | func (r PaneRect) Center() (cx, cy int) { | |
| 14 | 14 | return r.X + r.W/2, r.Y + r.H/2 | |
| 15 | 15 | } | |
| 16 | + | ||
| 17 | + | // OverlapsVertically returns true if the two rectangles overlap in the Y axis. | |
| 18 | + | // Used for left/right navigation to prefer panes at the same vertical level. | |
| 19 | + | func (r PaneRect) OverlapsVertically(other PaneRect) bool { | |
| 20 | + | // r spans [r.Y, r.Y+r.H), other spans [other.Y, other.Y+other.H) | |
| 21 | + | return r.Y < other.Y+other.H && other.Y < r.Y+r.H | |
| 22 | + | } | |
| 23 | + | ||
| 24 | + | // OverlapsHorizontally returns true if the two rectangles overlap in the X axis. | |
| 25 | + | // Used for up/down navigation to prefer panes at the same horizontal level. | |
| 26 | + | func (r PaneRect) OverlapsHorizontally(other PaneRect) bool { | |
| 27 | + | // r spans [r.X, r.X+r.W), other spans [other.X, other.X+other.W) | |
| 28 | + | return r.X < other.X+other.W && other.X < r.X+r.W | |
| 29 | + | } | |
| @@ -13,3 +13,17 @@ type PaneRect struct { | |||||
| 13 | func (r PaneRect) Center() (cx, cy int) { | 13 | func (r PaneRect) Center() (cx, cy int) { | ||
| 14 | return r.X + r.W/2, r.Y + r.H/2 | 14 | return r.X + r.W/2, r.Y + r.H/2 | ||
| 15 | } | 15 | } | ||
| 16 | + | ||||
| 17 | + | // OverlapsVertically returns true if the two rectangles overlap in the Y axis. | |||
| 18 | + | // Used for left/right navigation to prefer panes at the same vertical level. | |||
| 19 | + | func (r PaneRect) OverlapsVertically(other PaneRect) bool { | |||
| 20 | + | // r spans [r.Y, r.Y+r.H), other spans [other.Y, other.Y+other.H) | |||
| 21 | + | return r.Y < other.Y+other.H && other.Y < r.Y+r.H | |||
| 22 | + | } | |||
| 23 | + | ||||
| 24 | + | // OverlapsHorizontally returns true if the two rectangles overlap in the X axis. | |||
| 25 | + | // Used for up/down navigation to prefer panes at the same horizontal level. | |||
| 26 | + | func (r PaneRect) OverlapsHorizontally(other PaneRect) bool { | |||
| 27 | + | // r spans [r.X, r.X+r.W), other spans [other.X, other.X+other.W) | |||
| 28 | + | return r.X < other.X+other.W && other.X < r.X+r.W | |||
| 29 | + | } | |||
M internal/ui/component/pane_view.go +12 -0
| @@ -244,6 +244,18 @@ func (pv *PaneView) AttachHoverHandler(ctx context.Context) { | |||
| 244 | 244 | } | |
| 245 | 245 | } | |
| 246 | 246 | ||
| 247 | + | // CancelPendingHover cancels any pending hover focus timer. | |
| 248 | + | // Called when keyboard navigation occurs to prevent hover from overriding keyboard focus. | |
| 249 | + | func (pv *PaneView) CancelPendingHover() { | |
| 250 | + | pv.mu.RLock() | |
| 251 | + | handler := pv.hoverHandler | |
| 252 | + | pv.mu.RUnlock() | |
| 253 | + | ||
| 254 | + | if handler != nil { | |
| 255 | + | handler.Cancel() | |
| 256 | + | } | |
| 257 | + | } | |
| 258 | + | ||
| 247 | 259 | // Widget returns the underlying overlay widget for embedding in containers. | |
| 248 | 260 | func (pv *PaneView) Widget() layout.Widget { | |
| 249 | 261 | return pv.overlay | |
| @@ -244,6 +244,18 @@ func (pv *PaneView) AttachHoverHandler(ctx context.Context) { | |||||
| 244 | } | 244 | } | ||
| 245 | } | 245 | } | ||
| 246 | 246 | ||||
| 247 | + | // CancelPendingHover cancels any pending hover focus timer. | |||
| 248 | + | // Called when keyboard navigation occurs to prevent hover from overriding keyboard focus. | |||
| 249 | + | func (pv *PaneView) CancelPendingHover() { | |||
| 250 | + | pv.mu.RLock() | |||
| 251 | + | handler := pv.hoverHandler | |||
| 252 | + | pv.mu.RUnlock() | |||
| 253 | + | ||||
| 254 | + | if handler != nil { | |||
| 255 | + | handler.Cancel() | |||
| 256 | + | } | |||
| 257 | + | } | |||
| 258 | + | ||||
| 247 | // Widget returns the underlying overlay widget for embedding in containers. | 259 | // Widget returns the underlying overlay widget for embedding in containers. | ||
| 248 | func (pv *PaneView) Widget() layout.Widget { | 260 | func (pv *PaneView) Widget() layout.Widget { | ||
| 249 | return pv.overlay | 261 | return pv.overlay | ||
M internal/ui/component/workspace_view.go +54 -0
| @@ -6,6 +6,7 @@ import ( | |||
| 6 | 6 | "errors" | |
| 7 | 7 | "fmt" | |
| 8 | 8 | "sync" | |
| 9 | + | "time" | |
| 9 | 10 | ||
| 10 | 11 | "github.com/bnema/dumber/internal/domain/entity" | |
| 11 | 12 | "github.com/bnema/dumber/internal/logging" | |
| @@ -50,6 +51,10 @@ type WorkspaceView struct { | |||
| 50 | 51 | onPaneFocused func(paneID entity.PaneID) | |
| 51 | 52 | onSplitRatioDragged func(nodeID string, ratio float64) | |
| 52 | 53 | ||
| 54 | + | // Hover suppression for keyboard navigation (Issue #89) | |
| 55 | + | // Prevents hover focus from overriding keyboard-initiated focus changes | |
| 56 | + | hoverSuppressedUntil time.Time | |
| 57 | + | ||
| 53 | 58 | mu sync.RWMutex | |
| 54 | 59 | } | |
| 55 | 60 | ||
| @@ -86,6 +91,11 @@ func (a *paneViewFactoryAdapter) CreatePaneView(node *entity.PaneNode) layout.Wi | |||
| 86 | 91 | ||
| 87 | 92 | // Set up hover callback for focus-follows-mouse | |
| 88 | 93 | pv.SetOnHover(func(paneID entity.PaneID) { | |
| 94 | + | // Skip if hover is suppressed (keyboard navigation just happened) | |
| 95 | + | if a.wv.IsHoverSuppressed() { | |
| 96 | + | return | |
| 97 | + | } | |
| 98 | + | ||
| 89 | 99 | // Skip if this pane is already active | |
| 90 | 100 | if a.wv.GetActivePaneID() == paneID { | |
| 91 | 101 | return | |
| @@ -347,6 +357,18 @@ func (wv *WorkspaceView) FocusPane(paneID entity.PaneID) bool { | |||
| 347 | 357 | return pv.GrabFocus() | |
| 348 | 358 | } | |
| 349 | 359 | ||
| 360 | + | // DeactivatePane removes the active visual indicator from a pane. | |
| 361 | + | // This is used when keyboard navigation overrides mouse hover focus. | |
| 362 | + | func (wv *WorkspaceView) DeactivatePane(paneID entity.PaneID) { | |
| 363 | + | wv.mu.RLock() | |
| 364 | + | pv, ok := wv.paneViews[paneID] | |
| 365 | + | wv.mu.RUnlock() | |
| 366 | + | ||
| 367 | + | if ok && pv != nil { | |
| 368 | + | pv.SetActive(false) | |
| 369 | + | } | |
| 370 | + | } | |
| 371 | + | ||
| 350 | 372 | // SetWebViewWidget attaches a WebView widget to a specific pane. | |
| 351 | 373 | func (wv *WorkspaceView) SetWebViewWidget(paneID entity.PaneID, widget layout.Widget) error { | |
| 352 | 374 | wv.mu.RLock() | |
| @@ -755,3 +777,35 @@ func (wv *WorkspaceView) GetActivePaneView() *PaneView { | |||
| 755 | 777 | defer wv.mu.RUnlock() | |
| 756 | 778 | return wv.paneViews[wv.getActivePaneIDInternal()] | |
| 757 | 779 | } | |
| 780 | + | ||
| 781 | + | // KeyboardFocusSuppressDuration is the time to suppress hover focus after keyboard navigation. | |
| 782 | + | const KeyboardFocusSuppressDuration = 300 * time.Millisecond | |
| 783 | + | ||
| 784 | + | // SuppressHover temporarily blocks hover focus changes after keyboard navigation. | |
| 785 | + | // This prevents the race condition where a pending hover timer overrides | |
| 786 | + | // an explicit keyboard focus change (Issue #89). | |
| 787 | + | func (wv *WorkspaceView) SuppressHover(duration time.Duration) { | |
| 788 | + | wv.mu.Lock() | |
| 789 | + | wv.hoverSuppressedUntil = time.Now().Add(duration) | |
| 790 | + | wv.mu.Unlock() | |
| 791 | + | } | |
| 792 | + | ||
| 793 | + | // IsHoverSuppressed returns true if hover focus is temporarily suppressed. | |
| 794 | + | func (wv *WorkspaceView) IsHoverSuppressed() bool { | |
| 795 | + | wv.mu.RLock() | |
| 796 | + | defer wv.mu.RUnlock() | |
| 797 | + | return time.Now().Before(wv.hoverSuppressedUntil) | |
| 798 | + | } | |
| 799 | + | ||
| 800 | + | // CancelAllPendingHovers cancels any pending hover timers on all pane views. | |
| 801 | + | // Called when keyboard navigation occurs to ensure hover doesn't override keyboard focus. | |
| 802 | + | func (wv *WorkspaceView) CancelAllPendingHovers() { | |
| 803 | + | wv.mu.RLock() | |
| 804 | + | defer wv.mu.RUnlock() | |
| 805 | + | ||
| 806 | + | for _, pv := range wv.paneViews { | |
| 807 | + | if pv != nil { | |
| 808 | + | pv.CancelPendingHover() | |
| 809 | + | } | |
| 810 | + | } | |
| 811 | + | } | |
| @@ -6,6 +6,7 @@ import ( | |||||
| 6 | "errors" | 6 | "errors" | ||
| 7 | "fmt" | 7 | "fmt" | ||
| 8 | "sync" | 8 | "sync" | ||
| 9 | + | "time" | |||
| 9 | 10 | ||||
| 10 | "github.com/bnema/dumber/internal/domain/entity" | 11 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 11 | "github.com/bnema/dumber/internal/logging" | 12 | "github.com/bnema/dumber/internal/logging" | ||
| @@ -50,6 +51,10 @@ type WorkspaceView struct { | |||||
| 50 | onPaneFocused func(paneID entity.PaneID) | 51 | onPaneFocused func(paneID entity.PaneID) | ||
| 51 | onSplitRatioDragged func(nodeID string, ratio float64) | 52 | onSplitRatioDragged func(nodeID string, ratio float64) | ||
| 52 | 53 | ||||
| 54 | + | // Hover suppression for keyboard navigation (Issue #89) | |||
| 55 | + | // Prevents hover focus from overriding keyboard-initiated focus changes | |||
| 56 | + | hoverSuppressedUntil time.Time | |||
| 57 | + | ||||
| 53 | mu sync.RWMutex | 58 | mu sync.RWMutex | ||
| 54 | } | 59 | } | ||
| 55 | 60 | ||||
| @@ -86,6 +91,11 @@ func (a *paneViewFactoryAdapter) CreatePaneView(node *entity.PaneNode) layout.Wi | |||||
| 86 | 91 | ||||
| 87 | // Set up hover callback for focus-follows-mouse | 92 | // Set up hover callback for focus-follows-mouse | ||
| 88 | pv.SetOnHover(func(paneID entity.PaneID) { | 93 | pv.SetOnHover(func(paneID entity.PaneID) { | ||
| 94 | + | // Skip if hover is suppressed (keyboard navigation just happened) | |||
| 95 | + | if a.wv.IsHoverSuppressed() { | |||
| 96 | + | return | |||
| 97 | + | } | |||
| 98 | + | ||||
| 89 | // Skip if this pane is already active | 99 | // Skip if this pane is already active | ||
| 90 | if a.wv.GetActivePaneID() == paneID { | 100 | if a.wv.GetActivePaneID() == paneID { | ||
| 91 | return | 101 | return | ||
| @@ -347,6 +357,18 @@ func (wv *WorkspaceView) FocusPane(paneID entity.PaneID) bool { | |||||
| 347 | return pv.GrabFocus() | 357 | return pv.GrabFocus() | ||
| 348 | } | 358 | } | ||
| 349 | 359 | ||||
| 360 | + | // DeactivatePane removes the active visual indicator from a pane. | |||
| 361 | + | // This is used when keyboard navigation overrides mouse hover focus. | |||
| 362 | + | func (wv *WorkspaceView) DeactivatePane(paneID entity.PaneID) { | |||
| 363 | + | wv.mu.RLock() | |||
| 364 | + | pv, ok := wv.paneViews[paneID] | |||
| 365 | + | wv.mu.RUnlock() | |||
| 366 | + | ||||
| 367 | + | if ok && pv != nil { | |||
| 368 | + | pv.SetActive(false) | |||
| 369 | + | } | |||
| 370 | + | } | |||
| 371 | + | ||||
| 350 | // SetWebViewWidget attaches a WebView widget to a specific pane. | 372 | // SetWebViewWidget attaches a WebView widget to a specific pane. | ||
| 351 | func (wv *WorkspaceView) SetWebViewWidget(paneID entity.PaneID, widget layout.Widget) error { | 373 | func (wv *WorkspaceView) SetWebViewWidget(paneID entity.PaneID, widget layout.Widget) error { | ||
| 352 | wv.mu.RLock() | 374 | wv.mu.RLock() | ||
| @@ -755,3 +777,35 @@ func (wv *WorkspaceView) GetActivePaneView() *PaneView { | |||||
| 755 | defer wv.mu.RUnlock() | 777 | defer wv.mu.RUnlock() | ||
| 756 | return wv.paneViews[wv.getActivePaneIDInternal()] | 778 | return wv.paneViews[wv.getActivePaneIDInternal()] | ||
| 757 | } | 779 | } | ||
| 780 | + | ||||
| 781 | + | // KeyboardFocusSuppressDuration is the time to suppress hover focus after keyboard navigation. | |||
| 782 | + | const KeyboardFocusSuppressDuration = 300 * time.Millisecond | |||
| 783 | + | ||||
| 784 | + | // SuppressHover temporarily blocks hover focus changes after keyboard navigation. | |||
| 785 | + | // This prevents the race condition where a pending hover timer overrides | |||
| 786 | + | // an explicit keyboard focus change (Issue #89). | |||
| 787 | + | func (wv *WorkspaceView) SuppressHover(duration time.Duration) { | |||
| 788 | + | wv.mu.Lock() | |||
| 789 | + | wv.hoverSuppressedUntil = time.Now().Add(duration) | |||
| 790 | + | wv.mu.Unlock() | |||
| 791 | + | } | |||
| 792 | + | ||||
| 793 | + | // IsHoverSuppressed returns true if hover focus is temporarily suppressed. | |||
| 794 | + | func (wv *WorkspaceView) IsHoverSuppressed() bool { | |||
| 795 | + | wv.mu.RLock() | |||
| 796 | + | defer wv.mu.RUnlock() | |||
| 797 | + | return time.Now().Before(wv.hoverSuppressedUntil) | |||
| 798 | + | } | |||
| 799 | + | ||||
| 800 | + | // CancelAllPendingHovers cancels any pending hover timers on all pane views. | |||
| 801 | + | // Called when keyboard navigation occurs to ensure hover doesn't override keyboard focus. | |||
| 802 | + | func (wv *WorkspaceView) CancelAllPendingHovers() { | |||
| 803 | + | wv.mu.RLock() | |||
| 804 | + | defer wv.mu.RUnlock() | |||
| 805 | + | ||||
| 806 | + | for _, pv := range wv.paneViews { | |||
| 807 | + | if pv != nil { | |||
| 808 | + | pv.CancelPendingHover() | |||
| 809 | + | } | |||
| 810 | + | } | |||
| 811 | + | } | |||
M internal/ui/component/workspace_view_test.go +14 -0
| @@ -129,6 +129,7 @@ func setupStackedLeafMocks( | |||
| 129 | 129 | mockFavicon := mocks.NewMockImageWidget(t) | |
| 130 | 130 | mockLabel := mocks.NewMockLabelWidget(t) | |
| 131 | 131 | mockButton := mocks.NewMockButtonWidget(t) | |
| 132 | + | mockCloseButton := mocks.NewMockButtonWidget(t) | |
| 132 | 133 | ||
| 133 | 134 | mockFactory.EXPECT().NewBox(layout.OrientationVertical, 0).Return(mockStackBox).Once() | |
| 134 | 135 | mockStackBox.EXPECT().SetHexpand(true).Once() | |
| @@ -150,13 +151,26 @@ func setupStackedLeafMocks( | |||
| 150 | 151 | mockLabel.EXPECT().SetXalign(float32(0.0)).Once() | |
| 151 | 152 | mockTitleBar.EXPECT().Append(mockLabel).Once() | |
| 152 | 153 | ||
| 154 | + | // Close button (uses SetIconName directly instead of child image) | |
| 155 | + | mockFactory.EXPECT().NewButton().Return(mockCloseButton).Once() | |
| 156 | + | mockCloseButton.EXPECT().SetIconName("window-close-symbolic").Once() | |
| 157 | + | mockCloseButton.EXPECT().AddCssClass("stacked-pane-close-button").Once() | |
| 158 | + | mockCloseButton.EXPECT().SetFocusOnClick(false).Once() | |
| 159 | + | mockCloseButton.EXPECT().SetVexpand(false).Once() | |
| 160 | + | mockCloseButton.EXPECT().SetHexpand(false).Once() | |
| 161 | + | mockTitleBar.EXPECT().Append(mockCloseButton).Once() | |
| 162 | + | ||
| 163 | + | // Title bar button | |
| 153 | 164 | mockFactory.EXPECT().NewButton().Return(mockButton).Once() | |
| 154 | 165 | mockButton.EXPECT().SetChild(mockTitleBar).Once() | |
| 155 | 166 | mockButton.EXPECT().AddCssClass("stacked-pane-title-button").Once() | |
| 156 | 167 | mockButton.EXPECT().SetFocusOnClick(false).Once() | |
| 157 | 168 | mockButton.EXPECT().SetVexpand(false).Once() | |
| 158 | 169 | mockButton.EXPECT().SetHexpand(true).Once() | |
| 170 | + | ||
| 171 | + | // Click handlers | |
| 159 | 172 | mockButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(1)).Once() | |
| 173 | + | mockCloseButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(2)).Once() | |
| 160 | 174 | ||
| 161 | 175 | mockStackBox.EXPECT().Append(mockButton).Once() | |
| 162 | 176 | mockStackBox.EXPECT().Append(container).Once() | |
| @@ -129,6 +129,7 @@ func setupStackedLeafMocks( | |||||
| 129 | mockFavicon := mocks.NewMockImageWidget(t) | 129 | mockFavicon := mocks.NewMockImageWidget(t) | ||
| 130 | mockLabel := mocks.NewMockLabelWidget(t) | 130 | mockLabel := mocks.NewMockLabelWidget(t) | ||
| 131 | mockButton := mocks.NewMockButtonWidget(t) | 131 | mockButton := mocks.NewMockButtonWidget(t) | ||
| 132 | + | mockCloseButton := mocks.NewMockButtonWidget(t) | |||
| 132 | 133 | ||||
| 133 | mockFactory.EXPECT().NewBox(layout.OrientationVertical, 0).Return(mockStackBox).Once() | 134 | mockFactory.EXPECT().NewBox(layout.OrientationVertical, 0).Return(mockStackBox).Once() | ||
| 134 | mockStackBox.EXPECT().SetHexpand(true).Once() | 135 | mockStackBox.EXPECT().SetHexpand(true).Once() | ||
| @@ -150,13 +151,26 @@ func setupStackedLeafMocks( | |||||
| 150 | mockLabel.EXPECT().SetXalign(float32(0.0)).Once() | 151 | mockLabel.EXPECT().SetXalign(float32(0.0)).Once() | ||
| 151 | mockTitleBar.EXPECT().Append(mockLabel).Once() | 152 | mockTitleBar.EXPECT().Append(mockLabel).Once() | ||
| 152 | 153 | ||||
| 154 | + | // Close button (uses SetIconName directly instead of child image) | |||
| 155 | + | mockFactory.EXPECT().NewButton().Return(mockCloseButton).Once() | |||
| 156 | + | mockCloseButton.EXPECT().SetIconName("window-close-symbolic").Once() | |||
| 157 | + | mockCloseButton.EXPECT().AddCssClass("stacked-pane-close-button").Once() | |||
| 158 | + | mockCloseButton.EXPECT().SetFocusOnClick(false).Once() | |||
| 159 | + | mockCloseButton.EXPECT().SetVexpand(false).Once() | |||
| 160 | + | mockCloseButton.EXPECT().SetHexpand(false).Once() | |||
| 161 | + | mockTitleBar.EXPECT().Append(mockCloseButton).Once() | |||
| 162 | + | ||||
| 163 | + | // Title bar button | |||
| 153 | mockFactory.EXPECT().NewButton().Return(mockButton).Once() | 164 | mockFactory.EXPECT().NewButton().Return(mockButton).Once() | ||
| 154 | mockButton.EXPECT().SetChild(mockTitleBar).Once() | 165 | mockButton.EXPECT().SetChild(mockTitleBar).Once() | ||
| 155 | mockButton.EXPECT().AddCssClass("stacked-pane-title-button").Once() | 166 | mockButton.EXPECT().AddCssClass("stacked-pane-title-button").Once() | ||
| 156 | mockButton.EXPECT().SetFocusOnClick(false).Once() | 167 | mockButton.EXPECT().SetFocusOnClick(false).Once() | ||
| 157 | mockButton.EXPECT().SetVexpand(false).Once() | 168 | mockButton.EXPECT().SetVexpand(false).Once() | ||
| 158 | mockButton.EXPECT().SetHexpand(true).Once() | 169 | mockButton.EXPECT().SetHexpand(true).Once() | ||
| 170 | + | ||||
| 171 | + | // Click handlers | |||
| 159 | mockButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(1)).Once() | 172 | mockButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(1)).Once() | ||
| 173 | + | mockCloseButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(2)).Once() | |||
| 160 | 174 | ||||
| 161 | mockStackBox.EXPECT().Append(mockButton).Once() | 175 | mockStackBox.EXPECT().Append(mockButton).Once() | ||
| 162 | mockStackBox.EXPECT().Append(container).Once() | 176 | mockStackBox.EXPECT().Append(container).Once() | ||
M internal/ui/coordinator/workspace.go +53 -16
| @@ -179,12 +179,14 @@ func (c *WorkspaceCoordinator) prepareSplit( | |||
| 179 | 179 | return nil, false | |
| 180 | 180 | } | |
| 181 | 181 | ||
| 182 | - | isStackSplit := (direction == usecase.SplitLeft || direction == usecase.SplitRight) && | |
| 183 | - | activePane.Parent != nil && activePane.Parent.IsStacked | |
| 182 | + | // Check if active pane is inside a stack | |
| 183 | + | isInStack := activePane.Parent != nil && activePane.Parent.IsStacked | |
| 184 | + | // isStackSplit is only true for horizontal splits - these use a special code path | |
| 185 | + | isStackSplit := (direction == usecase.SplitLeft || direction == usecase.SplitRight) && isInStack | |
| 184 | 186 | ||
| 185 | 187 | var existingWidget layout.Widget | |
| 186 | 188 | if wsView != nil { | |
| 187 | - | existingWidget = c.resolveSplitWidget(wsView, activePane, isStackSplit) | |
| 189 | + | existingWidget = c.resolveSplitWidget(wsView, activePane, isInStack) | |
| 188 | 190 | } | |
| 189 | 191 | ||
| 190 | 192 | return &splitContext{ | |
| @@ -199,27 +201,29 @@ func (c *WorkspaceCoordinator) prepareSplit( | |||
| 199 | 201 | func (c *WorkspaceCoordinator) resolveSplitWidget( | |
| 200 | 202 | wsView *component.WorkspaceView, | |
| 201 | 203 | activePane *entity.PaneNode, | |
| 202 | - | isStackSplit bool, | |
| 204 | + | isInStack bool, | |
| 203 | 205 | ) layout.Widget { | |
| 204 | 206 | if wsView == nil { | |
| 205 | 207 | return nil | |
| 206 | 208 | } | |
| 207 | 209 | ||
| 208 | - | if !isStackSplit { | |
| 209 | - | return wsView.GetRootWidget() | |
| 210 | - | } | |
| 210 | + | // When splitting from inside a stack (any direction), we need the stack widget | |
| 211 | + | // because the domain model promotes the split to happen around the stack container | |
| 212 | + | if isInStack { | |
| 213 | + | tr := wsView.TreeRenderer() | |
| 214 | + | if tr == nil { | |
| 215 | + | return nil | |
| 216 | + | } | |
| 211 | 217 | ||
| 212 | - | tr := wsView.TreeRenderer() | |
| 213 | - | if tr == nil { | |
| 214 | - | return nil | |
| 215 | - | } | |
| 218 | + | stackedView := tr.GetStackedViewForPane(string(activePane.Pane.ID)) | |
| 219 | + | if stackedView == nil { | |
| 220 | + | return nil | |
| 221 | + | } | |
| 216 | 222 | ||
| 217 | - | stackedView := tr.GetStackedViewForPane(string(activePane.Pane.ID)) | |
| 218 | - | if stackedView == nil { | |
| 219 | - | return nil | |
| 223 | + | return stackedView.Widget() | |
| 220 | 224 | } | |
| 221 | 225 | ||
| 222 | - | return stackedView.Widget() | |
| 226 | + | return wsView.GetRootWidget() | |
| 223 | 227 | } | |
| 224 | 228 | ||
| 225 | 229 | func (c *WorkspaceCoordinator) applySplitToView( | |
| @@ -1137,6 +1141,9 @@ func (c *WorkspaceCoordinator) FocusPane(ctx context.Context, direction usecase. | |||
| 1137 | 1141 | return nil | |
| 1138 | 1142 | } | |
| 1139 | 1143 | ||
| 1144 | + | // Save old active pane ID before navigation updates the domain model | |
| 1145 | + | oldActivePaneID := ws.ActivePaneID | |
| 1146 | + | ||
| 1140 | 1147 | // Use geometric navigation if focus manager is available | |
| 1141 | 1148 | var newPane *entity.PaneNode | |
| 1142 | 1149 | var err error | |
| @@ -1161,6 +1168,15 @@ func (c *WorkspaceCoordinator) FocusPane(ctx context.Context, direction usecase. | |||
| 1161 | 1168 | return nil | |
| 1162 | 1169 | } | |
| 1163 | 1170 | ||
| 1171 | + | // Cancel pending hover timers and suppress future hover focus (Issue #89) | |
| 1172 | + | wsView.CancelAllPendingHovers() | |
| 1173 | + | wsView.SuppressHover(component.KeyboardFocusSuppressDuration) | |
| 1174 | + | ||
| 1175 | + | // Deactivate old pane explicitly (domain model already updated by NavigateGeometric) | |
| 1176 | + | if oldActivePaneID != "" && oldActivePaneID != newPane.Pane.ID { | |
| 1177 | + | wsView.DeactivatePane(oldActivePaneID) | |
| 1178 | + | } | |
| 1179 | + | ||
| 1164 | 1180 | // Update the workspace view's active pane | |
| 1165 | 1181 | if err := wsView.SetActivePaneID(newPane.Pane.ID); err != nil { | |
| 1166 | 1182 | log.Warn().Err(err).Msg("failed to update active pane in view") | |
| @@ -1277,7 +1293,7 @@ func (c *WorkspaceCoordinator) StackPane(ctx context.Context) error { | |||
| 1277 | 1293 | stackCtx.wsView.ShowOmnibox(ctx, "") | |
| 1278 | 1294 | } | |
| 1279 | 1295 | ||
| 1280 | - | // Set up title bar click callback | |
| 1296 | + | // Set up title bar click and close callbacks | |
| 1281 | 1297 | tr := stackCtx.wsView.TreeRenderer() | |
| 1282 | 1298 | if tr != nil { | |
| 1283 | 1299 | stackedView := tr.GetStackedViewForPane(string(stackCtx.activePaneID)) | |
| @@ -1286,6 +1302,9 @@ func (c *WorkspaceCoordinator) StackPane(ctx context.Context) error { | |||
| 1286 | 1302 | stackedView.SetOnActivate(func(index int) { | |
| 1287 | 1303 | c.onTitleBarClick(ctx, capturedStackNode, stackedView, index) | |
| 1288 | 1304 | }) | |
| 1305 | + | stackedView.SetOnClosePane(func(paneID string) { | |
| 1306 | + | c.onStackedPaneClose(ctx, entity.PaneID(paneID)) | |
| 1307 | + | }) | |
| 1289 | 1308 | } | |
| 1290 | 1309 | } | |
| 1291 | 1310 | ||
| @@ -1531,6 +1550,10 @@ func (c *WorkspaceCoordinator) onTitleBarClick(ctx context.Context, stackNode *e | |||
| 1531 | 1550 | // Update workspace view (also updates domain model) | |
| 1532 | 1551 | _, wsView := c.getActiveWS() | |
| 1533 | 1552 | if wsView != nil { | |
| 1553 | + | // Cancel pending hover timers and suppress future hover focus (Issue #89) | |
| 1554 | + | wsView.CancelAllPendingHovers() | |
| 1555 | + | wsView.SuppressHover(component.KeyboardFocusSuppressDuration) | |
| 1556 | + | ||
| 1534 | 1557 | if err := wsView.SetActivePaneID(clickedPaneID); err != nil { | |
| 1535 | 1558 | log.Warn().Err(err).Msg("failed to set active pane in workspace view") | |
| 1536 | 1559 | } | |
| @@ -1543,6 +1566,17 @@ func (c *WorkspaceCoordinator) onTitleBarClick(ctx context.Context, stackNode *e | |||
| 1543 | 1566 | Msg("switched active pane via title bar click") | |
| 1544 | 1567 | } | |
| 1545 | 1568 | ||
| 1569 | + | // onStackedPaneClose handles close button clicks on stacked pane title bars. | |
| 1570 | + | func (c *WorkspaceCoordinator) onStackedPaneClose(ctx context.Context, paneID entity.PaneID) { | |
| 1571 | + | log := logging.FromContext(ctx) | |
| 1572 | + | ||
| 1573 | + | log.Debug().Str("pane_id", string(paneID)).Msg("closing stacked pane via close button") | |
| 1574 | + | ||
| 1575 | + | if err := c.ClosePaneByID(ctx, paneID); err != nil { | |
| 1576 | + | log.Error().Err(err).Str("pane_id", string(paneID)).Msg("failed to close stacked pane") | |
| 1577 | + | } | |
| 1578 | + | } | |
| 1579 | + | ||
| 1546 | 1580 | // --- Popup Insertion --- | |
| 1547 | 1581 | ||
| 1548 | 1582 | // SetOnCreatePopupTab sets the callback for creating popup tabs. | |
| @@ -1836,6 +1870,9 @@ func (c *WorkspaceCoordinator) attachPopupPaneView( | |||
| 1836 | 1870 | stackedView.SetOnActivate(func(index int) { | |
| 1837 | 1871 | c.onTitleBarClick(ctx, capturedStackNode, stackedView, index) | |
| 1838 | 1872 | }) | |
| 1873 | + | stackedView.SetOnClosePane(func(paneID string) { | |
| 1874 | + | c.onStackedPaneClose(ctx, entity.PaneID(paneID)) | |
| 1875 | + | }) | |
| 1839 | 1876 | } | |
| 1840 | 1877 | } | |
| 1841 | 1878 | return nil | |
| @@ -179,12 +179,14 @@ func (c *WorkspaceCoordinator) prepareSplit( | |||||
| 179 | return nil, false | 179 | return nil, false | ||
| 180 | } | 180 | } | ||
| 181 | 181 | ||||
| 182 | - | isStackSplit := (direction == usecase.SplitLeft || direction == usecase.SplitRight) && | 182 | + | // Check if active pane is inside a stack |
| 183 | - | activePane.Parent != nil && activePane.Parent.IsStacked | 183 | + | isInStack := activePane.Parent != nil && activePane.Parent.IsStacked |
| 184 | + | // isStackSplit is only true for horizontal splits - these use a special code path | |||
| 185 | + | isStackSplit := (direction == usecase.SplitLeft || direction == usecase.SplitRight) && isInStack | |||
| 184 | 186 | ||||
| 185 | var existingWidget layout.Widget | 187 | var existingWidget layout.Widget | ||
| 186 | if wsView != nil { | 188 | if wsView != nil { | ||
| 187 | - | existingWidget = c.resolveSplitWidget(wsView, activePane, isStackSplit) | 189 | + | existingWidget = c.resolveSplitWidget(wsView, activePane, isInStack) |
| 188 | } | 190 | } | ||
| 189 | 191 | ||||
| 190 | return &splitContext{ | 192 | return &splitContext{ | ||
| @@ -199,27 +201,29 @@ func (c *WorkspaceCoordinator) prepareSplit( | |||||
| 199 | func (c *WorkspaceCoordinator) resolveSplitWidget( | 201 | func (c *WorkspaceCoordinator) resolveSplitWidget( | ||
| 200 | wsView *component.WorkspaceView, | 202 | wsView *component.WorkspaceView, | ||
| 201 | activePane *entity.PaneNode, | 203 | activePane *entity.PaneNode, | ||
| 202 | - | isStackSplit bool, | 204 | + | isInStack bool, |
| 203 | ) layout.Widget { | 205 | ) layout.Widget { | ||
| 204 | if wsView == nil { | 206 | if wsView == nil { | ||
| 205 | return nil | 207 | return nil | ||
| 206 | } | 208 | } | ||
| 207 | 209 | ||||
| 208 | - | if !isStackSplit { | 210 | + | // When splitting from inside a stack (any direction), we need the stack widget |
| 209 | - | return wsView.GetRootWidget() | 211 | + | // because the domain model promotes the split to happen around the stack container |
| 210 | - | } | 212 | + | if isInStack { |
| 213 | + | tr := wsView.TreeRenderer() | |||
| 214 | + | if tr == nil { | |||
| 215 | + | return nil | |||
| 216 | + | } | |||
| 211 | 217 | ||||
| 212 | - | tr := wsView.TreeRenderer() | 218 | + | stackedView := tr.GetStackedViewForPane(string(activePane.Pane.ID)) |
| 213 | - | if tr == nil { | 219 | + | if stackedView == nil { |
| 214 | - | return nil | 220 | + | return nil |
| 215 | - | } | 221 | + | } |
| 216 | 222 | ||||
| 217 | - | stackedView := tr.GetStackedViewForPane(string(activePane.Pane.ID)) | 223 | + | return stackedView.Widget() |
| 218 | - | if stackedView == nil { | |||
| 219 | - | return nil | |||
| 220 | } | 224 | } | ||
| 221 | 225 | ||||
| 222 | - | return stackedView.Widget() | 226 | + | return wsView.GetRootWidget() |
| 223 | } | 227 | } | ||
| 224 | 228 | ||||
| 225 | func (c *WorkspaceCoordinator) applySplitToView( | 229 | func (c *WorkspaceCoordinator) applySplitToView( | ||
| @@ -1137,6 +1141,9 @@ func (c *WorkspaceCoordinator) FocusPane(ctx context.Context, direction usecase. | |||||
| 1137 | return nil | 1141 | return nil | ||
| 1138 | } | 1142 | } | ||
| 1139 | 1143 | ||||
| 1144 | + | // Save old active pane ID before navigation updates the domain model | |||
| 1145 | + | oldActivePaneID := ws.ActivePaneID | |||
| 1146 | + | ||||
| 1140 | // Use geometric navigation if focus manager is available | 1147 | // Use geometric navigation if focus manager is available | ||
| 1141 | var newPane *entity.PaneNode | 1148 | var newPane *entity.PaneNode | ||
| 1142 | var err error | 1149 | var err error | ||
| @@ -1161,6 +1168,15 @@ func (c *WorkspaceCoordinator) FocusPane(ctx context.Context, direction usecase. | |||||
| 1161 | return nil | 1168 | return nil | ||
| 1162 | } | 1169 | } | ||
| 1163 | 1170 | ||||
| 1171 | + | // Cancel pending hover timers and suppress future hover focus (Issue #89) | |||
| 1172 | + | wsView.CancelAllPendingHovers() | |||
| 1173 | + | wsView.SuppressHover(component.KeyboardFocusSuppressDuration) | |||
| 1174 | + | ||||
| 1175 | + | // Deactivate old pane explicitly (domain model already updated by NavigateGeometric) | |||
| 1176 | + | if oldActivePaneID != "" && oldActivePaneID != newPane.Pane.ID { | |||
| 1177 | + | wsView.DeactivatePane(oldActivePaneID) | |||
| 1178 | + | } | |||
| 1179 | + | ||||
| 1164 | // Update the workspace view's active pane | 1180 | // Update the workspace view's active pane | ||
| 1165 | if err := wsView.SetActivePaneID(newPane.Pane.ID); err != nil { | 1181 | if err := wsView.SetActivePaneID(newPane.Pane.ID); err != nil { | ||
| 1166 | log.Warn().Err(err).Msg("failed to update active pane in view") | 1182 | log.Warn().Err(err).Msg("failed to update active pane in view") | ||
| @@ -1277,7 +1293,7 @@ func (c *WorkspaceCoordinator) StackPane(ctx context.Context) error { | |||||
| 1277 | stackCtx.wsView.ShowOmnibox(ctx, "") | 1293 | stackCtx.wsView.ShowOmnibox(ctx, "") | ||
| 1278 | } | 1294 | } | ||
| 1279 | 1295 | ||||
| 1280 | - | // Set up title bar click callback | 1296 | + | // Set up title bar click and close callbacks |
| 1281 | tr := stackCtx.wsView.TreeRenderer() | 1297 | tr := stackCtx.wsView.TreeRenderer() | ||
| 1282 | if tr != nil { | 1298 | if tr != nil { | ||
| 1283 | stackedView := tr.GetStackedViewForPane(string(stackCtx.activePaneID)) | 1299 | stackedView := tr.GetStackedViewForPane(string(stackCtx.activePaneID)) | ||
| @@ -1286,6 +1302,9 @@ func (c *WorkspaceCoordinator) StackPane(ctx context.Context) error { | |||||
| 1286 | stackedView.SetOnActivate(func(index int) { | 1302 | stackedView.SetOnActivate(func(index int) { | ||
| 1287 | c.onTitleBarClick(ctx, capturedStackNode, stackedView, index) | 1303 | c.onTitleBarClick(ctx, capturedStackNode, stackedView, index) | ||
| 1288 | }) | 1304 | }) | ||
| 1305 | + | stackedView.SetOnClosePane(func(paneID string) { | |||
| 1306 | + | c.onStackedPaneClose(ctx, entity.PaneID(paneID)) | |||
| 1307 | + | }) | |||
| 1289 | } | 1308 | } | ||
| 1290 | } | 1309 | } | ||
| 1291 | 1310 | ||||
| @@ -1531,6 +1550,10 @@ func (c *WorkspaceCoordinator) onTitleBarClick(ctx context.Context, stackNode *e | |||||
| 1531 | // Update workspace view (also updates domain model) | 1550 | // Update workspace view (also updates domain model) | ||
| 1532 | _, wsView := c.getActiveWS() | 1551 | _, wsView := c.getActiveWS() | ||
| 1533 | if wsView != nil { | 1552 | if wsView != nil { | ||
| 1553 | + | // Cancel pending hover timers and suppress future hover focus (Issue #89) | |||
| 1554 | + | wsView.CancelAllPendingHovers() | |||
| 1555 | + | wsView.SuppressHover(component.KeyboardFocusSuppressDuration) | |||
| 1556 | + | ||||
| 1534 | if err := wsView.SetActivePaneID(clickedPaneID); err != nil { | 1557 | if err := wsView.SetActivePaneID(clickedPaneID); err != nil { | ||
| 1535 | log.Warn().Err(err).Msg("failed to set active pane in workspace view") | 1558 | log.Warn().Err(err).Msg("failed to set active pane in workspace view") | ||
| 1536 | } | 1559 | } | ||
| @@ -1543,6 +1566,17 @@ func (c *WorkspaceCoordinator) onTitleBarClick(ctx context.Context, stackNode *e | |||||
| 1543 | Msg("switched active pane via title bar click") | 1566 | Msg("switched active pane via title bar click") | ||
| 1544 | } | 1567 | } | ||
| 1545 | 1568 | ||||
| 1569 | + | // onStackedPaneClose handles close button clicks on stacked pane title bars. | |||
| 1570 | + | func (c *WorkspaceCoordinator) onStackedPaneClose(ctx context.Context, paneID entity.PaneID) { | |||
| 1571 | + | log := logging.FromContext(ctx) | |||
| 1572 | + | ||||
| 1573 | + | log.Debug().Str("pane_id", string(paneID)).Msg("closing stacked pane via close button") | |||
| 1574 | + | ||||
| 1575 | + | if err := c.ClosePaneByID(ctx, paneID); err != nil { | |||
| 1576 | + | log.Error().Err(err).Str("pane_id", string(paneID)).Msg("failed to close stacked pane") | |||
| 1577 | + | } | |||
| 1578 | + | } | |||
| 1579 | + | ||||
| 1546 | // --- Popup Insertion --- | 1580 | // --- Popup Insertion --- | ||
| 1547 | 1581 | ||||
| 1548 | // SetOnCreatePopupTab sets the callback for creating popup tabs. | 1582 | // SetOnCreatePopupTab sets the callback for creating popup tabs. | ||
| @@ -1836,6 +1870,9 @@ func (c *WorkspaceCoordinator) attachPopupPaneView( | |||||
| 1836 | stackedView.SetOnActivate(func(index int) { | 1870 | stackedView.SetOnActivate(func(index int) { | ||
| 1837 | c.onTitleBarClick(ctx, capturedStackNode, stackedView, index) | 1871 | c.onTitleBarClick(ctx, capturedStackNode, stackedView, index) | ||
| 1838 | }) | 1872 | }) | ||
| 1873 | + | stackedView.SetOnClosePane(func(paneID string) { | |||
| 1874 | + | c.onStackedPaneClose(ctx, entity.PaneID(paneID)) | |||
| 1875 | + | }) | |||
| 1839 | } | 1876 | } | ||
| 1840 | } | 1877 | } | ||
| 1841 | return nil | 1878 | return nil | ||
M internal/ui/focus/manager.go +35 -10
| @@ -38,16 +38,24 @@ func NewManager(panesUC *usecase.ManagePanesUseCase) *Manager { | |||
| 38 | 38 | // For panes in stacks, uses the stack container's geometry since individual | |
| 39 | 39 | // collapsed panes have no allocated size. | |
| 40 | 40 | // Only visible panes are included - for stacked panes, only the active one is visible. | |
| 41 | - | func (m *Manager) CollectPaneRects(provider PaneGeometryProvider) []entity.PaneRect { | |
| 41 | + | func (m *Manager) CollectPaneRects(ctx context.Context, provider PaneGeometryProvider) []entity.PaneRect { | |
| 42 | + | log := logging.FromContext(ctx) | |
| 42 | 43 | var rects []entity.PaneRect | |
| 43 | 44 | ||
| 44 | 45 | container := provider.ContainerWidget() | |
| 45 | 46 | ||
| 46 | - | for _, paneID := range provider.GetPaneIDs() { | |
| 47 | + | allPaneIDs := provider.GetPaneIDs() | |
| 48 | + | log.Debug().Int("total_panes", len(allPaneIDs)).Msg("CollectPaneRects: starting") | |
| 49 | + | ||
| 50 | + | for _, paneID := range allPaneIDs { | |
| 47 | 51 | paneWidget := provider.GetPaneWidget(paneID) | |
| 48 | 52 | ||
| 49 | 53 | // Skip panes that aren't visible (includes inactive stacked panes) | |
| 50 | 54 | if paneWidget == nil || !paneWidget.IsVisible() { | |
| 55 | + | log.Debug(). | |
| 56 | + | Str("pane_id", string(paneID)). | |
| 57 | + | Bool("widget_nil", paneWidget == nil). | |
| 58 | + | Msg("CollectPaneRects: skipping invisible pane") | |
| 51 | 59 | continue | |
| 52 | 60 | } | |
| 53 | 61 | ||
| @@ -77,6 +85,14 @@ func (m *Manager) CollectPaneRects(provider PaneGeometryProvider) []entity.PaneR | |||
| 77 | 85 | continue | |
| 78 | 86 | } | |
| 79 | 87 | ||
| 88 | + | log.Debug(). | |
| 89 | + | Str("pane_id", string(paneID)). | |
| 90 | + | Int("x", int(x)). | |
| 91 | + | Int("y", int(y)). | |
| 92 | + | Int("w", w). | |
| 93 | + | Int("h", h). | |
| 94 | + | Msg("CollectPaneRects: added visible pane") | |
| 95 | + | ||
| 80 | 96 | rects = append(rects, entity.PaneRect{ | |
| 81 | 97 | PaneID: paneID, | |
| 82 | 98 | X: int(x), | |
| @@ -86,6 +102,8 @@ func (m *Manager) CollectPaneRects(provider PaneGeometryProvider) []entity.PaneR | |||
| 86 | 102 | }) | |
| 87 | 103 | } | |
| 88 | 104 | ||
| 105 | + | log.Debug().Int("visible_panes", len(rects)).Msg("CollectPaneRects: done") | |
| 106 | + | ||
| 89 | 107 | return rects | |
| 90 | 108 | } | |
| 91 | 109 | ||
| @@ -119,8 +137,12 @@ func (m *Manager) NavigateGeometric( | |||
| 119 | 137 | stackNode := activeNode.Parent | |
| 120 | 138 | canNavigate, newPaneID := m.navigateWithinStack(stackNode, activeNode, direction) | |
| 121 | 139 | if canNavigate { | |
| 122 | - | ws.ActivePaneID = newPaneID | |
| 123 | - | return ws.FindPane(newPaneID), nil | |
| 140 | + | // Apply focus change through use case (handles stack index updates) | |
| 141 | + | targetNode, err := m.panesUC.ApplyFocusChange(ctx, ws, newPaneID) | |
| 142 | + | if err != nil { | |
| 143 | + | return nil, err | |
| 144 | + | } | |
| 145 | + | return targetNode, nil | |
| 124 | 146 | } | |
| 125 | 147 | // At stack boundary - fall through to geometric navigation | |
| 126 | 148 | log.Debug().Msg("at stack boundary, using geometric navigation") | |
| @@ -128,7 +150,7 @@ func (m *Manager) NavigateGeometric( | |||
| 128 | 150 | } | |
| 129 | 151 | ||
| 130 | 152 | // Collect geometry from visible panes | |
| 131 | - | rects := m.CollectPaneRects(provider) | |
| 153 | + | rects := m.CollectPaneRects(ctx, provider) | |
| 132 | 154 | if len(rects) == 0 { | |
| 133 | 155 | return nil, nil | |
| 134 | 156 | } | |
| @@ -146,16 +168,20 @@ func (m *Manager) NavigateGeometric( | |||
| 146 | 168 | return nil, nil | |
| 147 | 169 | } | |
| 148 | 170 | ||
| 149 | - | // Update workspace | |
| 150 | - | ws.ActivePaneID = output.TargetPaneID | |
| 171 | + | // Apply focus change through use case (handles stack index updates) | |
| 172 | + | targetNode, err := m.panesUC.ApplyFocusChange(ctx, ws, output.TargetPaneID) | |
| 173 | + | if err != nil { | |
| 174 | + | return nil, err | |
| 175 | + | } | |
| 151 | 176 | ||
| 152 | - | // Return target node | |
| 153 | - | return ws.FindPane(output.TargetPaneID), nil | |
| 177 | + | return targetNode, nil | |
| 154 | 178 | } | |
| 155 | 179 | ||
| 156 | 180 | // navigateWithinStack tries to navigate within a stack. | |
| 157 | 181 | // Returns (canNavigate, newPaneID). If canNavigate is false, the pane is at a | |
| 158 | 182 | // boundary and navigation should escape the stack. | |
| 183 | + | // Note: This method does not modify domain state; the caller should use | |
| 184 | + | // ApplyFocusChange to apply the focus change. | |
| 159 | 185 | func (m *Manager) navigateWithinStack( | |
| 160 | 186 | stackNode, currentNode *entity.PaneNode, direction usecase.NavigateDirection, | |
| 161 | 187 | ) (bool, entity.PaneID) { | |
| @@ -173,7 +199,6 @@ func (m *Manager) navigateWithinStack( | |||
| 173 | 199 | return false, "" | |
| 174 | 200 | } | |
| 175 | 201 | ||
| 176 | - | stackNode.ActiveStackIndex = newIdx | |
| 177 | 202 | return m.getPaneIDFromStackChild(stackNode.Children[newIdx]) | |
| 178 | 203 | } | |
| 179 | 204 | ||
| @@ -38,16 +38,24 @@ func NewManager(panesUC *usecase.ManagePanesUseCase) *Manager { | |||||
| 38 | // For panes in stacks, uses the stack container's geometry since individual | 38 | // For panes in stacks, uses the stack container's geometry since individual | ||
| 39 | // collapsed panes have no allocated size. | 39 | // collapsed panes have no allocated size. | ||
| 40 | // Only visible panes are included - for stacked panes, only the active one is visible. | 40 | // Only visible panes are included - for stacked panes, only the active one is visible. | ||
| 41 | - | func (m *Manager) CollectPaneRects(provider PaneGeometryProvider) []entity.PaneRect { | 41 | + | func (m *Manager) CollectPaneRects(ctx context.Context, provider PaneGeometryProvider) []entity.PaneRect { |
| 42 | + | log := logging.FromContext(ctx) | |||
| 42 | var rects []entity.PaneRect | 43 | var rects []entity.PaneRect | ||
| 43 | 44 | ||||
| 44 | container := provider.ContainerWidget() | 45 | container := provider.ContainerWidget() | ||
| 45 | 46 | ||||
| 46 | - | for _, paneID := range provider.GetPaneIDs() { | 47 | + | allPaneIDs := provider.GetPaneIDs() |
| 48 | + | log.Debug().Int("total_panes", len(allPaneIDs)).Msg("CollectPaneRects: starting") | |||
| 49 | + | ||||
| 50 | + | for _, paneID := range allPaneIDs { | |||
| 47 | paneWidget := provider.GetPaneWidget(paneID) | 51 | paneWidget := provider.GetPaneWidget(paneID) | ||
| 48 | 52 | ||||
| 49 | // Skip panes that aren't visible (includes inactive stacked panes) | 53 | // Skip panes that aren't visible (includes inactive stacked panes) | ||
| 50 | if paneWidget == nil || !paneWidget.IsVisible() { | 54 | if paneWidget == nil || !paneWidget.IsVisible() { | ||
| 55 | + | log.Debug(). | |||
| 56 | + | Str("pane_id", string(paneID)). | |||
| 57 | + | Bool("widget_nil", paneWidget == nil). | |||
| 58 | + | Msg("CollectPaneRects: skipping invisible pane") | |||
| 51 | continue | 59 | continue | ||
| 52 | } | 60 | } | ||
| 53 | 61 | ||||
| @@ -77,6 +85,14 @@ func (m *Manager) CollectPaneRects(provider PaneGeometryProvider) []entity.PaneR | |||||
| 77 | continue | 85 | continue | ||
| 78 | } | 86 | } | ||
| 79 | 87 | ||||
| 88 | + | log.Debug(). | |||
| 89 | + | Str("pane_id", string(paneID)). | |||
| 90 | + | Int("x", int(x)). | |||
| 91 | + | Int("y", int(y)). | |||
| 92 | + | Int("w", w). | |||
| 93 | + | Int("h", h). | |||
| 94 | + | Msg("CollectPaneRects: added visible pane") | |||
| 95 | + | ||||
| 80 | rects = append(rects, entity.PaneRect{ | 96 | rects = append(rects, entity.PaneRect{ | ||
| 81 | PaneID: paneID, | 97 | PaneID: paneID, | ||
| 82 | X: int(x), | 98 | X: int(x), | ||
| @@ -86,6 +102,8 @@ func (m *Manager) CollectPaneRects(provider PaneGeometryProvider) []entity.PaneR | |||||
| 86 | }) | 102 | }) | ||
| 87 | } | 103 | } | ||
| 88 | 104 | ||||
| 105 | + | log.Debug().Int("visible_panes", len(rects)).Msg("CollectPaneRects: done") | |||
| 106 | + | ||||
| 89 | return rects | 107 | return rects | ||
| 90 | } | 108 | } | ||
| 91 | 109 | ||||
| @@ -119,8 +137,12 @@ func (m *Manager) NavigateGeometric( | |||||
| 119 | stackNode := activeNode.Parent | 137 | stackNode := activeNode.Parent | ||
| 120 | canNavigate, newPaneID := m.navigateWithinStack(stackNode, activeNode, direction) | 138 | canNavigate, newPaneID := m.navigateWithinStack(stackNode, activeNode, direction) | ||
| 121 | if canNavigate { | 139 | if canNavigate { | ||
| 122 | - | ws.ActivePaneID = newPaneID | 140 | + | // Apply focus change through use case (handles stack index updates) |
| 123 | - | return ws.FindPane(newPaneID), nil | 141 | + | targetNode, err := m.panesUC.ApplyFocusChange(ctx, ws, newPaneID) |
| 142 | + | if err != nil { | |||
| 143 | + | return nil, err | |||
| 144 | + | } | |||
| 145 | + | return targetNode, nil | |||
| 124 | } | 146 | } | ||
| 125 | // At stack boundary - fall through to geometric navigation | 147 | // At stack boundary - fall through to geometric navigation | ||
| 126 | log.Debug().Msg("at stack boundary, using geometric navigation") | 148 | log.Debug().Msg("at stack boundary, using geometric navigation") | ||
| @@ -128,7 +150,7 @@ func (m *Manager) NavigateGeometric( | |||||
| 128 | } | 150 | } | ||
| 129 | 151 | ||||
| 130 | // Collect geometry from visible panes | 152 | // Collect geometry from visible panes | ||
| 131 | - | rects := m.CollectPaneRects(provider) | 153 | + | rects := m.CollectPaneRects(ctx, provider) |
| 132 | if len(rects) == 0 { | 154 | if len(rects) == 0 { | ||
| 133 | return nil, nil | 155 | return nil, nil | ||
| 134 | } | 156 | } | ||
| @@ -146,16 +168,20 @@ func (m *Manager) NavigateGeometric( | |||||
| 146 | return nil, nil | 168 | return nil, nil | ||
| 147 | } | 169 | } | ||
| 148 | 170 | ||||
| 149 | - | // Update workspace | 171 | + | // Apply focus change through use case (handles stack index updates) |
| 150 | - | ws.ActivePaneID = output.TargetPaneID | 172 | + | targetNode, err := m.panesUC.ApplyFocusChange(ctx, ws, output.TargetPaneID) |
| 173 | + | if err != nil { | |||
| 174 | + | return nil, err | |||
| 175 | + | } | |||
| 151 | 176 | ||||
| 152 | - | // Return target node | 177 | + | return targetNode, nil |
| 153 | - | return ws.FindPane(output.TargetPaneID), nil | |||
| 154 | } | 178 | } | ||
| 155 | 179 | ||||
| 156 | // navigateWithinStack tries to navigate within a stack. | 180 | // navigateWithinStack tries to navigate within a stack. | ||
| 157 | // Returns (canNavigate, newPaneID). If canNavigate is false, the pane is at a | 181 | // Returns (canNavigate, newPaneID). If canNavigate is false, the pane is at a | ||
| 158 | // boundary and navigation should escape the stack. | 182 | // boundary and navigation should escape the stack. | ||
| 183 | + | // Note: This method does not modify domain state; the caller should use | |||
| 184 | + | // ApplyFocusChange to apply the focus change. | |||
| 159 | func (m *Manager) navigateWithinStack( | 185 | func (m *Manager) navigateWithinStack( | ||
| 160 | stackNode, currentNode *entity.PaneNode, direction usecase.NavigateDirection, | 186 | stackNode, currentNode *entity.PaneNode, direction usecase.NavigateDirection, | ||
| 161 | ) (bool, entity.PaneID) { | 187 | ) (bool, entity.PaneID) { | ||
| @@ -173,7 +199,6 @@ func (m *Manager) navigateWithinStack( | |||||
| 173 | return false, "" | 199 | return false, "" | ||
| 174 | } | 200 | } | ||
| 175 | 201 | ||||
| 176 | - | stackNode.ActiveStackIndex = newIdx | |||
| 177 | return m.getPaneIDFromStackChild(stackNode.Children[newIdx]) | 202 | return m.getPaneIDFromStackChild(stackNode.Children[newIdx]) | ||
| 178 | } | 203 | } | ||
| 179 | 204 | ||||
M internal/ui/focus/manager_test.go +57 -61
| @@ -11,6 +11,8 @@ import ( | |||
| 11 | 11 | ||
| 12 | 12 | // TestNavigateWithinStack_UsesActiveStackIndex tests that navigation uses | |
| 13 | 13 | // ActiveStackIndex from the domain model instead of pointer comparison. | |
| 14 | + | // Note: navigateWithinStack is a pure function that returns the target pane ID | |
| 15 | + | // without modifying state. The actual state mutation happens in ApplyFocusChange. | |
| 14 | 16 | func TestNavigateWithinStack_UsesActiveStackIndex(t *testing.T) { | |
| 15 | 17 | // Create a stack with 3 children | |
| 16 | 18 | pane0 := &entity.Pane{ID: "pane0"} | |
| @@ -36,60 +38,53 @@ func TestNavigateWithinStack_UsesActiveStackIndex(t *testing.T) { | |||
| 36 | 38 | m := NewManager(nil) | |
| 37 | 39 | ||
| 38 | 40 | tests := []struct { | |
| 39 | - | name string | |
| 40 | - | activeIndex int | |
| 41 | - | direction usecase.NavigateDirection | |
| 42 | - | wantCanNav bool | |
| 43 | - | wantPaneID entity.PaneID | |
| 44 | - | wantNewIndex int | |
| 41 | + | name string | |
| 42 | + | activeIndex int | |
| 43 | + | direction usecase.NavigateDirection | |
| 44 | + | wantCanNav bool | |
| 45 | + | wantPaneID entity.PaneID | |
| 45 | 46 | }{ | |
| 46 | 47 | { | |
| 47 | - | name: "navigate down from middle", | |
| 48 | - | activeIndex: 1, | |
| 49 | - | direction: usecase.NavDown, | |
| 50 | - | wantCanNav: true, | |
| 51 | - | wantPaneID: "pane2", | |
| 52 | - | wantNewIndex: 2, | |
| 48 | + | name: "navigate down from middle", | |
| 49 | + | activeIndex: 1, | |
| 50 | + | direction: usecase.NavDown, | |
| 51 | + | wantCanNav: true, | |
| 52 | + | wantPaneID: "pane2", | |
| 53 | 53 | }, | |
| 54 | 54 | { | |
| 55 | - | name: "navigate up from middle", | |
| 56 | - | activeIndex: 1, | |
| 57 | - | direction: usecase.NavUp, | |
| 58 | - | wantCanNav: true, | |
| 59 | - | wantPaneID: "pane0", | |
| 60 | - | wantNewIndex: 0, | |
| 55 | + | name: "navigate up from middle", | |
| 56 | + | activeIndex: 1, | |
| 57 | + | direction: usecase.NavUp, | |
| 58 | + | wantCanNav: true, | |
| 59 | + | wantPaneID: "pane0", | |
| 61 | 60 | }, | |
| 62 | 61 | { | |
| 63 | - | name: "navigate down from last - boundary", | |
| 64 | - | activeIndex: 2, | |
| 65 | - | direction: usecase.NavDown, | |
| 66 | - | wantCanNav: false, | |
| 67 | - | wantPaneID: "", | |
| 68 | - | wantNewIndex: 2, // unchanged | |
| 62 | + | name: "navigate down from last - boundary", | |
| 63 | + | activeIndex: 2, | |
| 64 | + | direction: usecase.NavDown, | |
| 65 | + | wantCanNav: false, | |
| 66 | + | wantPaneID: "", | |
| 69 | 67 | }, | |
| 70 | 68 | { | |
| 71 | - | name: "navigate up from first - boundary", | |
| 72 | - | activeIndex: 0, | |
| 73 | - | direction: usecase.NavUp, | |
| 74 | - | wantCanNav: false, | |
| 75 | - | wantPaneID: "", | |
| 76 | - | wantNewIndex: 0, // unchanged | |
| 69 | + | name: "navigate up from first - boundary", | |
| 70 | + | activeIndex: 0, | |
| 71 | + | direction: usecase.NavUp, | |
| 72 | + | wantCanNav: false, | |
| 73 | + | wantPaneID: "", | |
| 77 | 74 | }, | |
| 78 | 75 | { | |
| 79 | - | name: "navigate down from first", | |
| 80 | - | activeIndex: 0, | |
| 81 | - | direction: usecase.NavDown, | |
| 82 | - | wantCanNav: true, | |
| 83 | - | wantPaneID: "pane1", | |
| 84 | - | wantNewIndex: 1, | |
| 76 | + | name: "navigate down from first", | |
| 77 | + | activeIndex: 0, | |
| 78 | + | direction: usecase.NavDown, | |
| 79 | + | wantCanNav: true, | |
| 80 | + | wantPaneID: "pane1", | |
| 85 | 81 | }, | |
| 86 | 82 | { | |
| 87 | - | name: "navigate up from last", | |
| 88 | - | activeIndex: 2, | |
| 89 | - | direction: usecase.NavUp, | |
| 90 | - | wantCanNav: true, | |
| 91 | - | wantPaneID: "pane1", | |
| 92 | - | wantNewIndex: 1, | |
| 83 | + | name: "navigate up from last", | |
| 84 | + | activeIndex: 2, | |
| 85 | + | direction: usecase.NavUp, | |
| 86 | + | wantCanNav: true, | |
| 87 | + | wantPaneID: "pane1", | |
| 93 | 88 | }, | |
| 94 | 89 | } | |
| 95 | 90 | ||
| @@ -98,16 +93,11 @@ func TestNavigateWithinStack_UsesActiveStackIndex(t *testing.T) { | |||
| 98 | 93 | // Set up the active index | |
| 99 | 94 | stackNode.ActiveStackIndex = tt.activeIndex | |
| 100 | 95 | ||
| 101 | - | // Call the private function through reflection or test it indirectly | |
| 102 | - | // Since navigateWithinStack is private, we test its behavior | |
| 96 | + | // navigateWithinStack is a pure function - it does not modify state | |
| 103 | 97 | canNav, paneID := m.navigateWithinStack(stackNode, stackNode.Children[tt.activeIndex], tt.direction) | |
| 104 | 98 | ||
| 105 | 99 | assert.Equal(t, tt.wantCanNav, canNav) | |
| 106 | 100 | assert.Equal(t, tt.wantPaneID, paneID) | |
| 107 | - | ||
| 108 | - | if canNav { | |
| 109 | - | assert.Equal(t, tt.wantNewIndex, stackNode.ActiveStackIndex) | |
| 110 | - | } | |
| 111 | 101 | }) | |
| 112 | 102 | } | |
| 113 | 103 | } | |
| @@ -193,9 +183,10 @@ func TestNavigateWithinStack_InvalidDirection(t *testing.T) { | |||
| 193 | 183 | assert.Empty(t, paneID) | |
| 194 | 184 | } | |
| 195 | 185 | ||
| 196 | - | // TestNavigateWithinStack_UpdatesActiveStackIndex tests that navigation properly | |
| 197 | - | // updates the ActiveStackIndex on the stack node. | |
| 198 | - | func TestNavigateWithinStack_UpdatesActiveStackIndex(t *testing.T) { | |
| 186 | + | // TestNavigateWithinStack_ReturnsCorrectPaneID tests that navigation returns | |
| 187 | + | // the correct target pane ID for sequential navigation. | |
| 188 | + | // Note: navigateWithinStack is now a pure function - state updates happen in ApplyFocusChange. | |
| 189 | + | func TestNavigateWithinStack_ReturnsCorrectPaneID(t *testing.T) { | |
| 199 | 190 | pane0 := &entity.Pane{ID: "pane0"} | |
| 200 | 191 | pane1 := &entity.Pane{ID: "pane1"} | |
| 201 | 192 | pane2 := &entity.Pane{ID: "pane2"} | |
| @@ -213,22 +204,27 @@ func TestNavigateWithinStack_UpdatesActiveStackIndex(t *testing.T) { | |||
| 213 | 204 | ||
| 214 | 205 | m := NewManager(nil) | |
| 215 | 206 | ||
| 216 | - | // Navigate down twice | |
| 207 | + | // Navigate down from first - should return pane1 | |
| 217 | 208 | stackNode.ActiveStackIndex = 0 | |
| 218 | - | canNav, _ := m.navigateWithinStack(stackNode, child0, usecase.NavDown) | |
| 209 | + | canNav, paneID := m.navigateWithinStack(stackNode, child0, usecase.NavDown) | |
| 219 | 210 | assert.True(t, canNav) | |
| 220 | - | assert.Equal(t, 1, stackNode.ActiveStackIndex) | |
| 211 | + | assert.Equal(t, entity.PaneID("pane1"), paneID) | |
| 221 | 212 | ||
| 222 | - | canNav, _ = m.navigateWithinStack(stackNode, child1, usecase.NavDown) | |
| 213 | + | // Navigate down from middle - should return pane2 | |
| 214 | + | stackNode.ActiveStackIndex = 1 | |
| 215 | + | canNav, paneID = m.navigateWithinStack(stackNode, child1, usecase.NavDown) | |
| 223 | 216 | assert.True(t, canNav) | |
| 224 | - | assert.Equal(t, 2, stackNode.ActiveStackIndex) | |
| 217 | + | assert.Equal(t, entity.PaneID("pane2"), paneID) | |
| 225 | 218 | ||
| 226 | - | // Navigate up twice | |
| 227 | - | canNav, _ = m.navigateWithinStack(stackNode, child2, usecase.NavUp) | |
| 219 | + | // Navigate up from last - should return pane1 | |
| 220 | + | stackNode.ActiveStackIndex = 2 | |
| 221 | + | canNav, paneID = m.navigateWithinStack(stackNode, child2, usecase.NavUp) | |
| 228 | 222 | assert.True(t, canNav) | |
| 229 | - | assert.Equal(t, 1, stackNode.ActiveStackIndex) | |
| 223 | + | assert.Equal(t, entity.PaneID("pane1"), paneID) | |
| 230 | 224 | ||
| 231 | - | canNav, _ = m.navigateWithinStack(stackNode, child1, usecase.NavUp) | |
| 225 | + | // Navigate up from middle - should return pane0 | |
| 226 | + | stackNode.ActiveStackIndex = 1 | |
| 227 | + | canNav, paneID = m.navigateWithinStack(stackNode, child1, usecase.NavUp) | |
| 232 | 228 | assert.True(t, canNav) | |
| 233 | - | assert.Equal(t, 0, stackNode.ActiveStackIndex) | |
| 229 | + | assert.Equal(t, entity.PaneID("pane0"), paneID) | |
| 234 | 230 | } | |
| @@ -11,6 +11,8 @@ import ( | |||||
| 11 | 11 | ||||
| 12 | // TestNavigateWithinStack_UsesActiveStackIndex tests that navigation uses | 12 | // TestNavigateWithinStack_UsesActiveStackIndex tests that navigation uses | ||
| 13 | // ActiveStackIndex from the domain model instead of pointer comparison. | 13 | // ActiveStackIndex from the domain model instead of pointer comparison. | ||
| 14 | + | // Note: navigateWithinStack is a pure function that returns the target pane ID | |||
| 15 | + | // without modifying state. The actual state mutation happens in ApplyFocusChange. | |||
| 14 | func TestNavigateWithinStack_UsesActiveStackIndex(t *testing.T) { | 16 | func TestNavigateWithinStack_UsesActiveStackIndex(t *testing.T) { | ||
| 15 | // Create a stack with 3 children | 17 | // Create a stack with 3 children | ||
| 16 | pane0 := &entity.Pane{ID: "pane0"} | 18 | pane0 := &entity.Pane{ID: "pane0"} | ||
| @@ -36,60 +38,53 @@ func TestNavigateWithinStack_UsesActiveStackIndex(t *testing.T) { | |||||
| 36 | m := NewManager(nil) | 38 | m := NewManager(nil) | ||
| 37 | 39 | ||||
| 38 | tests := []struct { | 40 | tests := []struct { | ||
| 39 | - | name string | 41 | + | name string |
| 40 | - | activeIndex int | 42 | + | activeIndex int |
| 41 | - | direction usecase.NavigateDirection | 43 | + | direction usecase.NavigateDirection |
| 42 | - | wantCanNav bool | 44 | + | wantCanNav bool |
| 43 | - | wantPaneID entity.PaneID | 45 | + | wantPaneID entity.PaneID |
| 44 | - | wantNewIndex int | |||
| 45 | }{ | 46 | }{ | ||
| 46 | { | 47 | { | ||
| 47 | - | name: "navigate down from middle", | 48 | + | name: "navigate down from middle", |
| 48 | - | activeIndex: 1, | 49 | + | activeIndex: 1, |
| 49 | - | direction: usecase.NavDown, | 50 | + | direction: usecase.NavDown, |
| 50 | - | wantCanNav: true, | 51 | + | wantCanNav: true, |
| 51 | - | wantPaneID: "pane2", | 52 | + | wantPaneID: "pane2", |
| 52 | - | wantNewIndex: 2, | |||
| 53 | }, | 53 | }, | ||
| 54 | { | 54 | { | ||
| 55 | - | name: "navigate up from middle", | 55 | + | name: "navigate up from middle", |
| 56 | - | activeIndex: 1, | 56 | + | activeIndex: 1, |
| 57 | - | direction: usecase.NavUp, | 57 | + | direction: usecase.NavUp, |
| 58 | - | wantCanNav: true, | 58 | + | wantCanNav: true, |
| 59 | - | wantPaneID: "pane0", | 59 | + | wantPaneID: "pane0", |
| 60 | - | wantNewIndex: 0, | |||
| 61 | }, | 60 | }, | ||
| 62 | { | 61 | { | ||
| 63 | - | name: "navigate down from last - boundary", | 62 | + | name: "navigate down from last - boundary", |
| 64 | - | activeIndex: 2, | 63 | + | activeIndex: 2, |
| 65 | - | direction: usecase.NavDown, | 64 | + | direction: usecase.NavDown, |
| 66 | - | wantCanNav: false, | 65 | + | wantCanNav: false, |
| 67 | - | wantPaneID: "", | 66 | + | wantPaneID: "", |
| 68 | - | wantNewIndex: 2, // unchanged | |||
| 69 | }, | 67 | }, | ||
| 70 | { | 68 | { | ||
| 71 | - | name: "navigate up from first - boundary", | 69 | + | name: "navigate up from first - boundary", |
| 72 | - | activeIndex: 0, | 70 | + | activeIndex: 0, |
| 73 | - | direction: usecase.NavUp, | 71 | + | direction: usecase.NavUp, |
| 74 | - | wantCanNav: false, | 72 | + | wantCanNav: false, |
| 75 | - | wantPaneID: "", | 73 | + | wantPaneID: "", |
| 76 | - | wantNewIndex: 0, // unchanged | |||
| 77 | }, | 74 | }, | ||
| 78 | { | 75 | { | ||
| 79 | - | name: "navigate down from first", | 76 | + | name: "navigate down from first", |
| 80 | - | activeIndex: 0, | 77 | + | activeIndex: 0, |
| 81 | - | direction: usecase.NavDown, | 78 | + | direction: usecase.NavDown, |
| 82 | - | wantCanNav: true, | 79 | + | wantCanNav: true, |
| 83 | - | wantPaneID: "pane1", | 80 | + | wantPaneID: "pane1", |
| 84 | - | wantNewIndex: 1, | |||
| 85 | }, | 81 | }, | ||
| 86 | { | 82 | { | ||
| 87 | - | name: "navigate up from last", | 83 | + | name: "navigate up from last", |
| 88 | - | activeIndex: 2, | 84 | + | activeIndex: 2, |
| 89 | - | direction: usecase.NavUp, | 85 | + | direction: usecase.NavUp, |
| 90 | - | wantCanNav: true, | 86 | + | wantCanNav: true, |
| 91 | - | wantPaneID: "pane1", | 87 | + | wantPaneID: "pane1", |
| 92 | - | wantNewIndex: 1, | |||
| 93 | }, | 88 | }, | ||
| 94 | } | 89 | } | ||
| 95 | 90 | ||||
| @@ -98,16 +93,11 @@ func TestNavigateWithinStack_UsesActiveStackIndex(t *testing.T) { | |||||
| 98 | // Set up the active index | 93 | // Set up the active index | ||
| 99 | stackNode.ActiveStackIndex = tt.activeIndex | 94 | stackNode.ActiveStackIndex = tt.activeIndex | ||
| 100 | 95 | ||||
| 101 | - | // Call the private function through reflection or test it indirectly | 96 | + | // navigateWithinStack is a pure function - it does not modify state |
| 102 | - | // Since navigateWithinStack is private, we test its behavior | |||
| 103 | canNav, paneID := m.navigateWithinStack(stackNode, stackNode.Children[tt.activeIndex], tt.direction) | 97 | canNav, paneID := m.navigateWithinStack(stackNode, stackNode.Children[tt.activeIndex], tt.direction) | ||
| 104 | 98 | ||||
| 105 | assert.Equal(t, tt.wantCanNav, canNav) | 99 | assert.Equal(t, tt.wantCanNav, canNav) | ||
| 106 | assert.Equal(t, tt.wantPaneID, paneID) | 100 | assert.Equal(t, tt.wantPaneID, paneID) | ||
| 107 | - | ||||
| 108 | - | if canNav { | |||
| 109 | - | assert.Equal(t, tt.wantNewIndex, stackNode.ActiveStackIndex) | |||
| 110 | - | } | |||
| 111 | }) | 101 | }) | ||
| 112 | } | 102 | } | ||
| 113 | } | 103 | } | ||
| @@ -193,9 +183,10 @@ func TestNavigateWithinStack_InvalidDirection(t *testing.T) { | |||||
| 193 | assert.Empty(t, paneID) | 183 | assert.Empty(t, paneID) | ||
| 194 | } | 184 | } | ||
| 195 | 185 | ||||
| 196 | - | // TestNavigateWithinStack_UpdatesActiveStackIndex tests that navigation properly | 186 | + | // TestNavigateWithinStack_ReturnsCorrectPaneID tests that navigation returns |
| 197 | - | // updates the ActiveStackIndex on the stack node. | 187 | + | // the correct target pane ID for sequential navigation. |
| 198 | - | func TestNavigateWithinStack_UpdatesActiveStackIndex(t *testing.T) { | 188 | + | // Note: navigateWithinStack is now a pure function - state updates happen in ApplyFocusChange. |
| 189 | + | func TestNavigateWithinStack_ReturnsCorrectPaneID(t *testing.T) { | |||
| 199 | pane0 := &entity.Pane{ID: "pane0"} | 190 | pane0 := &entity.Pane{ID: "pane0"} | ||
| 200 | pane1 := &entity.Pane{ID: "pane1"} | 191 | pane1 := &entity.Pane{ID: "pane1"} | ||
| 201 | pane2 := &entity.Pane{ID: "pane2"} | 192 | pane2 := &entity.Pane{ID: "pane2"} | ||
| @@ -213,22 +204,27 @@ func TestNavigateWithinStack_UpdatesActiveStackIndex(t *testing.T) { | |||||
| 213 | 204 | ||||
| 214 | m := NewManager(nil) | 205 | m := NewManager(nil) | ||
| 215 | 206 | ||||
| 216 | - | // Navigate down twice | 207 | + | // Navigate down from first - should return pane1 |
| 217 | stackNode.ActiveStackIndex = 0 | 208 | stackNode.ActiveStackIndex = 0 | ||
| 218 | - | canNav, _ := m.navigateWithinStack(stackNode, child0, usecase.NavDown) | 209 | + | canNav, paneID := m.navigateWithinStack(stackNode, child0, usecase.NavDown) |
| 219 | assert.True(t, canNav) | 210 | assert.True(t, canNav) | ||
| 220 | - | assert.Equal(t, 1, stackNode.ActiveStackIndex) | 211 | + | assert.Equal(t, entity.PaneID("pane1"), paneID) |
| 221 | 212 | ||||
| 222 | - | canNav, _ = m.navigateWithinStack(stackNode, child1, usecase.NavDown) | 213 | + | // Navigate down from middle - should return pane2 |
| 214 | + | stackNode.ActiveStackIndex = 1 | |||
| 215 | + | canNav, paneID = m.navigateWithinStack(stackNode, child1, usecase.NavDown) | |||
| 223 | assert.True(t, canNav) | 216 | assert.True(t, canNav) | ||
| 224 | - | assert.Equal(t, 2, stackNode.ActiveStackIndex) | 217 | + | assert.Equal(t, entity.PaneID("pane2"), paneID) |
| 225 | 218 | ||||
| 226 | - | // Navigate up twice | 219 | + | // Navigate up from last - should return pane1 |
| 227 | - | canNav, _ = m.navigateWithinStack(stackNode, child2, usecase.NavUp) | 220 | + | stackNode.ActiveStackIndex = 2 |
| 221 | + | canNav, paneID = m.navigateWithinStack(stackNode, child2, usecase.NavUp) | |||
| 228 | assert.True(t, canNav) | 222 | assert.True(t, canNav) | ||
| 229 | - | assert.Equal(t, 1, stackNode.ActiveStackIndex) | 223 | + | assert.Equal(t, entity.PaneID("pane1"), paneID) |
| 230 | 224 | ||||
| 231 | - | canNav, _ = m.navigateWithinStack(stackNode, child1, usecase.NavUp) | 225 | + | // Navigate up from middle - should return pane0 |
| 226 | + | stackNode.ActiveStackIndex = 1 | |||
| 227 | + | canNav, paneID = m.navigateWithinStack(stackNode, child1, usecase.NavUp) | |||
| 232 | assert.True(t, canNav) | 228 | assert.True(t, canNav) | ||
| 233 | - | assert.Equal(t, 0, stackNode.ActiveStackIndex) | 229 | + | assert.Equal(t, entity.PaneID("pane0"), paneID) |
| 234 | } | 230 | } | ||
M internal/ui/input/hover.go +6 -0
| @@ -115,6 +115,12 @@ func (h *HoverHandler) handleEnter() { | |||
| 115 | 115 | ||
| 116 | 116 | // handleLeave cancels any pending focus switch. | |
| 117 | 117 | func (h *HoverHandler) handleLeave() { | |
| 118 | + | h.Cancel() | |
| 119 | + | } | |
| 120 | + | ||
| 121 | + | // Cancel cancels any pending focus switch. | |
| 122 | + | // This can be called externally to cancel hover when keyboard navigation occurs. | |
| 123 | + | func (h *HoverHandler) Cancel() { | |
| 118 | 124 | h.timerMu.Lock() | |
| 119 | 125 | defer h.timerMu.Unlock() | |
| 120 | 126 | ||
| @@ -115,6 +115,12 @@ func (h *HoverHandler) handleEnter() { | |||||
| 115 | 115 | ||||
| 116 | // handleLeave cancels any pending focus switch. | 116 | // handleLeave cancels any pending focus switch. | ||
| 117 | func (h *HoverHandler) handleLeave() { | 117 | func (h *HoverHandler) handleLeave() { | ||
| 118 | + | h.Cancel() | |||
| 119 | + | } | |||
| 120 | + | ||||
| 121 | + | // Cancel cancels any pending focus switch. | |||
| 122 | + | // This can be called externally to cancel hover when keyboard navigation occurs. | |||
| 123 | + | func (h *HoverHandler) Cancel() { | |||
| 118 | h.timerMu.Lock() | 124 | h.timerMu.Lock() | ||
| 119 | defer h.timerMu.Unlock() | 125 | defer h.timerMu.Unlock() | ||
| 120 | 126 | ||||
M internal/ui/layout/gtk_widgets.go +3 -2
| @@ -519,8 +519,9 @@ func (btn *gtkButton) GetParent() Widget { | |||
| 519 | 519 | return >kWidget{inner: parent} | |
| 520 | 520 | } | |
| 521 | 521 | ||
| 522 | - | func (btn *gtkButton) SetLabel(label string) { btn.inner.SetLabel(label) } | |
| 523 | - | func (btn *gtkButton) GetLabel() string { return btn.inner.GetLabel() } | |
| 522 | + | func (btn *gtkButton) SetLabel(label string) { btn.inner.SetLabel(label) } | |
| 523 | + | func (btn *gtkButton) GetLabel() string { return btn.inner.GetLabel() } | |
| 524 | + | func (btn *gtkButton) SetIconName(name string) { btn.inner.SetIconName(name) } | |
| 524 | 525 | ||
| 525 | 526 | func (btn *gtkButton) SetChild(child Widget) { | |
| 526 | 527 | if child == nil { | |
| @@ -519,8 +519,9 @@ func (btn *gtkButton) GetParent() Widget { | |||||
| 519 | return >kWidget{inner: parent} | 519 | return >kWidget{inner: parent} | ||
| 520 | } | 520 | } | ||
| 521 | 521 | ||||
| 522 | - | func (btn *gtkButton) SetLabel(label string) { btn.inner.SetLabel(label) } | 522 | + | func (btn *gtkButton) SetLabel(label string) { btn.inner.SetLabel(label) } |
| 523 | - | func (btn *gtkButton) GetLabel() string { return btn.inner.GetLabel() } | 523 | + | func (btn *gtkButton) GetLabel() string { return btn.inner.GetLabel() } |
| 524 | + | func (btn *gtkButton) SetIconName(name string) { btn.inner.SetIconName(name) } | |||
| 524 | 525 | ||||
| 525 | func (btn *gtkButton) SetChild(child Widget) { | 526 | func (btn *gtkButton) SetChild(child Widget) { | ||
| 526 | if child == nil { | 527 | if child == nil { | ||
M internal/ui/layout/mocks/mocks.go +40 -0
| @@ -8951,6 +8951,46 @@ func (_c *MockButtonWidget_SetHexpand_Call) RunAndReturn(run func(expand bool)) | |||
| 8951 | 8951 | return _c | |
| 8952 | 8952 | } | |
| 8953 | 8953 | ||
| 8954 | + | // SetIconName provides a mock function for the type MockButtonWidget | |
| 8955 | + | func (_mock *MockButtonWidget) SetIconName(iconName string) { | |
| 8956 | + | _mock.Called(iconName) | |
| 8957 | + | return | |
| 8958 | + | } | |
| 8959 | + | ||
| 8960 | + | // MockButtonWidget_SetIconName_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'SetIconName' | |
| 8961 | + | type MockButtonWidget_SetIconName_Call struct { | |
| 8962 | + | *mock.Call | |
| 8963 | + | } | |
| 8964 | + | ||
| 8965 | + | // SetIconName is a helper method to define mock.On call | |
| 8966 | + | // - iconName string | |
| 8967 | + | func (_e *MockButtonWidget_Expecter) SetIconName(iconName interface{}) *MockButtonWidget_SetIconName_Call { | |
| 8968 | + | return &MockButtonWidget_SetIconName_Call{Call: _e.mock.On("SetIconName", iconName)} | |
| 8969 | + | } | |
| 8970 | + | ||
| 8971 | + | func (_c *MockButtonWidget_SetIconName_Call) Run(run func(iconName string)) *MockButtonWidget_SetIconName_Call { | |
| 8972 | + | _c.Call.Run(func(args mock.Arguments) { | |
| 8973 | + | var arg0 string | |
| 8974 | + | if args[0] != nil { | |
| 8975 | + | arg0 = args[0].(string) | |
| 8976 | + | } | |
| 8977 | + | run( | |
| 8978 | + | arg0, | |
| 8979 | + | ) | |
| 8980 | + | }) | |
| 8981 | + | return _c | |
| 8982 | + | } | |
| 8983 | + | ||
| 8984 | + | func (_c *MockButtonWidget_SetIconName_Call) Return() *MockButtonWidget_SetIconName_Call { | |
| 8985 | + | _c.Call.Return() | |
| 8986 | + | return _c | |
| 8987 | + | } | |
| 8988 | + | ||
| 8989 | + | func (_c *MockButtonWidget_SetIconName_Call) RunAndReturn(run func(iconName string)) *MockButtonWidget_SetIconName_Call { | |
| 8990 | + | _c.Run(run) | |
| 8991 | + | return _c | |
| 8992 | + | } | |
| 8993 | + | ||
| 8954 | 8994 | // SetLabel provides a mock function for the type MockButtonWidget | |
| 8955 | 8995 | func (_mock *MockButtonWidget) SetLabel(label string) { | |
| 8956 | 8996 | _mock.Called(label) | |
| @@ -8951,6 +8951,46 @@ func (_c *MockButtonWidget_SetHexpand_Call) RunAndReturn(run func(expand bool)) | |||||
| 8951 | return _c | 8951 | return _c | ||
| 8952 | } | 8952 | } | ||
| 8953 | 8953 | ||||
| 8954 | + | // SetIconName provides a mock function for the type MockButtonWidget | |||
| 8955 | + | func (_mock *MockButtonWidget) SetIconName(iconName string) { | |||
| 8956 | + | _mock.Called(iconName) | |||
| 8957 | + | return | |||
| 8958 | + | } | |||
| 8959 | + | ||||
| 8960 | + | // MockButtonWidget_SetIconName_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'SetIconName' | |||
| 8961 | + | type MockButtonWidget_SetIconName_Call struct { | |||
| 8962 | + | *mock.Call | |||
| 8963 | + | } | |||
| 8964 | + | ||||
| 8965 | + | // SetIconName is a helper method to define mock.On call | |||
| 8966 | + | // - iconName string | |||
| 8967 | + | func (_e *MockButtonWidget_Expecter) SetIconName(iconName interface{}) *MockButtonWidget_SetIconName_Call { | |||
| 8968 | + | return &MockButtonWidget_SetIconName_Call{Call: _e.mock.On("SetIconName", iconName)} | |||
| 8969 | + | } | |||
| 8970 | + | ||||
| 8971 | + | func (_c *MockButtonWidget_SetIconName_Call) Run(run func(iconName string)) *MockButtonWidget_SetIconName_Call { | |||
| 8972 | + | _c.Call.Run(func(args mock.Arguments) { | |||
| 8973 | + | var arg0 string | |||
| 8974 | + | if args[0] != nil { | |||
| 8975 | + | arg0 = args[0].(string) | |||
| 8976 | + | } | |||
| 8977 | + | run( | |||
| 8978 | + | arg0, | |||
| 8979 | + | ) | |||
| 8980 | + | }) | |||
| 8981 | + | return _c | |||
| 8982 | + | } | |||
| 8983 | + | ||||
| 8984 | + | func (_c *MockButtonWidget_SetIconName_Call) Return() *MockButtonWidget_SetIconName_Call { | |||
| 8985 | + | _c.Call.Return() | |||
| 8986 | + | return _c | |||
| 8987 | + | } | |||
| 8988 | + | ||||
| 8989 | + | func (_c *MockButtonWidget_SetIconName_Call) RunAndReturn(run func(iconName string)) *MockButtonWidget_SetIconName_Call { | |||
| 8990 | + | _c.Run(run) | |||
| 8991 | + | return _c | |||
| 8992 | + | } | |||
| 8993 | + | ||||
| 8954 | // SetLabel provides a mock function for the type MockButtonWidget | 8994 | // SetLabel provides a mock function for the type MockButtonWidget | ||
| 8955 | func (_mock *MockButtonWidget) SetLabel(label string) { | 8995 | func (_mock *MockButtonWidget) SetLabel(label string) { | ||
| 8956 | _mock.Called(label) | 8996 | _mock.Called(label) | ||
M internal/ui/layout/stacked.go +120 -83
| @@ -17,7 +17,10 @@ var ErrIndexOutOfBounds = errors.New("index out of bounds") | |||
| 17 | 17 | // ErrCannotRemoveLastPane is returned when trying to remove the last pane from a stack. | |
| 18 | 18 | var ErrCannotRemoveLastPane = errors.New("cannot remove last pane from stack") | |
| 19 | 19 | ||
| 20 | - | const stackedTitleMaxWidthChars = 30 | |
| 20 | + | const ( | |
| 21 | + | stackedTitleMaxWidthChars = 30 | |
| 22 | + | stackedPaneCloseIcon = "window-close-symbolic" | |
| 23 | + | ) | |
| 21 | 24 | ||
| 22 | 25 | // stackedPane represents a single pane within a stacked container. | |
| 23 | 26 | type stackedPane struct { | |
| @@ -38,7 +41,8 @@ type StackedView struct { | |||
| 38 | 41 | panes []*stackedPane | |
| 39 | 42 | activeIndex int | |
| 40 | 43 | ||
| 41 | - | onActivate func(index int) // called when a pane is activated via title bar click | |
| 44 | + | onActivate func(index int) // called when a pane is activated via title bar click | |
| 45 | + | onClosePane func(paneID string) // called when a pane's close button is clicked | |
| 42 | 46 | ||
| 43 | 47 | mu sync.RWMutex | |
| 44 | 48 | } | |
| @@ -57,20 +61,18 @@ func NewStackedView(factory WidgetFactory) *StackedView { | |||
| 57 | 61 | } | |
| 58 | 62 | } | |
| 59 | 63 | ||
| 60 | - | // AddPane adds a new pane to the stack. | |
| 61 | - | // The new pane becomes active (visible). | |
| 62 | - | func (sv *StackedView) AddPane(ctx context.Context, paneID, title, faviconIconName string, container Widget) int { | |
| 63 | - | log := logging.FromContext(ctx) | |
| 64 | - | sv.mu.Lock() | |
| 65 | - | defer sv.mu.Unlock() | |
| 66 | - | ||
| 67 | - | log.Debug(). | |
| 68 | - | Str("title", title). | |
| 69 | - | Bool("container_nil", container == nil). | |
| 70 | - | Int("current_count", len(sv.panes)). | |
| 71 | - | Msg("StackedView.AddPane called") | |
| 64 | + | // titleBarComponents holds the widgets created for a pane's title bar. | |
| 65 | + | type titleBarComponents struct { | |
| 66 | + | titleBar BoxWidget | |
| 67 | + | titleButton ButtonWidget | |
| 68 | + | closeBtn ButtonWidget | |
| 69 | + | favicon ImageWidget | |
| 70 | + | label LabelWidget | |
| 71 | + | } | |
| 72 | 72 | ||
| 73 | - | // Create title bar - must not expand vertically | |
| 73 | + | // createTitleBar creates the title bar widgets for a pane. | |
| 74 | + | func (sv *StackedView) createTitleBar(title, faviconIconName string) titleBarComponents { | |
| 75 | + | // Create title bar container - must not expand vertically | |
| 74 | 76 | titleBar := sv.factory.NewBox(OrientationHorizontal, 4) | |
| 75 | 77 | titleBar.AddCssClass("stacked-pane-titlebar") | |
| 76 | 78 | titleBar.SetVexpand(false) | |
| @@ -93,6 +95,15 @@ func (sv *StackedView) AddPane(ctx context.Context, paneID, title, faviconIconNa | |||
| 93 | 95 | label.SetXalign(0.0) | |
| 94 | 96 | titleBar.Append(label) | |
| 95 | 97 | ||
| 98 | + | // Create close button using GTK's native icon button support | |
| 99 | + | closeBtn := sv.factory.NewButton() | |
| 100 | + | closeBtn.SetIconName(stackedPaneCloseIcon) | |
| 101 | + | closeBtn.AddCssClass("stacked-pane-close-button") | |
| 102 | + | closeBtn.SetFocusOnClick(false) | |
| 103 | + | closeBtn.SetVexpand(false) | |
| 104 | + | closeBtn.SetHexpand(false) | |
| 105 | + | titleBar.Append(closeBtn) | |
| 106 | + | ||
| 96 | 107 | // Make title bar clickable - ensure it doesn't expand vertically | |
| 97 | 108 | titleButton := sv.factory.NewButton() | |
| 98 | 109 | titleButton.SetChild(titleBar) | |
| @@ -101,35 +112,52 @@ func (sv *StackedView) AddPane(ctx context.Context, paneID, title, faviconIconNa | |||
| 101 | 112 | titleButton.SetVexpand(false) // Critical: don't let title bar expand vertically | |
| 102 | 113 | titleButton.SetHexpand(true) // But fill horizontal space | |
| 103 | 114 | ||
| 115 | + | return titleBarComponents{ | |
| 116 | + | titleBar: titleBar, | |
| 117 | + | titleButton: titleButton, | |
| 118 | + | closeBtn: closeBtn, | |
| 119 | + | favicon: favicon, | |
| 120 | + | label: label, | |
| 121 | + | } | |
| 122 | + | } | |
| 123 | + | ||
| 124 | + | // AddPane adds a new pane to the stack. | |
| 125 | + | // The new pane becomes active (visible). | |
| 126 | + | func (sv *StackedView) AddPane(ctx context.Context, paneID, title, faviconIconName string, container Widget) int { | |
| 127 | + | log := logging.FromContext(ctx) | |
| 128 | + | sv.mu.Lock() | |
| 129 | + | defer sv.mu.Unlock() | |
| 130 | + | ||
| 131 | + | log.Debug(). | |
| 132 | + | Str("title", title). | |
| 133 | + | Bool("container_nil", container == nil). | |
| 134 | + | Int("current_count", len(sv.panes)). | |
| 135 | + | Msg("StackedView.AddPane called") | |
| 136 | + | ||
| 137 | + | // Create title bar components | |
| 138 | + | tb := sv.createTitleBar(title, faviconIconName) | |
| 139 | + | ||
| 104 | 140 | pane := &stackedPane{ | |
| 105 | 141 | paneID: paneID, | |
| 106 | - | titleBar: titleBar, | |
| 142 | + | titleBar: tb.titleBar, | |
| 107 | 143 | container: container, | |
| 108 | 144 | title: title, | |
| 109 | - | favicon: favicon, | |
| 110 | - | label: label, | |
| 145 | + | favicon: tb.favicon, | |
| 146 | + | label: tb.label, | |
| 111 | 147 | isActive: false, | |
| 112 | 148 | } | |
| 113 | 149 | ||
| 114 | 150 | index := len(sv.panes) | |
| 115 | 151 | sv.panes = append(sv.panes, pane) | |
| 116 | 152 | ||
| 117 | - | // Connect click handler | |
| 118 | - | titleButton.ConnectClicked(func() { | |
| 119 | - | sv.mu.RLock() | |
| 120 | - | callback := sv.onActivate | |
| 121 | - | sv.mu.RUnlock() | |
| 122 | - | ||
| 123 | - | if callback != nil { | |
| 124 | - | callback(index) | |
| 125 | - | } | |
| 126 | - | }) | |
| 153 | + | // Connect click handlers using paneID (not index, to handle removals) | |
| 154 | + | sv.connectTitleBarHandlers(tb, paneID) | |
| 127 | 155 | ||
| 128 | 156 | // Add to container | |
| 129 | 157 | log.Debug(). | |
| 130 | 158 | Int("index", index). | |
| 131 | 159 | Msg("StackedView: appending titleButton and container to box") | |
| 132 | - | sv.box.Append(titleButton) | |
| 160 | + | sv.box.Append(tb.titleButton) | |
| 133 | 161 | if container != nil { | |
| 134 | 162 | sv.box.Append(container) | |
| 135 | 163 | } | |
| @@ -145,6 +173,33 @@ func (sv *StackedView) AddPane(ctx context.Context, paneID, title, faviconIconNa | |||
| 145 | 173 | return index | |
| 146 | 174 | } | |
| 147 | 175 | ||
| 176 | + | // connectTitleBarHandlers connects the click handlers for a title bar. | |
| 177 | + | // Uses paneID lookup instead of captured index to handle pane removals correctly. | |
| 178 | + | func (sv *StackedView) connectTitleBarHandlers(tb titleBarComponents, paneID string) { | |
| 179 | + | // Connect title bar click handler for activation | |
| 180 | + | tb.titleButton.ConnectClicked(func() { | |
| 181 | + | sv.mu.RLock() | |
| 182 | + | callback := sv.onActivate | |
| 183 | + | currentIndex := sv.findPaneIndexInternal(paneID) | |
| 184 | + | sv.mu.RUnlock() | |
| 185 | + | ||
| 186 | + | if callback != nil && currentIndex >= 0 { | |
| 187 | + | callback(currentIndex) | |
| 188 | + | } | |
| 189 | + | }) | |
| 190 | + | ||
| 191 | + | // Connect close button click handler | |
| 192 | + | tb.closeBtn.ConnectClicked(func() { | |
| 193 | + | sv.mu.RLock() | |
| 194 | + | onClose := sv.onClosePane | |
| 195 | + | sv.mu.RUnlock() | |
| 196 | + | ||
| 197 | + | if onClose != nil { | |
| 198 | + | onClose(paneID) | |
| 199 | + | } | |
| 200 | + | }) | |
| 201 | + | } | |
| 202 | + | ||
| 148 | 203 | // InsertPaneAfter inserts a new pane after the specified index position. | |
| 149 | 204 | // Use afterIndex=-1 to insert at the beginning. | |
| 150 | 205 | // The new pane becomes active (visible). | |
| @@ -172,44 +227,16 @@ func (sv *StackedView) InsertPaneAfter( | |||
| 172 | 227 | Int("current_count", len(sv.panes)). | |
| 173 | 228 | Msg("StackedView.InsertPaneAfter called") | |
| 174 | 229 | ||
| 175 | - | // Create title bar - must not expand vertically | |
| 176 | - | titleBar := sv.factory.NewBox(OrientationHorizontal, 4) | |
| 177 | - | titleBar.AddCssClass("stacked-pane-titlebar") | |
| 178 | - | titleBar.SetVexpand(false) | |
| 179 | - | ||
| 180 | - | // Create favicon image | |
| 181 | - | favicon := sv.factory.NewImage() | |
| 182 | - | if faviconIconName != "" { | |
| 183 | - | favicon.SetFromIconName(faviconIconName) | |
| 184 | - | } else { | |
| 185 | - | favicon.SetFromIconName("web-browser-symbolic") | |
| 186 | - | } | |
| 187 | - | favicon.SetPixelSize(16) | |
| 188 | - | titleBar.Append(favicon) | |
| 189 | - | ||
| 190 | - | // Create title label | |
| 191 | - | label := sv.factory.NewLabel(title) | |
| 192 | - | label.SetEllipsize(EllipsizeEnd) | |
| 193 | - | label.SetMaxWidthChars(stackedTitleMaxWidthChars) | |
| 194 | - | label.SetHexpand(true) | |
| 195 | - | label.SetXalign(0.0) | |
| 196 | - | titleBar.Append(label) | |
| 197 | - | ||
| 198 | - | // Make title bar clickable - ensure it doesn't expand vertically | |
| 199 | - | titleButton := sv.factory.NewButton() | |
| 200 | - | titleButton.SetChild(titleBar) | |
| 201 | - | titleButton.AddCssClass("stacked-pane-title-button") | |
| 202 | - | titleButton.SetFocusOnClick(false) | |
| 203 | - | titleButton.SetVexpand(false) | |
| 204 | - | titleButton.SetHexpand(true) | |
| 230 | + | // Create title bar components | |
| 231 | + | tb := sv.createTitleBar(title, faviconIconName) | |
| 205 | 232 | ||
| 206 | 233 | pane := &stackedPane{ | |
| 207 | 234 | paneID: paneID, | |
| 208 | - | titleBar: titleBar, | |
| 235 | + | titleBar: tb.titleBar, | |
| 209 | 236 | container: container, | |
| 210 | 237 | title: title, | |
| 211 | - | favicon: favicon, | |
| 212 | - | label: label, | |
| 238 | + | favicon: tb.favicon, | |
| 239 | + | label: tb.label, | |
| 213 | 240 | isActive: false, | |
| 214 | 241 | } | |
| 215 | 242 | ||
| @@ -218,19 +245,25 @@ func (sv *StackedView) InsertPaneAfter( | |||
| 218 | 245 | copy(sv.panes[insertIndex+1:], sv.panes[insertIndex:]) | |
| 219 | 246 | sv.panes[insertIndex] = pane | |
| 220 | 247 | ||
| 221 | - | // Connect click handler - use captured index | |
| 222 | - | capturedIndex := insertIndex | |
| 223 | - | titleButton.ConnectClicked(func() { | |
| 224 | - | sv.mu.RLock() | |
| 225 | - | callback := sv.onActivate | |
| 226 | - | sv.mu.RUnlock() | |
| 227 | - | ||
| 228 | - | if callback != nil { | |
| 229 | - | callback(capturedIndex) | |
| 230 | - | } | |
| 231 | - | }) | |
| 248 | + | // Connect click handlers using paneID (not index, to handle removals) | |
| 249 | + | sv.connectTitleBarHandlers(tb, paneID) | |
| 232 | 250 | ||
| 233 | 251 | // Insert widgets at correct position in GTK box | |
| 252 | + | sv.insertTitleBarWidgets(tb.titleButton, container, insertIndex) | |
| 253 | + | ||
| 254 | + | // Set this pane as active | |
| 255 | + | sv.setActiveInternal(ctx, insertIndex) | |
| 256 | + | ||
| 257 | + | log.Debug(). | |
| 258 | + | Int("insert_index", insertIndex). | |
| 259 | + | Int("new_count", len(sv.panes)). | |
| 260 | + | Msg("StackedView.InsertPaneAfter completed") | |
| 261 | + | ||
| 262 | + | return insertIndex | |
| 263 | + | } | |
| 264 | + | ||
| 265 | + | // insertTitleBarWidgets inserts the title bar button and container at the correct position. | |
| 266 | + | func (sv *StackedView) insertTitleBarWidgets(titleButton ButtonWidget, container Widget, insertIndex int) { | |
| 234 | 267 | if insertIndex > 0 && sv.panes[insertIndex-1] != nil { | |
| 235 | 268 | // Insert after the previous pane's container | |
| 236 | 269 | prevPane := sv.panes[insertIndex-1] | |
| @@ -262,16 +295,6 @@ func (sv *StackedView) InsertPaneAfter( | |||
| 262 | 295 | } | |
| 263 | 296 | sv.box.Prepend(titleButton) | |
| 264 | 297 | } | |
| 265 | - | ||
| 266 | - | // Set this pane as active | |
| 267 | - | sv.setActiveInternal(ctx, insertIndex) | |
| 268 | - | ||
| 269 | - | log.Debug(). | |
| 270 | - | Int("insert_index", insertIndex). | |
| 271 | - | Int("new_count", len(sv.panes)). | |
| 272 | - | Msg("StackedView.InsertPaneAfter completed") | |
| 273 | - | ||
| 274 | - | return insertIndex | |
| 275 | 298 | } | |
| 276 | 299 | ||
| 277 | 300 | // RemovePane removes a pane from the stack by index. | |
| @@ -429,6 +452,12 @@ func (sv *StackedView) FindPaneIndex(paneID string) int { | |||
| 429 | 452 | sv.mu.RLock() | |
| 430 | 453 | defer sv.mu.RUnlock() | |
| 431 | 454 | ||
| 455 | + | return sv.findPaneIndexInternal(paneID) | |
| 456 | + | } | |
| 457 | + | ||
| 458 | + | // findPaneIndexInternal returns the index of a pane by ID. | |
| 459 | + | // Caller must hold at least a read lock on sv.mu. | |
| 460 | + | func (sv *StackedView) findPaneIndexInternal(paneID string) int { | |
| 432 | 461 | for i, pane := range sv.panes { | |
| 433 | 462 | if pane.paneID == paneID { | |
| 434 | 463 | return i | |
| @@ -445,6 +474,14 @@ func (sv *StackedView) SetOnActivate(fn func(index int)) { | |||
| 445 | 474 | sv.onActivate = fn | |
| 446 | 475 | } | |
| 447 | 476 | ||
| 477 | + | // SetOnClosePane sets the callback for when a pane's close button is clicked. | |
| 478 | + | func (sv *StackedView) SetOnClosePane(fn func(paneID string)) { | |
| 479 | + | sv.mu.Lock() | |
| 480 | + | defer sv.mu.Unlock() | |
| 481 | + | ||
| 482 | + | sv.onClosePane = fn | |
| 483 | + | } | |
| 484 | + | ||
| 448 | 485 | // UpdateTitle updates the title of a pane at the given index. | |
| 449 | 486 | func (sv *StackedView) UpdateTitle(index int, title string) error { | |
| 450 | 487 | sv.mu.Lock() | |
| @@ -17,7 +17,10 @@ var ErrIndexOutOfBounds = errors.New("index out of bounds") | |||||
| 17 | // ErrCannotRemoveLastPane is returned when trying to remove the last pane from a stack. | 17 | // ErrCannotRemoveLastPane is returned when trying to remove the last pane from a stack. | ||
| 18 | var ErrCannotRemoveLastPane = errors.New("cannot remove last pane from stack") | 18 | var ErrCannotRemoveLastPane = errors.New("cannot remove last pane from stack") | ||
| 19 | 19 | ||||
| 20 | - | const stackedTitleMaxWidthChars = 30 | 20 | + | const ( |
| 21 | + | stackedTitleMaxWidthChars = 30 | |||
| 22 | + | stackedPaneCloseIcon = "window-close-symbolic" | |||
| 23 | + | ) | |||
| 21 | 24 | ||||
| 22 | // stackedPane represents a single pane within a stacked container. | 25 | // stackedPane represents a single pane within a stacked container. | ||
| 23 | type stackedPane struct { | 26 | type stackedPane struct { | ||
| @@ -38,7 +41,8 @@ type StackedView struct { | |||||
| 38 | panes []*stackedPane | 41 | panes []*stackedPane | ||
| 39 | activeIndex int | 42 | activeIndex int | ||
| 40 | 43 | ||||
| 41 | - | onActivate func(index int) // called when a pane is activated via title bar click | 44 | + | onActivate func(index int) // called when a pane is activated via title bar click |
| 45 | + | onClosePane func(paneID string) // called when a pane's close button is clicked | |||
| 42 | 46 | ||||
| 43 | mu sync.RWMutex | 47 | mu sync.RWMutex | ||
| 44 | } | 48 | } | ||
| @@ -57,20 +61,18 @@ func NewStackedView(factory WidgetFactory) *StackedView { | |||||
| 57 | } | 61 | } | ||
| 58 | } | 62 | } | ||
| 59 | 63 | ||||
| 60 | - | // AddPane adds a new pane to the stack. | 64 | + | // titleBarComponents holds the widgets created for a pane's title bar. |
| 61 | - | // The new pane becomes active (visible). | 65 | + | type titleBarComponents struct { |
| 62 | - | func (sv *StackedView) AddPane(ctx context.Context, paneID, title, faviconIconName string, container Widget) int { | 66 | + | titleBar BoxWidget |
| 63 | - | log := logging.FromContext(ctx) | 67 | + | titleButton ButtonWidget |
| 64 | - | sv.mu.Lock() | 68 | + | closeBtn ButtonWidget |
| 65 | - | defer sv.mu.Unlock() | 69 | + | favicon ImageWidget |
| 66 | - | 70 | + | label LabelWidget | |
| 67 | - | log.Debug(). | 71 | + | } |
| 68 | - | Str("title", title). | |||
| 69 | - | Bool("container_nil", container == nil). | |||
| 70 | - | Int("current_count", len(sv.panes)). | |||
| 71 | - | Msg("StackedView.AddPane called") | |||
| 72 | 72 | ||||
| 73 | - | // Create title bar - must not expand vertically | 73 | + | // createTitleBar creates the title bar widgets for a pane. |
| 74 | + | func (sv *StackedView) createTitleBar(title, faviconIconName string) titleBarComponents { | |||
| 75 | + | // Create title bar container - must not expand vertically | |||
| 74 | titleBar := sv.factory.NewBox(OrientationHorizontal, 4) | 76 | titleBar := sv.factory.NewBox(OrientationHorizontal, 4) | ||
| 75 | titleBar.AddCssClass("stacked-pane-titlebar") | 77 | titleBar.AddCssClass("stacked-pane-titlebar") | ||
| 76 | titleBar.SetVexpand(false) | 78 | titleBar.SetVexpand(false) | ||
| @@ -93,6 +95,15 @@ func (sv *StackedView) AddPane(ctx context.Context, paneID, title, faviconIconNa | |||||
| 93 | label.SetXalign(0.0) | 95 | label.SetXalign(0.0) | ||
| 94 | titleBar.Append(label) | 96 | titleBar.Append(label) | ||
| 95 | 97 | ||||
| 98 | + | // Create close button using GTK's native icon button support | |||
| 99 | + | closeBtn := sv.factory.NewButton() | |||
| 100 | + | closeBtn.SetIconName(stackedPaneCloseIcon) | |||
| 101 | + | closeBtn.AddCssClass("stacked-pane-close-button") | |||
| 102 | + | closeBtn.SetFocusOnClick(false) | |||
| 103 | + | closeBtn.SetVexpand(false) | |||
| 104 | + | closeBtn.SetHexpand(false) | |||
| 105 | + | titleBar.Append(closeBtn) | |||
| 106 | + | ||||
| 96 | // Make title bar clickable - ensure it doesn't expand vertically | 107 | // Make title bar clickable - ensure it doesn't expand vertically | ||
| 97 | titleButton := sv.factory.NewButton() | 108 | titleButton := sv.factory.NewButton() | ||
| 98 | titleButton.SetChild(titleBar) | 109 | titleButton.SetChild(titleBar) | ||
| @@ -101,35 +112,52 @@ func (sv *StackedView) AddPane(ctx context.Context, paneID, title, faviconIconNa | |||||
| 101 | titleButton.SetVexpand(false) // Critical: don't let title bar expand vertically | 112 | titleButton.SetVexpand(false) // Critical: don't let title bar expand vertically | ||
| 102 | titleButton.SetHexpand(true) // But fill horizontal space | 113 | titleButton.SetHexpand(true) // But fill horizontal space | ||
| 103 | 114 | ||||
| 115 | + | return titleBarComponents{ | |||
| 116 | + | titleBar: titleBar, | |||
| 117 | + | titleButton: titleButton, | |||
| 118 | + | closeBtn: closeBtn, | |||
| 119 | + | favicon: favicon, | |||
| 120 | + | label: label, | |||
| 121 | + | } | |||
| 122 | + | } | |||
| 123 | + | ||||
| 124 | + | // AddPane adds a new pane to the stack. | |||
| 125 | + | // The new pane becomes active (visible). | |||
| 126 | + | func (sv *StackedView) AddPane(ctx context.Context, paneID, title, faviconIconName string, container Widget) int { | |||
| 127 | + | log := logging.FromContext(ctx) | |||
| 128 | + | sv.mu.Lock() | |||
| 129 | + | defer sv.mu.Unlock() | |||
| 130 | + | ||||
| 131 | + | log.Debug(). | |||
| 132 | + | Str("title", title). | |||
| 133 | + | Bool("container_nil", container == nil). | |||
| 134 | + | Int("current_count", len(sv.panes)). | |||
| 135 | + | Msg("StackedView.AddPane called") | |||
| 136 | + | ||||
| 137 | + | // Create title bar components | |||
| 138 | + | tb := sv.createTitleBar(title, faviconIconName) | |||
| 139 | + | ||||
| 104 | pane := &stackedPane{ | 140 | pane := &stackedPane{ | ||
| 105 | paneID: paneID, | 141 | paneID: paneID, | ||
| 106 | - | titleBar: titleBar, | 142 | + | titleBar: tb.titleBar, |
| 107 | container: container, | 143 | container: container, | ||
| 108 | title: title, | 144 | title: title, | ||
| 109 | - | favicon: favicon, | 145 | + | favicon: tb.favicon, |
| 110 | - | label: label, | 146 | + | label: tb.label, |
| 111 | isActive: false, | 147 | isActive: false, | ||
| 112 | } | 148 | } | ||
| 113 | 149 | ||||
| 114 | index := len(sv.panes) | 150 | index := len(sv.panes) | ||
| 115 | sv.panes = append(sv.panes, pane) | 151 | sv.panes = append(sv.panes, pane) | ||
| 116 | 152 | ||||
| 117 | - | // Connect click handler | 153 | + | // Connect click handlers using paneID (not index, to handle removals) |
| 118 | - | titleButton.ConnectClicked(func() { | 154 | + | sv.connectTitleBarHandlers(tb, paneID) |
| 119 | - | sv.mu.RLock() | |||
| 120 | - | callback := sv.onActivate | |||
| 121 | - | sv.mu.RUnlock() | |||
| 122 | - | ||||
| 123 | - | if callback != nil { | |||
| 124 | - | callback(index) | |||
| 125 | - | } | |||
| 126 | - | }) | |||
| 127 | 155 | ||||
| 128 | // Add to container | 156 | // Add to container | ||
| 129 | log.Debug(). | 157 | log.Debug(). | ||
| 130 | Int("index", index). | 158 | Int("index", index). | ||
| 131 | Msg("StackedView: appending titleButton and container to box") | 159 | Msg("StackedView: appending titleButton and container to box") | ||
| 132 | - | sv.box.Append(titleButton) | 160 | + | sv.box.Append(tb.titleButton) |
| 133 | if container != nil { | 161 | if container != nil { | ||
| 134 | sv.box.Append(container) | 162 | sv.box.Append(container) | ||
| 135 | } | 163 | } | ||
| @@ -145,6 +173,33 @@ func (sv *StackedView) AddPane(ctx context.Context, paneID, title, faviconIconNa | |||||
| 145 | return index | 173 | return index | ||
| 146 | } | 174 | } | ||
| 147 | 175 | ||||
| 176 | + | // connectTitleBarHandlers connects the click handlers for a title bar. | |||
| 177 | + | // Uses paneID lookup instead of captured index to handle pane removals correctly. | |||
| 178 | + | func (sv *StackedView) connectTitleBarHandlers(tb titleBarComponents, paneID string) { | |||
| 179 | + | // Connect title bar click handler for activation | |||
| 180 | + | tb.titleButton.ConnectClicked(func() { | |||
| 181 | + | sv.mu.RLock() | |||
| 182 | + | callback := sv.onActivate | |||
| 183 | + | currentIndex := sv.findPaneIndexInternal(paneID) | |||
| 184 | + | sv.mu.RUnlock() | |||
| 185 | + | ||||
| 186 | + | if callback != nil && currentIndex >= 0 { | |||
| 187 | + | callback(currentIndex) | |||
| 188 | + | } | |||
| 189 | + | }) | |||
| 190 | + | ||||
| 191 | + | // Connect close button click handler | |||
| 192 | + | tb.closeBtn.ConnectClicked(func() { | |||
| 193 | + | sv.mu.RLock() | |||
| 194 | + | onClose := sv.onClosePane | |||
| 195 | + | sv.mu.RUnlock() | |||
| 196 | + | ||||
| 197 | + | if onClose != nil { | |||
| 198 | + | onClose(paneID) | |||
| 199 | + | } | |||
| 200 | + | }) | |||
| 201 | + | } | |||
| 202 | + | ||||
| 148 | // InsertPaneAfter inserts a new pane after the specified index position. | 203 | // InsertPaneAfter inserts a new pane after the specified index position. | ||
| 149 | // Use afterIndex=-1 to insert at the beginning. | 204 | // Use afterIndex=-1 to insert at the beginning. | ||
| 150 | // The new pane becomes active (visible). | 205 | // The new pane becomes active (visible). | ||
| @@ -172,44 +227,16 @@ func (sv *StackedView) InsertPaneAfter( | |||||
| 172 | Int("current_count", len(sv.panes)). | 227 | Int("current_count", len(sv.panes)). | ||
| 173 | Msg("StackedView.InsertPaneAfter called") | 228 | Msg("StackedView.InsertPaneAfter called") | ||
| 174 | 229 | ||||
| 175 | - | // Create title bar - must not expand vertically | 230 | + | // Create title bar components |
| 176 | - | titleBar := sv.factory.NewBox(OrientationHorizontal, 4) | 231 | + | tb := sv.createTitleBar(title, faviconIconName) |
| 177 | - | titleBar.AddCssClass("stacked-pane-titlebar") | |||
| 178 | - | titleBar.SetVexpand(false) | |||
| 179 | - | ||||
| 180 | - | // Create favicon image | |||
| 181 | - | favicon := sv.factory.NewImage() | |||
| 182 | - | if faviconIconName != "" { | |||
| 183 | - | favicon.SetFromIconName(faviconIconName) | |||
| 184 | - | } else { | |||
| 185 | - | favicon.SetFromIconName("web-browser-symbolic") | |||
| 186 | - | } | |||
| 187 | - | favicon.SetPixelSize(16) | |||
| 188 | - | titleBar.Append(favicon) | |||
| 189 | - | ||||
| 190 | - | // Create title label | |||
| 191 | - | label := sv.factory.NewLabel(title) | |||
| 192 | - | label.SetEllipsize(EllipsizeEnd) | |||
| 193 | - | label.SetMaxWidthChars(stackedTitleMaxWidthChars) | |||
| 194 | - | label.SetHexpand(true) | |||
| 195 | - | label.SetXalign(0.0) | |||
| 196 | - | titleBar.Append(label) | |||
| 197 | - | ||||
| 198 | - | // Make title bar clickable - ensure it doesn't expand vertically | |||
| 199 | - | titleButton := sv.factory.NewButton() | |||
| 200 | - | titleButton.SetChild(titleBar) | |||
| 201 | - | titleButton.AddCssClass("stacked-pane-title-button") | |||
| 202 | - | titleButton.SetFocusOnClick(false) | |||
| 203 | - | titleButton.SetVexpand(false) | |||
| 204 | - | titleButton.SetHexpand(true) | |||
| 205 | 232 | ||||
| 206 | pane := &stackedPane{ | 233 | pane := &stackedPane{ | ||
| 207 | paneID: paneID, | 234 | paneID: paneID, | ||
| 208 | - | titleBar: titleBar, | 235 | + | titleBar: tb.titleBar, |
| 209 | container: container, | 236 | container: container, | ||
| 210 | title: title, | 237 | title: title, | ||
| 211 | - | favicon: favicon, | 238 | + | favicon: tb.favicon, |
| 212 | - | label: label, | 239 | + | label: tb.label, |
| 213 | isActive: false, | 240 | isActive: false, | ||
| 214 | } | 241 | } | ||
| 215 | 242 | ||||
| @@ -218,19 +245,25 @@ func (sv *StackedView) InsertPaneAfter( | |||||
| 218 | copy(sv.panes[insertIndex+1:], sv.panes[insertIndex:]) | 245 | copy(sv.panes[insertIndex+1:], sv.panes[insertIndex:]) | ||
| 219 | sv.panes[insertIndex] = pane | 246 | sv.panes[insertIndex] = pane | ||
| 220 | 247 | ||||
| 221 | - | // Connect click handler - use captured index | 248 | + | // Connect click handlers using paneID (not index, to handle removals) |
| 222 | - | capturedIndex := insertIndex | 249 | + | sv.connectTitleBarHandlers(tb, paneID) |
| 223 | - | titleButton.ConnectClicked(func() { | |||
| 224 | - | sv.mu.RLock() | |||
| 225 | - | callback := sv.onActivate | |||
| 226 | - | sv.mu.RUnlock() | |||
| 227 | - | ||||
| 228 | - | if callback != nil { | |||
| 229 | - | callback(capturedIndex) | |||
| 230 | - | } | |||
| 231 | - | }) | |||
| 232 | 250 | ||||
| 233 | // Insert widgets at correct position in GTK box | 251 | // Insert widgets at correct position in GTK box | ||
| 252 | + | sv.insertTitleBarWidgets(tb.titleButton, container, insertIndex) | |||
| 253 | + | ||||
| 254 | + | // Set this pane as active | |||
| 255 | + | sv.setActiveInternal(ctx, insertIndex) | |||
| 256 | + | ||||
| 257 | + | log.Debug(). | |||
| 258 | + | Int("insert_index", insertIndex). | |||
| 259 | + | Int("new_count", len(sv.panes)). | |||
| 260 | + | Msg("StackedView.InsertPaneAfter completed") | |||
| 261 | + | ||||
| 262 | + | return insertIndex | |||
| 263 | + | } | |||
| 264 | + | ||||
| 265 | + | // insertTitleBarWidgets inserts the title bar button and container at the correct position. | |||
| 266 | + | func (sv *StackedView) insertTitleBarWidgets(titleButton ButtonWidget, container Widget, insertIndex int) { | |||
| 234 | if insertIndex > 0 && sv.panes[insertIndex-1] != nil { | 267 | if insertIndex > 0 && sv.panes[insertIndex-1] != nil { | ||
| 235 | // Insert after the previous pane's container | 268 | // Insert after the previous pane's container | ||
| 236 | prevPane := sv.panes[insertIndex-1] | 269 | prevPane := sv.panes[insertIndex-1] | ||
| @@ -262,16 +295,6 @@ func (sv *StackedView) InsertPaneAfter( | |||||
| 262 | } | 295 | } | ||
| 263 | sv.box.Prepend(titleButton) | 296 | sv.box.Prepend(titleButton) | ||
| 264 | } | 297 | } | ||
| 265 | - | ||||
| 266 | - | // Set this pane as active | |||
| 267 | - | sv.setActiveInternal(ctx, insertIndex) | |||
| 268 | - | ||||
| 269 | - | log.Debug(). | |||
| 270 | - | Int("insert_index", insertIndex). | |||
| 271 | - | Int("new_count", len(sv.panes)). | |||
| 272 | - | Msg("StackedView.InsertPaneAfter completed") | |||
| 273 | - | ||||
| 274 | - | return insertIndex | |||
| 275 | } | 298 | } | ||
| 276 | 299 | ||||
| 277 | // RemovePane removes a pane from the stack by index. | 300 | // RemovePane removes a pane from the stack by index. | ||
| @@ -429,6 +452,12 @@ func (sv *StackedView) FindPaneIndex(paneID string) int { | |||||
| 429 | sv.mu.RLock() | 452 | sv.mu.RLock() | ||
| 430 | defer sv.mu.RUnlock() | 453 | defer sv.mu.RUnlock() | ||
| 431 | 454 | ||||
| 455 | + | return sv.findPaneIndexInternal(paneID) | |||
| 456 | + | } | |||
| 457 | + | ||||
| 458 | + | // findPaneIndexInternal returns the index of a pane by ID. | |||
| 459 | + | // Caller must hold at least a read lock on sv.mu. | |||
| 460 | + | func (sv *StackedView) findPaneIndexInternal(paneID string) int { | |||
| 432 | for i, pane := range sv.panes { | 461 | for i, pane := range sv.panes { | ||
| 433 | if pane.paneID == paneID { | 462 | if pane.paneID == paneID { | ||
| 434 | return i | 463 | return i | ||
| @@ -445,6 +474,14 @@ func (sv *StackedView) SetOnActivate(fn func(index int)) { | |||||
| 445 | sv.onActivate = fn | 474 | sv.onActivate = fn | ||
| 446 | } | 475 | } | ||
| 447 | 476 | ||||
| 477 | + | // SetOnClosePane sets the callback for when a pane's close button is clicked. | |||
| 478 | + | func (sv *StackedView) SetOnClosePane(fn func(paneID string)) { | |||
| 479 | + | sv.mu.Lock() | |||
| 480 | + | defer sv.mu.Unlock() | |||
| 481 | + | ||||
| 482 | + | sv.onClosePane = fn | |||
| 483 | + | } | |||
| 484 | + | ||||
| 448 | // UpdateTitle updates the title of a pane at the given index. | 485 | // UpdateTitle updates the title of a pane at the given index. | ||
| 449 | func (sv *StackedView) UpdateTitle(index int, title string) error { | 486 | func (sv *StackedView) UpdateTitle(index int, title string) error { | ||
| 450 | sv.mu.Lock() | 487 | sv.mu.Lock() | ||
M internal/ui/layout/stacked_test.go +26 -0
| @@ -32,6 +32,7 @@ func setupPaneMocks(t *testing.T, mockFactory *mocks.MockWidgetFactory, mockBox | |||
| 32 | 32 | mockFavicon := mocks.NewMockImageWidget(t) | |
| 33 | 33 | mockLabel := mocks.NewMockLabelWidget(t) | |
| 34 | 34 | mockButton := mocks.NewMockButtonWidget(t) | |
| 35 | + | mockCloseButton := mocks.NewMockButtonWidget(t) | |
| 35 | 36 | mockContainer := mocks.NewMockWidget(t) | |
| 36 | 37 | ||
| 37 | 38 | // Title bar creation | |
| @@ -53,6 +54,15 @@ func setupPaneMocks(t *testing.T, mockFactory *mocks.MockWidgetFactory, mockBox | |||
| 53 | 54 | mockLabel.EXPECT().SetXalign(float32(0.0)).Once() | |
| 54 | 55 | mockTitleBar.EXPECT().Append(mockLabel).Once() | |
| 55 | 56 | ||
| 57 | + | // Close button (uses SetIconName directly instead of child image) | |
| 58 | + | mockFactory.EXPECT().NewButton().Return(mockCloseButton).Once() | |
| 59 | + | mockCloseButton.EXPECT().SetIconName("window-close-symbolic").Once() | |
| 60 | + | mockCloseButton.EXPECT().AddCssClass("stacked-pane-close-button").Once() | |
| 61 | + | mockCloseButton.EXPECT().SetFocusOnClick(false).Once() | |
| 62 | + | mockCloseButton.EXPECT().SetVexpand(false).Once() | |
| 63 | + | mockCloseButton.EXPECT().SetHexpand(false).Once() | |
| 64 | + | mockTitleBar.EXPECT().Append(mockCloseButton).Once() | |
| 65 | + | ||
| 56 | 66 | // Button wrapping title bar | |
| 57 | 67 | mockFactory.EXPECT().NewButton().Return(mockButton).Once() | |
| 58 | 68 | mockButton.EXPECT().SetChild(mockTitleBar).Once() | |
| @@ -60,7 +70,10 @@ func setupPaneMocks(t *testing.T, mockFactory *mocks.MockWidgetFactory, mockBox | |||
| 60 | 70 | mockButton.EXPECT().SetFocusOnClick(false).Once() | |
| 61 | 71 | mockButton.EXPECT().SetVexpand(false).Once() | |
| 62 | 72 | mockButton.EXPECT().SetHexpand(true).Once() | |
| 73 | + | ||
| 74 | + | // Click handlers (title bar activation and close button) | |
| 63 | 75 | mockButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(1)).Once() | |
| 76 | + | mockCloseButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(2)).Once() | |
| 64 | 77 | ||
| 65 | 78 | // Adding to main box | |
| 66 | 79 | mockBox.EXPECT().Append(mockButton).Once() | |
| @@ -559,6 +572,7 @@ func setupInsertPaneMocks( | |||
| 559 | 572 | mockFavicon := mocks.NewMockImageWidget(t) | |
| 560 | 573 | mockLabel := mocks.NewMockLabelWidget(t) | |
| 561 | 574 | mockButton := mocks.NewMockButtonWidget(t) | |
| 575 | + | mockCloseButton := mocks.NewMockButtonWidget(t) | |
| 562 | 576 | mockContainer := mocks.NewMockWidget(t) | |
| 563 | 577 | ||
| 564 | 578 | // Title bar creation | |
| @@ -580,6 +594,15 @@ func setupInsertPaneMocks( | |||
| 580 | 594 | mockLabel.EXPECT().SetXalign(float32(0.0)).Once() | |
| 581 | 595 | mockTitleBar.EXPECT().Append(mockLabel).Once() | |
| 582 | 596 | ||
| 597 | + | // Close button (uses SetIconName directly instead of child image) | |
| 598 | + | mockFactory.EXPECT().NewButton().Return(mockCloseButton).Once() | |
| 599 | + | mockCloseButton.EXPECT().SetIconName("window-close-symbolic").Once() | |
| 600 | + | mockCloseButton.EXPECT().AddCssClass("stacked-pane-close-button").Once() | |
| 601 | + | mockCloseButton.EXPECT().SetFocusOnClick(false).Once() | |
| 602 | + | mockCloseButton.EXPECT().SetVexpand(false).Once() | |
| 603 | + | mockCloseButton.EXPECT().SetHexpand(false).Once() | |
| 604 | + | mockTitleBar.EXPECT().Append(mockCloseButton).Once() | |
| 605 | + | ||
| 583 | 606 | // Button wrapping title bar | |
| 584 | 607 | mockFactory.EXPECT().NewButton().Return(mockButton).Once() | |
| 585 | 608 | mockButton.EXPECT().SetChild(mockTitleBar).Once() | |
| @@ -587,7 +610,10 @@ func setupInsertPaneMocks( | |||
| 587 | 610 | mockButton.EXPECT().SetFocusOnClick(false).Once() | |
| 588 | 611 | mockButton.EXPECT().SetVexpand(false).Once() | |
| 589 | 612 | mockButton.EXPECT().SetHexpand(true).Once() | |
| 613 | + | ||
| 614 | + | // Click handlers (title bar activation and close button) | |
| 590 | 615 | mockButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(1)).Once() | |
| 616 | + | mockCloseButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(2)).Once() | |
| 591 | 617 | ||
| 592 | 618 | // Position-aware insertion using InsertChildAfter | |
| 593 | 619 | if siblingContainer != nil { | |
| @@ -32,6 +32,7 @@ func setupPaneMocks(t *testing.T, mockFactory *mocks.MockWidgetFactory, mockBox | |||||
| 32 | mockFavicon := mocks.NewMockImageWidget(t) | 32 | mockFavicon := mocks.NewMockImageWidget(t) | ||
| 33 | mockLabel := mocks.NewMockLabelWidget(t) | 33 | mockLabel := mocks.NewMockLabelWidget(t) | ||
| 34 | mockButton := mocks.NewMockButtonWidget(t) | 34 | mockButton := mocks.NewMockButtonWidget(t) | ||
| 35 | + | mockCloseButton := mocks.NewMockButtonWidget(t) | |||
| 35 | mockContainer := mocks.NewMockWidget(t) | 36 | mockContainer := mocks.NewMockWidget(t) | ||
| 36 | 37 | ||||
| 37 | // Title bar creation | 38 | // Title bar creation | ||
| @@ -53,6 +54,15 @@ func setupPaneMocks(t *testing.T, mockFactory *mocks.MockWidgetFactory, mockBox | |||||
| 53 | mockLabel.EXPECT().SetXalign(float32(0.0)).Once() | 54 | mockLabel.EXPECT().SetXalign(float32(0.0)).Once() | ||
| 54 | mockTitleBar.EXPECT().Append(mockLabel).Once() | 55 | mockTitleBar.EXPECT().Append(mockLabel).Once() | ||
| 55 | 56 | ||||
| 57 | + | // Close button (uses SetIconName directly instead of child image) | |||
| 58 | + | mockFactory.EXPECT().NewButton().Return(mockCloseButton).Once() | |||
| 59 | + | mockCloseButton.EXPECT().SetIconName("window-close-symbolic").Once() | |||
| 60 | + | mockCloseButton.EXPECT().AddCssClass("stacked-pane-close-button").Once() | |||
| 61 | + | mockCloseButton.EXPECT().SetFocusOnClick(false).Once() | |||
| 62 | + | mockCloseButton.EXPECT().SetVexpand(false).Once() | |||
| 63 | + | mockCloseButton.EXPECT().SetHexpand(false).Once() | |||
| 64 | + | mockTitleBar.EXPECT().Append(mockCloseButton).Once() | |||
| 65 | + | ||||
| 56 | // Button wrapping title bar | 66 | // Button wrapping title bar | ||
| 57 | mockFactory.EXPECT().NewButton().Return(mockButton).Once() | 67 | mockFactory.EXPECT().NewButton().Return(mockButton).Once() | ||
| 58 | mockButton.EXPECT().SetChild(mockTitleBar).Once() | 68 | mockButton.EXPECT().SetChild(mockTitleBar).Once() | ||
| @@ -60,7 +70,10 @@ func setupPaneMocks(t *testing.T, mockFactory *mocks.MockWidgetFactory, mockBox | |||||
| 60 | mockButton.EXPECT().SetFocusOnClick(false).Once() | 70 | mockButton.EXPECT().SetFocusOnClick(false).Once() | ||
| 61 | mockButton.EXPECT().SetVexpand(false).Once() | 71 | mockButton.EXPECT().SetVexpand(false).Once() | ||
| 62 | mockButton.EXPECT().SetHexpand(true).Once() | 72 | mockButton.EXPECT().SetHexpand(true).Once() | ||
| 73 | + | ||||
| 74 | + | // Click handlers (title bar activation and close button) | |||
| 63 | mockButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(1)).Once() | 75 | mockButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(1)).Once() | ||
| 76 | + | mockCloseButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(2)).Once() | |||
| 64 | 77 | ||||
| 65 | // Adding to main box | 78 | // Adding to main box | ||
| 66 | mockBox.EXPECT().Append(mockButton).Once() | 79 | mockBox.EXPECT().Append(mockButton).Once() | ||
| @@ -559,6 +572,7 @@ func setupInsertPaneMocks( | |||||
| 559 | mockFavicon := mocks.NewMockImageWidget(t) | 572 | mockFavicon := mocks.NewMockImageWidget(t) | ||
| 560 | mockLabel := mocks.NewMockLabelWidget(t) | 573 | mockLabel := mocks.NewMockLabelWidget(t) | ||
| 561 | mockButton := mocks.NewMockButtonWidget(t) | 574 | mockButton := mocks.NewMockButtonWidget(t) | ||
| 575 | + | mockCloseButton := mocks.NewMockButtonWidget(t) | |||
| 562 | mockContainer := mocks.NewMockWidget(t) | 576 | mockContainer := mocks.NewMockWidget(t) | ||
| 563 | 577 | ||||
| 564 | // Title bar creation | 578 | // Title bar creation | ||
| @@ -580,6 +594,15 @@ func setupInsertPaneMocks( | |||||
| 580 | mockLabel.EXPECT().SetXalign(float32(0.0)).Once() | 594 | mockLabel.EXPECT().SetXalign(float32(0.0)).Once() | ||
| 581 | mockTitleBar.EXPECT().Append(mockLabel).Once() | 595 | mockTitleBar.EXPECT().Append(mockLabel).Once() | ||
| 582 | 596 | ||||
| 597 | + | // Close button (uses SetIconName directly instead of child image) | |||
| 598 | + | mockFactory.EXPECT().NewButton().Return(mockCloseButton).Once() | |||
| 599 | + | mockCloseButton.EXPECT().SetIconName("window-close-symbolic").Once() | |||
| 600 | + | mockCloseButton.EXPECT().AddCssClass("stacked-pane-close-button").Once() | |||
| 601 | + | mockCloseButton.EXPECT().SetFocusOnClick(false).Once() | |||
| 602 | + | mockCloseButton.EXPECT().SetVexpand(false).Once() | |||
| 603 | + | mockCloseButton.EXPECT().SetHexpand(false).Once() | |||
| 604 | + | mockTitleBar.EXPECT().Append(mockCloseButton).Once() | |||
| 605 | + | ||||
| 583 | // Button wrapping title bar | 606 | // Button wrapping title bar | ||
| 584 | mockFactory.EXPECT().NewButton().Return(mockButton).Once() | 607 | mockFactory.EXPECT().NewButton().Return(mockButton).Once() | ||
| 585 | mockButton.EXPECT().SetChild(mockTitleBar).Once() | 608 | mockButton.EXPECT().SetChild(mockTitleBar).Once() | ||
| @@ -587,7 +610,10 @@ func setupInsertPaneMocks( | |||||
| 587 | mockButton.EXPECT().SetFocusOnClick(false).Once() | 610 | mockButton.EXPECT().SetFocusOnClick(false).Once() | ||
| 588 | mockButton.EXPECT().SetVexpand(false).Once() | 611 | mockButton.EXPECT().SetVexpand(false).Once() | ||
| 589 | mockButton.EXPECT().SetHexpand(true).Once() | 612 | mockButton.EXPECT().SetHexpand(true).Once() | ||
| 613 | + | ||||
| 614 | + | // Click handlers (title bar activation and close button) | |||
| 590 | mockButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(1)).Once() | 615 | mockButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(1)).Once() | ||
| 616 | + | mockCloseButton.EXPECT().ConnectClicked(mock.Anything).Return(uint32(2)).Once() | |||
| 591 | 617 | ||||
| 592 | // Position-aware insertion using InsertChildAfter | 618 | // Position-aware insertion using InsertChildAfter | ||
| 593 | if siblingContainer != nil { | 619 | if siblingContainer != nil { | ||
M internal/ui/layout/widgets.go +1 -0
| @@ -179,6 +179,7 @@ type ButtonWidget interface { | |||
| 179 | 179 | GetLabel() string | |
| 180 | 180 | SetChild(child Widget) | |
| 181 | 181 | GetChild() Widget | |
| 182 | + | SetIconName(iconName string) | |
| 182 | 183 | ||
| 183 | 184 | // Connect click handler, returns signal ID for disconnection | |
| 184 | 185 | ConnectClicked(callback func()) uint32 | |
| @@ -179,6 +179,7 @@ type ButtonWidget interface { | |||||
| 179 | GetLabel() string | 179 | GetLabel() string | ||
| 180 | SetChild(child Widget) | 180 | SetChild(child Widget) | ||
| 181 | GetChild() Widget | 181 | GetChild() Widget | ||
| 182 | + | SetIconName(iconName string) | |||
| 182 | 183 | ||||
| 183 | // Connect click handler, returns signal ID for disconnection | 184 | // Connect click handler, returns signal ID for disconnection | ||
| 184 | ConnectClicked(callback func()) uint32 | 185 | ConnectClicked(callback func()) uint32 | ||
M internal/ui/theme/css.go +37 -0
| @@ -623,6 +623,43 @@ button.stacked-pane-title-button:focus { | |||
| 623 | 623 | button.stacked-pane-title-button:hover .stacked-pane-titlebar label { | |
| 624 | 624 | color: var(--accent); | |
| 625 | 625 | } | |
| 626 | + | ||
| 627 | + | /* Close button in stacked pane title bar */ | |
| 628 | + | .stacked-pane-close-button { | |
| 629 | + | background: transparent; | |
| 630 | + | border: none; | |
| 631 | + | border-radius: 4px; | |
| 632 | + | padding: 4px; | |
| 633 | + | margin: 0 2px; | |
| 634 | + | min-width: 20px; | |
| 635 | + | min-height: 20px; | |
| 636 | + | opacity: 0.4; | |
| 637 | + | transition: opacity 100ms ease-in-out, background-color 100ms ease-in-out; | |
| 638 | + | } | |
| 639 | + | ||
| 640 | + | .stacked-pane-close-button > image { | |
| 641 | + | margin: 0; | |
| 642 | + | padding: 0; | |
| 643 | + | } | |
| 644 | + | ||
| 645 | + | .stacked-pane-close-button:hover { | |
| 646 | + | opacity: 1.0; | |
| 647 | + | background: alpha(var(--destructive), 0.2); | |
| 648 | + | } | |
| 649 | + | ||
| 650 | + | .stacked-pane-close-button:hover > image { | |
| 651 | + | color: var(--destructive); | |
| 652 | + | } | |
| 653 | + | ||
| 654 | + | /* Show close button more prominently when hovering title bar */ | |
| 655 | + | button.stacked-pane-title-button:hover .stacked-pane-close-button { | |
| 656 | + | opacity: 0.7; | |
| 657 | + | } | |
| 658 | + | ||
| 659 | + | .stacked-pane-close-button:focus { | |
| 660 | + | outline: none; | |
| 661 | + | box-shadow: none; | |
| 662 | + | } | |
| 626 | 663 | ` | |
| 627 | 664 | } | |
| 628 | 665 | ||
| @@ -623,6 +623,43 @@ button.stacked-pane-title-button:focus { | |||||
| 623 | button.stacked-pane-title-button:hover .stacked-pane-titlebar label { | 623 | button.stacked-pane-title-button:hover .stacked-pane-titlebar label { | ||
| 624 | color: var(--accent); | 624 | color: var(--accent); | ||
| 625 | } | 625 | } | ||
| 626 | + | ||||
| 627 | + | /* Close button in stacked pane title bar */ | |||
| 628 | + | .stacked-pane-close-button { | |||
| 629 | + | background: transparent; | |||
| 630 | + | border: none; | |||
| 631 | + | border-radius: 4px; | |||
| 632 | + | padding: 4px; | |||
| 633 | + | margin: 0 2px; | |||
| 634 | + | min-width: 20px; | |||
| 635 | + | min-height: 20px; | |||
| 636 | + | opacity: 0.4; | |||
| 637 | + | transition: opacity 100ms ease-in-out, background-color 100ms ease-in-out; | |||
| 638 | + | } | |||
| 639 | + | ||||
| 640 | + | .stacked-pane-close-button > image { | |||
| 641 | + | margin: 0; | |||
| 642 | + | padding: 0; | |||
| 643 | + | } | |||
| 644 | + | ||||
| 645 | + | .stacked-pane-close-button:hover { | |||
| 646 | + | opacity: 1.0; | |||
| 647 | + | background: alpha(var(--destructive), 0.2); | |||
| 648 | + | } | |||
| 649 | + | ||||
| 650 | + | .stacked-pane-close-button:hover > image { | |||
| 651 | + | color: var(--destructive); | |||
| 652 | + | } | |||
| 653 | + | ||||
| 654 | + | /* Show close button more prominently when hovering title bar */ | |||
| 655 | + | button.stacked-pane-title-button:hover .stacked-pane-close-button { | |||
| 656 | + | opacity: 0.7; | |||
| 657 | + | } | |||
| 658 | + | ||||
| 659 | + | .stacked-pane-close-button:focus { | |||
| 660 | + | outline: none; | |||
| 661 | + | box-shadow: none; | |||
| 662 | + | } | |||
| 626 | ` | 663 | ` | ||
| 627 | } | 664 | } | ||
| 628 | 665 | ||||