refactor to reduce complexity of nesting

pyPhotoAlbum/mixins/mouse_interaction.py

@@ -25,210 +25,227 @@ class MouseInteractionMixin:
         self.is_dragging = False
         self.is_panning = False
 
+    def _handle_rotation_start(self, x: float, y: float):
+        """Start rotation interaction for selected element."""
+        self._begin_rotate(self.selected_element)
+        self.drag_start_pos = (x, y)
+        self.rotation_start_angle = self.selected_element.rotation
+        self.is_dragging = True
+
+    def _handle_resize_start(self, x: float, y: float, handle):
+        """Start resize interaction for selected element."""
+        self._begin_resize(self.selected_element)
+        self.resize_handle = handle
+        self.drag_start_pos = (x, y)
+        self.resize_start_pos = self.selected_element.position
+        self.resize_start_size = self.selected_element.size
+        self.is_dragging = True
+
+    def _handle_image_pan_start(self, x: float, y: float, element):
+        """Start image pan mode for an ImageData element."""
+        self.selected_elements = {element}
+        self.drag_start_pos = (x, y)
+        self.image_pan_mode = True
+        self.image_pan_start_crop = element.crop_info
+        self._begin_image_pan(element)
+        self.is_dragging = True
+        self.setCursor(Qt.CursorShape.SizeAllCursor)
+
+    def _handle_multi_select(self, element):
+        """Toggle element in multi-selection."""
+        if element in self.selected_elements:
+            self.selected_elements.remove(element)
+        else:
+            self.selected_elements.add(element)
+
+    def _handle_element_drag_start(self, x: float, y: float, element):
+        """Start dragging an element."""
+        self.selected_elements = {element}
+        self.drag_start_pos = (x, y)
+        self.drag_start_element_pos = element.position
+        if not self.rotation_mode:
+            self._begin_move(element)
+            self.is_dragging = True
+
     def mousePressEvent(self, event):
         """Handle mouse press events"""
-        # Ensure widget has focus for keyboard events
         self.setFocus()
 
         if event.button() == Qt.MouseButton.LeftButton:
-            x, y = event.position().x(), event.position().y()
-            ctrl_pressed = event.modifiers() & Qt.KeyboardModifier.ControlModifier
-            shift_pressed = event.modifiers() & Qt.KeyboardModifier.ShiftModifier
-
-            # Check if clicking on ghost page button
-            if self._check_ghost_page_click(x, y):
-                return
-
-            # Update current_page_index based on where user clicked
-            page, page_index, renderer = self._get_page_at(x, y)
-            if page_index >= 0:
-                self.current_page_index = page_index
-
-            if len(self.selected_elements) == 1 and self.selected_element:
-                if self.rotation_mode:
-                    # In rotation mode, start rotation tracking
-                    self._begin_rotate(self.selected_element)
-                    self.drag_start_pos = (x, y)
-                    self.rotation_start_angle = self.selected_element.rotation
-                    self.is_dragging = True
-                    return
-                else:
-                    # In normal mode, check for resize handles
-                    handle = self._get_resize_handle_at(x, y)
-                    if handle:
-                        self._begin_resize(self.selected_element)
-                        self.resize_handle = handle
-                        self.drag_start_pos = (x, y)
-                        self.resize_start_pos = self.selected_element.position
-                        self.resize_start_size = self.selected_element.size
-                        self.is_dragging = True
-                        return
-
-            element = self._get_element_at(x, y)
-            if element:
-                print(
-                    f"DEBUG: Clicked on element: {element}, ctrl_pressed: {ctrl_pressed}, shift_pressed: {shift_pressed}"
-                )
-                # Check if Ctrl is pressed and element is ImageData - enter image pan mode
-                if ctrl_pressed and isinstance(element, ImageData) and not self.rotation_mode:
-                    # Enter image pan mode - pan image within frame
-                    self.selected_elements = {element}
-                    self.drag_start_pos = (x, y)
-                    self.image_pan_mode = True
-                    self.image_pan_start_crop = element.crop_info
-                    self._begin_image_pan(element)
-                    self.is_dragging = True
-                    self.setCursor(Qt.CursorShape.SizeAllCursor)
-                    print(f"Entered image pan mode for {element}")
-                elif ctrl_pressed:
-                    # Multi-select mode (for non-ImageData elements or when Ctrl is pressed)
-                    print(f"DEBUG: Multi-select mode triggered")
-                    if element in self.selected_elements:
-                        print(f"DEBUG: Removing element from selection")
-                        self.selected_elements.remove(element)
-                    else:
-                        print(f"DEBUG: Adding element to selection. Current count: {len(self.selected_elements)}")
-                        self.selected_elements.add(element)
-                    print(f"DEBUG: Total selected elements: {len(self.selected_elements)}")
-                elif shift_pressed:
-                    # Shift can be used for multi-select as well
-                    if element in self.selected_elements:
-                        self.selected_elements.remove(element)
-                    else:
-                        self.selected_elements.add(element)
-                else:
-                    # Normal drag mode
-                    print(f"DEBUG: Normal drag mode - single selection")
-                    self.selected_elements = {element}
-                    self.drag_start_pos = (x, y)
-                    self.drag_start_element_pos = element.position
-                    if not self.rotation_mode:
-                        self._begin_move(element)
-                        self.is_dragging = True
-            else:
-                if not ctrl_pressed:
-                    self.selected_elements.clear()
-
-            self.update()
-
+            self._handle_left_click(event)
         elif event.button() == Qt.MouseButton.MiddleButton:
             self.is_panning = True
             self.drag_start_pos = (event.position().x(), event.position().y())
             self.setCursor(Qt.CursorShape.ClosedHandCursor)
 
+    def _handle_left_click(self, event):
+        """Handle left mouse button click."""
+        x, y = event.position().x(), event.position().y()
+        ctrl_pressed = event.modifiers() & Qt.KeyboardModifier.ControlModifier
+        shift_pressed = event.modifiers() & Qt.KeyboardModifier.ShiftModifier
+
+        # Check if clicking on ghost page button
+        if self._check_ghost_page_click(x, y):
+            return
+
+        # Update current_page_index based on where user clicked
+        page, page_index, renderer = self._get_page_at(x, y)
+        if page_index >= 0:
+            self.current_page_index = page_index
+
+        # Handle interaction with already-selected element
+        if len(self.selected_elements) == 1 and self.selected_element:
+            if self.rotation_mode:
+                self._handle_rotation_start(x, y)
+                return
+            else:
+                handle = self._get_resize_handle_at(x, y)
+                if handle:
+                    self._handle_resize_start(x, y, handle)
+                    return
+
+        # Handle click on element
+        element = self._get_element_at(x, y)
+        if element:
+            if ctrl_pressed and isinstance(element, ImageData) and not self.rotation_mode:
+                self._handle_image_pan_start(x, y, element)
+            elif ctrl_pressed or shift_pressed:
+                self._handle_multi_select(element)
+            else:
+                self._handle_element_drag_start(x, y, element)
+        else:
+            if not ctrl_pressed:
+                self.selected_elements.clear()
+
+        self.update()
+
+    def _handle_canvas_pan(self, x: float, y: float):
+        """Handle canvas panning with middle mouse button."""
+        dx = x - self.drag_start_pos[0]
+        dy = y - self.drag_start_pos[1]
+
+        self.pan_offset[0] += dx
+        self.pan_offset[1] += dy
+
+        if hasattr(self, "clamp_pan_offset"):
+            self.clamp_pan_offset()
+
+        self.drag_start_pos = (x, y)
+        self.update()
+
+        main_window = self.window()
+        if hasattr(main_window, "update_scrollbars"):
+            main_window.update_scrollbars()
+
+    def _handle_rotation_move(self, x: float, y: float):
+        """Handle element rotation during drag."""
+        if not hasattr(self.selected_element, "_page_renderer"):
+            return
+
+        renderer = self.selected_element._page_renderer
+        elem_x, elem_y = self.selected_element.position
+        elem_w, elem_h = self.selected_element.size
+
+        center_page_x = elem_x + elem_w / 2
+        center_page_y = elem_y + elem_h / 2
+        screen_center_x, screen_center_y = renderer.page_to_screen(center_page_x, center_page_y)
+
+        dx = x - screen_center_x
+        dy = y - screen_center_y
+        angle = math.degrees(math.atan2(dy, dx))
+        angle = round(angle / self.rotation_snap_angle) * self.rotation_snap_angle
+        angle = angle % 360
+
+        self.selected_element.rotation = angle
+
+        main_window = self.window()
+        if hasattr(main_window, "show_status"):
+            main_window.show_status(f"Rotation: {angle:.1f}°", 100)
+
+    def _handle_resize_move(self, x: float, y: float):
+        """Handle element resize during drag."""
+        screen_dx = x - self.drag_start_pos[0]
+        screen_dy = y - self.drag_start_pos[1]
+
+        total_dx = screen_dx / self.zoom_level
+        total_dy = screen_dy / self.zoom_level
+
+        self._resize_element(total_dx, total_dy)
+
+    def _handle_element_move(self, x: float, y: float):
+        """Handle element movement during drag, including page transfer."""
+        current_page, current_page_index, current_renderer = self._get_page_at(x, y)
+
+        if current_page and hasattr(self.selected_element, "_parent_page"):
+            source_page = self.selected_element._parent_page
+
+            if current_page is not source_page:
+                self._transfer_element_to_page(
+                    self.selected_element, source_page, current_page, x, y, current_renderer
+                )
+            else:
+                self._move_element_within_page(x, y, source_page)
+        else:
+            # No page context - simple move without snapping
+            total_dx = (x - self.drag_start_pos[0]) / self.zoom_level
+            total_dy = (y - self.drag_start_pos[1]) / self.zoom_level
+
+            new_x = self.drag_start_element_pos[0] + total_dx
+            new_y = self.drag_start_element_pos[1] + total_dy
+
+            self.selected_element.position = (new_x, new_y)
+
+    def _move_element_within_page(self, x: float, y: float, page):
+        """Move element within its current page with snapping."""
+        total_dx = (x - self.drag_start_pos[0]) / self.zoom_level
+        total_dy = (y - self.drag_start_pos[1]) / self.zoom_level
+
+        new_x = self.drag_start_element_pos[0] + total_dx
+        new_y = self.drag_start_element_pos[1] + total_dy
+
+        main_window = self.window()
+        snap_sys = page.layout.snapping_system
+        page_size = page.layout.size
+        dpi = main_window.project.working_dpi
+
+        snapped_pos = snap_sys.snap_position(
+            position=(new_x, new_y),
+            size=self.selected_element.size,
+            page_size=page_size,
+            dpi=dpi,
+            project=main_window.project,
+        )
+
+        self.selected_element.position = snapped_pos
+
     def mouseMoveEvent(self, event):
         """Handle mouse move events"""
         x, y = event.position().x(), event.position().y()
 
-        # Update status bar with page information
         self._update_page_status(x, y)
 
+        # Canvas panning (middle mouse button)
         if self.is_panning and self.drag_start_pos:
-            dx = x - self.drag_start_pos[0]
-            dy = y - self.drag_start_pos[1]
-
-            self.pan_offset[0] += dx
-            self.pan_offset[1] += dy
-
-            # Clamp pan offset to content bounds
-            if hasattr(self, "clamp_pan_offset"):
-                self.clamp_pan_offset()
-
-            self.drag_start_pos = (x, y)
-            self.update()
-
-            # Update scrollbars if available
-            main_window = self.window()
-            if hasattr(main_window, "update_scrollbars"):
-                main_window.update_scrollbars()
-
+            self._handle_canvas_pan(x, y)
             return
 
         if not self.is_dragging or not self.drag_start_pos:
             return
 
-        if self.selected_element:
-            if self.image_pan_mode:
-                # Image pan mode - delegate to ImagePanMixin
-                self._handle_image_pan_move(x, y, self.selected_element)
+        if not self.selected_element:
+            return
 
-            elif self.rotation_mode:
-                # Rotation mode
-                if not hasattr(self.selected_element, "_page_renderer"):
-                    return
+        # Dispatch to appropriate handler based on interaction mode
+        if self.image_pan_mode:
+            self._handle_image_pan_move(x, y, self.selected_element)
+        elif self.rotation_mode:
+            self._handle_rotation_move(x, y)
+        elif self.resize_handle:
+            self._handle_resize_move(x, y)
+        else:
+            self._handle_element_move(x, y)
 
-                renderer = self.selected_element._page_renderer
-                elem_x, elem_y = self.selected_element.position
-                elem_w, elem_h = self.selected_element.size
-
-                center_page_x = elem_x + elem_w / 2
-                center_page_y = elem_y + elem_h / 2
-                screen_center_x, screen_center_y = renderer.page_to_screen(center_page_x, center_page_y)
-
-                dx = x - screen_center_x
-                dy = y - screen_center_y
-                angle = math.degrees(math.atan2(dy, dx))
-
-                angle = round(angle / self.rotation_snap_angle) * self.rotation_snap_angle
-                angle = angle % 360
-
-                self.selected_element.rotation = angle
-
-                main_window = self.window()
-                if hasattr(main_window, "show_status"):
-                    main_window.show_status(f"Rotation: {angle:.1f}°", 100)
-
-            elif self.resize_handle:
-                # Resize mode
-                screen_dx = x - self.drag_start_pos[0]
-                screen_dy = y - self.drag_start_pos[1]
-
-                total_dx = screen_dx / self.zoom_level
-                total_dy = screen_dy / self.zoom_level
-
-                self._resize_element(total_dx, total_dy)
-            else:
-                # Move mode
-                current_page, current_page_index, current_renderer = self._get_page_at(x, y)
-
-                if current_page and hasattr(self.selected_element, "_parent_page"):
-                    source_page = self.selected_element._parent_page
-
-                    if current_page is not source_page:
-                        self._transfer_element_to_page(
-                            self.selected_element, source_page, current_page, x, y, current_renderer
-                        )
-                    else:
-                        total_dx = (x - self.drag_start_pos[0]) / self.zoom_level
-                        total_dy = (y - self.drag_start_pos[1]) / self.zoom_level
-
-                        new_x = self.drag_start_element_pos[0] + total_dx
-                        new_y = self.drag_start_element_pos[1] + total_dy
-
-                        main_window = self.window()
-                        snap_sys = source_page.layout.snapping_system
-                        page_size = source_page.layout.size
-                        dpi = main_window.project.working_dpi
-
-                        snapped_pos = snap_sys.snap_position(
-                            position=(new_x, new_y),
-                            size=self.selected_element.size,
-                            page_size=page_size,
-                            dpi=dpi,
-                            project=main_window.project,
-                        )
-
-                        self.selected_element.position = snapped_pos
-                else:
-                    total_dx = (x - self.drag_start_pos[0]) / self.zoom_level
-                    total_dy = (y - self.drag_start_pos[1]) / self.zoom_level
-
-                    new_x = self.drag_start_element_pos[0] + total_dx
-                    new_y = self.drag_start_element_pos[1] + total_dy
-
-                    self.selected_element.position = (new_x, new_y)
-
-            self.update()
+        self.update()
 
     def mouseReleaseEvent(self, event):
         """Handle mouse release events"""

pyPhotoAlbum/mixins/operations/alignment_ops.py

@@ -14,6 +14,25 @@ class AlignmentOperationsMixin:
         """Get list of selected elements for alignment operations"""
         return list(self.gl_widget.selected_elements) if self.gl_widget.selected_elements else []
 
+    def _execute_alignment(self, alignment_func, status_msg: str):
+        """
+        Execute an alignment operation with common boilerplate.
+
+        Args:
+            alignment_func: AlignmentManager method to call with elements
+            status_msg: Status message format string (will receive element count)
+        """
+        elements = self._get_selected_elements_list()
+        if not self.require_selection(min_count=2):
+            return
+
+        changes = alignment_func(elements)
+        if changes:
+            cmd = AlignElementsCommand(changes)
+            self.project.history.execute(cmd)
+            self.update_view()
+            self.show_status(status_msg.format(len(elements)), 2000)
+
     @ribbon_action(
         label="Align Left",
         tooltip="Align selected elements to the left",
@@ -24,16 +43,7 @@ class AlignmentOperationsMixin:
     )
     def align_left(self):
         """Align selected elements to the left"""
-        elements = self._get_selected_elements_list()
-        if not self.require_selection(min_count=2):
-            return
-
-        changes = AlignmentManager.align_left(elements)
-        if changes:
-            cmd = AlignElementsCommand(changes)
-            self.project.history.execute(cmd)
-            self.update_view()
-            self.show_status(f"Aligned {len(elements)} elements to left", 2000)
+        self._execute_alignment(AlignmentManager.align_left, "Aligned {} elements to left")
 
     @ribbon_action(
         label="Align Right",
@@ -45,16 +55,7 @@ class AlignmentOperationsMixin:
     )
     def align_right(self):
         """Align selected elements to the right"""
-        elements = self._get_selected_elements_list()
-        if not self.require_selection(min_count=2):
-            return
-
-        changes = AlignmentManager.align_right(elements)
-        if changes:
-            cmd = AlignElementsCommand(changes)
-            self.project.history.execute(cmd)
-            self.update_view()
-            self.show_status(f"Aligned {len(elements)} elements to right", 2000)
+        self._execute_alignment(AlignmentManager.align_right, "Aligned {} elements to right")
 
     @ribbon_action(
         label="Align Top",
@@ -66,16 +67,7 @@ class AlignmentOperationsMixin:
     )
     def align_top(self):
         """Align selected elements to the top"""
-        elements = self._get_selected_elements_list()
-        if not self.require_selection(min_count=2):
-            return
-
-        changes = AlignmentManager.align_top(elements)
-        if changes:
-            cmd = AlignElementsCommand(changes)
-            self.project.history.execute(cmd)
-            self.update_view()
-            self.show_status(f"Aligned {len(elements)} elements to top", 2000)
+        self._execute_alignment(AlignmentManager.align_top, "Aligned {} elements to top")
 
     @ribbon_action(
         label="Align Bottom",
@@ -87,16 +79,7 @@ class AlignmentOperationsMixin:
     )
     def align_bottom(self):
         """Align selected elements to the bottom"""
-        elements = self._get_selected_elements_list()
-        if not self.require_selection(min_count=2):
-            return
-
-        changes = AlignmentManager.align_bottom(elements)
-        if changes:
-            cmd = AlignElementsCommand(changes)
-            self.project.history.execute(cmd)
-            self.update_view()
-            self.show_status(f"Aligned {len(elements)} elements to bottom", 2000)
+        self._execute_alignment(AlignmentManager.align_bottom, "Aligned {} elements to bottom")
 
     @ribbon_action(
         label="Align H-Center",
@@ -108,16 +91,7 @@ class AlignmentOperationsMixin:
     )
     def align_horizontal_center(self):
         """Align selected elements to horizontal center"""
-        elements = self._get_selected_elements_list()
-        if not self.require_selection(min_count=2):
-            return
-
-        changes = AlignmentManager.align_horizontal_center(elements)
-        if changes:
-            cmd = AlignElementsCommand(changes)
-            self.project.history.execute(cmd)
-            self.update_view()
-            self.show_status(f"Aligned {len(elements)} elements to horizontal center", 2000)
+        self._execute_alignment(AlignmentManager.align_horizontal_center, "Aligned {} elements to horizontal center")
 
     @ribbon_action(
         label="Align V-Center",
@@ -129,16 +103,7 @@ class AlignmentOperationsMixin:
     )
     def align_vertical_center(self):
         """Align selected elements to vertical center"""
-        elements = self._get_selected_elements_list()
-        if not self.require_selection(min_count=2):
-            return
-
-        changes = AlignmentManager.align_vertical_center(elements)
-        if changes:
-            cmd = AlignElementsCommand(changes)
-            self.project.history.execute(cmd)
-            self.update_view()
-            self.show_status(f"Aligned {len(elements)} elements to vertical center", 2000)
+        self._execute_alignment(AlignmentManager.align_vertical_center, "Aligned {} elements to vertical center")
 
     @ribbon_action(
         label="Maximize Pattern",

pyPhotoAlbum/mixins/operations/distribution_ops.py

@@ -14,6 +14,25 @@ class DistributionOperationsMixin:
         """Get list of selected elements for distribution operations"""
         return list(self.gl_widget.selected_elements) if self.gl_widget.selected_elements else []
 
+    def _execute_distribution(self, distribution_func, status_msg: str):
+        """
+        Execute a distribution operation with common boilerplate.
+
+        Args:
+            distribution_func: AlignmentManager method to call with elements
+            status_msg: Status message format string (will receive element count)
+        """
+        elements = self._get_selected_elements_list()
+        if not self.require_selection(min_count=3):
+            return
+
+        changes = distribution_func(elements)
+        if changes:
+            cmd = AlignElementsCommand(changes)
+            self.project.history.execute(cmd)
+            self.update_view()
+            self.show_status(status_msg.format(len(elements)), 2000)
+
     @ribbon_action(
         label="Distribute H",
         tooltip="Distribute selected elements evenly horizontally",
@@ -24,16 +43,7 @@ class DistributionOperationsMixin:
     )
     def distribute_horizontally(self):
         """Distribute selected elements evenly horizontally"""
-        elements = self._get_selected_elements_list()
-        if not self.require_selection(min_count=3):
-            return
-
-        changes = AlignmentManager.distribute_horizontally(elements)
-        if changes:
-            cmd = AlignElementsCommand(changes)
-            self.project.history.execute(cmd)
-            self.update_view()
-            self.show_status(f"Distributed {len(elements)} elements horizontally", 2000)
+        self._execute_distribution(AlignmentManager.distribute_horizontally, "Distributed {} elements horizontally")
 
     @ribbon_action(
         label="Distribute V",
@@ -45,16 +55,7 @@ class DistributionOperationsMixin:
     )
     def distribute_vertically(self):
         """Distribute selected elements evenly vertically"""
-        elements = self._get_selected_elements_list()
-        if not self.require_selection(min_count=3):
-            return
-
-        changes = AlignmentManager.distribute_vertically(elements)
-        if changes:
-            cmd = AlignElementsCommand(changes)
-            self.project.history.execute(cmd)
-            self.update_view()
-            self.show_status(f"Distributed {len(elements)} elements vertically", 2000)
+        self._execute_distribution(AlignmentManager.distribute_vertically, "Distributed {} elements vertically")
 
     @ribbon_action(
         label="Space H",
@@ -66,16 +67,7 @@ class DistributionOperationsMixin:
     )
     def space_horizontally(self):
         """Space selected elements equally horizontally"""
-        elements = self._get_selected_elements_list()
-        if not self.require_selection(min_count=3):
-            return
-
-        changes = AlignmentManager.space_horizontally(elements)
-        if changes:
-            cmd = AlignElementsCommand(changes)
-            self.project.history.execute(cmd)
-            self.update_view()
-            self.show_status(f"Spaced {len(elements)} elements horizontally", 2000)
+        self._execute_distribution(AlignmentManager.space_horizontally, "Spaced {} elements horizontally")
 
     @ribbon_action(
         label="Space V",
@@ -87,13 +79,4 @@ class DistributionOperationsMixin:
     )
     def space_vertically(self):
         """Space selected elements equally vertically"""
-        elements = self._get_selected_elements_list()
-        if not self.require_selection(min_count=3):
-            return
-
-        changes = AlignmentManager.space_vertically(elements)
-        if changes:
-            cmd = AlignElementsCommand(changes)
-            self.project.history.execute(cmd)
-            self.update_view()
-            self.show_status(f"Spaced {len(elements)} elements vertically", 2000)
+        self._execute_distribution(AlignmentManager.space_vertically, "Spaced {} elements vertically")

pyPhotoAlbum/mixins/operations/size_ops.py

@@ -14,6 +14,50 @@ class SizeOperationsMixin:
         """Get list of selected elements for size operations"""
         return list(self.gl_widget.selected_elements) if self.gl_widget.selected_elements else []
 
+    def _execute_resize(self, resize_func, status_msg: str):
+        """
+        Execute a resize operation on multiple elements.
+
+        Args:
+            resize_func: AlignmentManager method to call with elements
+            status_msg: Status message format string (will receive element count)
+        """
+        elements = self._get_selected_elements_list()
+        if not self.require_selection(min_count=2):
+            return
+
+        changes = resize_func(elements)
+        if changes:
+            cmd = ResizeElementsCommand(changes)
+            self.project.history.execute(cmd)
+            self.update_view()
+            self.show_status(status_msg.format(len(elements)), 2000)
+
+    def _execute_fit_to_page(self, fit_func, status_msg: str):
+        """
+        Execute a fit-to-page operation on a single element.
+
+        Args:
+            fit_func: Function that takes (element, page) and returns a change tuple
+            status_msg: Status message to display on success
+        """
+        if not self.require_selection(min_count=1):
+            return
+
+        page = self.get_current_page()
+        if not page:
+            self.show_warning("No Page", "Please create a page first.")
+            return
+
+        element = next(iter(self.gl_widget.selected_elements))
+        change = fit_func(element, page)
+
+        if change:
+            cmd = ResizeElementsCommand([change])
+            self.project.history.execute(cmd)
+            self.update_view()
+            self.show_status(status_msg, 2000)
+
     @ribbon_action(
         label="Same Size",
         tooltip="Make all selected elements the same size",
@@ -24,16 +68,7 @@ class SizeOperationsMixin:
     )
     def make_same_size(self):
         """Make all selected elements the same size"""
-        elements = self._get_selected_elements_list()
-        if not self.require_selection(min_count=2):
-            return
-
-        changes = AlignmentManager.make_same_size(elements)
-        if changes:
-            cmd = ResizeElementsCommand(changes)
-            self.project.history.execute(cmd)
-            self.update_view()
-            self.show_status(f"Resized {len(elements)} elements to same size", 2000)
+        self._execute_resize(AlignmentManager.make_same_size, "Resized {} elements to same size")
 
     @ribbon_action(
         label="Same Width",
@@ -45,16 +80,7 @@ class SizeOperationsMixin:
     )
     def make_same_width(self):
         """Make all selected elements the same width"""
-        elements = self._get_selected_elements_list()
-        if not self.require_selection(min_count=2):
-            return
-
-        changes = AlignmentManager.make_same_width(elements)
-        if changes:
-            cmd = ResizeElementsCommand(changes)
-            self.project.history.execute(cmd)
-            self.update_view()
-            self.show_status(f"Resized {len(elements)} elements to same width", 2000)
+        self._execute_resize(AlignmentManager.make_same_width, "Resized {} elements to same width")
 
     @ribbon_action(
         label="Same Height",
@@ -66,16 +92,7 @@ class SizeOperationsMixin:
     )
     def make_same_height(self):
         """Make all selected elements the same height"""
-        elements = self._get_selected_elements_list()
-        if not self.require_selection(min_count=2):
-            return
-
-        changes = AlignmentManager.make_same_height(elements)
-        if changes:
-            cmd = ResizeElementsCommand(changes)
-            self.project.history.execute(cmd)
-            self.update_view()
-            self.show_status(f"Resized {len(elements)} elements to same height", 2000)
+        self._execute_resize(AlignmentManager.make_same_height, "Resized {} elements to same height")
 
     @ribbon_action(
         label="Fit Width",
@@ -87,26 +104,10 @@ class SizeOperationsMixin:
     )
     def fit_to_width(self):
         """Fit selected element to page width"""
-        if not self.require_selection(min_count=1):
-            return
-
-        page = self.get_current_page()
-        if not page:
-            self.show_warning("No Page", "Please create a page first.")
-            return
-
-        # Get the first selected element
-        element = next(iter(self.gl_widget.selected_elements))
-
-        # Fit to page width
-        page_width = page.layout.size[0]
-        change = AlignmentManager.fit_to_page_width(element, page_width)
-
-        if change:
-            cmd = ResizeElementsCommand([change])
-            self.project.history.execute(cmd)
-            self.update_view()
-            self.show_status("Fitted element to page width", 2000)
+        self._execute_fit_to_page(
+            lambda elem, page: AlignmentManager.fit_to_page_width(elem, page.layout.size[0]),
+            "Fitted element to page width",
+        )
 
     @ribbon_action(
         label="Fit Height",
@@ -118,26 +119,10 @@ class SizeOperationsMixin:
     )
     def fit_to_height(self):
         """Fit selected element to page height"""
-        if not self.require_selection(min_count=1):
-            return
-
-        page = self.get_current_page()
-        if not page:
-            self.show_warning("No Page", "Please create a page first.")
-            return
-
-        # Get the first selected element
-        element = next(iter(self.gl_widget.selected_elements))
-
-        # Fit to page height
-        page_height = page.layout.size[1]
-        change = AlignmentManager.fit_to_page_height(element, page_height)
-
-        if change:
-            cmd = ResizeElementsCommand([change])
-            self.project.history.execute(cmd)
-            self.update_view()
-            self.show_status("Fitted element to page height", 2000)
+        self._execute_fit_to_page(
+            lambda elem, page: AlignmentManager.fit_to_page_height(elem, page.layout.size[1]),
+            "Fitted element to page height",
+        )
 
     @ribbon_action(
         label="Fit to Page",
@@ -149,26 +134,10 @@ class SizeOperationsMixin:
     )
     def fit_to_page(self):
         """Fit selected element to page dimensions"""
-        if not self.require_selection(min_count=1):
-            return
-
-        page = self.get_current_page()
-        if not page:
-            self.show_warning("No Page", "Please create a page first.")
-            return
-
-        # Get the first selected element
-        element = next(iter(self.gl_widget.selected_elements))
-
-        # Fit to page
-        page_width, page_height = page.layout.size
-        change = AlignmentManager.fit_to_page(element, page_width, page_height)
-
-        if change:
-            cmd = ResizeElementsCommand([change])
-            self.project.history.execute(cmd)
-            self.update_view()
-            self.show_status("Fitted element to page", 2000)
+        self._execute_fit_to_page(
+            lambda elem, page: AlignmentManager.fit_to_page(elem, page.layout.size[0], page.layout.size[1]),
+            "Fitted element to page",
+        )
 
     @ribbon_action(
         label="Expand Image",

pyPhotoAlbum/mixins/viewport.py

@@ -175,6 +175,84 @@ class ViewportMixin:
             "height": total_height,
         }
 
+    def _clamp_vertical_pan(self, viewport_height: float) -> float:
+        """Clamp vertical pan offset and return the original value before clamping."""
+        bounds = self.get_content_bounds()
+        content_height = bounds["height"]
+
+        # Save original for page selection (prevents clamping from changing which page we target)
+        original_pan_y = self.pan_offset[1]
+
+        if content_height > viewport_height:
+            max_pan_up = 0  # Can't pan beyond top edge
+            min_pan_up = -(content_height - viewport_height)  # Can't pan beyond bottom edge
+            self.pan_offset[1] = max(min_pan_up, min(max_pan_up, self.pan_offset[1]))
+
+        return original_pan_y
+
+    def _build_page_centerlines(self, pages, dpi: float) -> list:
+        """Build list of (center_y, center_x, width) tuples for each page."""
+        PAGE_MARGIN = 50
+        PAGE_SPACING = 50
+
+        centerlines = []
+        current_y = PAGE_MARGIN
+
+        for page in pages:
+            page_width_mm, page_height_mm = page.layout.size
+            screen_page_width = page_width_mm * dpi / 25.4 * self.zoom_level
+            screen_page_height = page_height_mm * dpi / 25.4 * self.zoom_level
+
+            page_center_y = current_y + screen_page_height / 2
+            page_center_x = PAGE_MARGIN + screen_page_width / 2
+
+            centerlines.append((page_center_y, page_center_x, screen_page_width))
+            current_y += screen_page_height + PAGE_SPACING
+
+        return centerlines
+
+    def _interpolate_target_centerline(self, centerlines: list, viewport_center_y: float) -> tuple:
+        """Find target centerline by interpolating between pages based on viewport position."""
+        if not centerlines:
+            return 0, 0
+
+        # Find the page index we're at or past
+        page_idx = self._find_page_at_viewport_y(centerlines, viewport_center_y)
+
+        if page_idx == 0:
+            return centerlines[0][1], centerlines[0][2]
+
+        # Interpolate between previous and current page
+        prev_y, prev_x, prev_w = centerlines[page_idx - 1]
+        curr_y, curr_x, curr_w = centerlines[page_idx]
+
+        if curr_y == prev_y:
+            return curr_x, curr_w
+
+        t = max(0, min(1, (viewport_center_y - prev_y) / (curr_y - prev_y)))
+        return prev_x + t * (curr_x - prev_x), prev_w + t * (curr_w - prev_w)
+
+    def _find_page_at_viewport_y(self, centerlines: list, viewport_center_y: float) -> int:
+        """Find index of page at or after viewport Y position."""
+        for i, (page_y, _, _) in enumerate(centerlines):
+            if viewport_center_y <= page_y:
+                return i
+        return len(centerlines) - 1  # Below all pages - use last
+
+    def _clamp_horizontal_pan(self, viewport_width: float, target_centerline_x: float, target_page_width: float):
+        """Clamp horizontal pan to keep viewport centered on target page."""
+        ideal_pan_x = viewport_width / 2 - target_centerline_x
+
+        if target_page_width > viewport_width:
+            max_deviation = (target_page_width / 2) + (viewport_width / 4)
+        else:
+            max_deviation = 100  # Small margin to avoid jitter
+
+        min_pan_x = ideal_pan_x - max_deviation
+        max_pan_x = ideal_pan_x + max_deviation
+
+        self.pan_offset[0] = max(min_pan_x, min(max_pan_x, self.pan_offset[0]))
+
     def clamp_pan_offset(self):
         """
         Clamp pan offset to prevent scrolling beyond content bounds.
@@ -194,118 +272,18 @@ class ViewportMixin:
         viewport_width = self.width()
         viewport_height = self.height()
 
+        # Vertical clamping (returns original pan_y for page selection)
+        original_pan_y = self._clamp_vertical_pan(viewport_height)
+
+        # Build page centerline data
         dpi = main_window.project.working_dpi
-        PAGE_MARGIN = 50
-        PAGE_SPACING = 50
-
-        # Vertical clamping
-        bounds = self.get_content_bounds()
-        content_height = bounds["height"]
-
-        # Save original pan_offset[1] BEFORE clamping for page selection
-        # This prevents clamping from changing which page we think we're on
-        original_pan_y = self.pan_offset[1]
-
-        if content_height > viewport_height:
-            # Content is taller than viewport - restrict panning
-            max_pan_up = 0  # Can't pan beyond top edge
-            min_pan_up = -(content_height - viewport_height)  # Can't pan beyond bottom edge
-            self.pan_offset[1] = max(min_pan_up, min(max_pan_up, self.pan_offset[1]))
-        # Don't force centering when content fits - preserve scroll position
-        # This prevents jumping when zooming in/out across the content_height == viewport_height boundary
-
-        # Horizontal clamping - centerline-based approach
-        # Calculate the centerline position for each page and interpolate
-
-        # Build list of page centerlines and their Y positions
-        page_centerlines = []
-        current_y = PAGE_MARGIN
-
-        for page in main_window.project.pages:
-            page_width_mm, page_height_mm = page.layout.size
-            page_width_px = page_width_mm * dpi / 25.4
-            page_height_px = page_height_mm * dpi / 25.4
-
-            screen_page_width = page_width_px * self.zoom_level
-            screen_page_height = page_height_px * self.zoom_level
-
-            # Calculate page center Y position (in world coordinates)
-            page_center_y = current_y + screen_page_height / 2
-
-            # Calculate the centerline X position (center of the page)
-            # Pages are left-aligned at PAGE_MARGIN, so center is at PAGE_MARGIN + width/2
-            page_center_x = PAGE_MARGIN + screen_page_width / 2
-
-            page_centerlines.append((page_center_y, page_center_x, screen_page_width))
-
-            current_y += screen_page_height + PAGE_SPACING
-
-        if not page_centerlines:
+        centerlines = self._build_page_centerlines(main_window.project.pages, dpi)
+        if not centerlines:
             return
 
-        # Determine current viewport center Y in world coordinates using ORIGINAL pan_y
-        # This prevents vertical clamping from changing which page we're targeting
-        # viewport_center_y in screen coords = viewport_height / 2
-        # Convert to world coords: world_y = (screen_y - pan_offset[1]) / zoom_level
-        # But we want screen position, so we use pan_offset directly
-        viewport_center_y_world = -original_pan_y + viewport_height / 2
+        # Find target centerline by interpolating based on viewport position
+        viewport_center_y = -original_pan_y + viewport_height / 2
+        target_x, target_width = self._interpolate_target_centerline(centerlines, viewport_center_y)
 
-        # Find which pages we're between and interpolate
-        target_centerline_x = page_centerlines[0][1]  # Default to first page
-        target_page_width = page_centerlines[0][2]
-        selected_page_index = 0
-
-        for i in range(len(page_centerlines)):
-            page_y, page_x, page_w = page_centerlines[i]
-
-            if viewport_center_y_world <= page_y:
-                # We're above or at this page's center
-                if i == 0:
-                    # First page
-                    target_centerline_x = page_x
-                    target_page_width = page_w
-                    selected_page_index = 0
-                else:
-                    # Interpolate between previous and current page
-                    prev_y, prev_x, prev_w = page_centerlines[i - 1]
-
-                    # Linear interpolation factor
-                    if page_y != prev_y:
-                        t = (viewport_center_y_world - prev_y) / (page_y - prev_y)
-                        t = max(0, min(1, t))  # Clamp to [0, 1]
-
-                        target_centerline_x = prev_x + t * (page_x - prev_x)
-                        target_page_width = prev_w + t * (page_w - prev_w)
-                        selected_page_index = i - 1 if t < 0.5 else i
-                    else:
-                        target_centerline_x = page_x
-                        target_page_width = page_w
-                        selected_page_index = i
-                break
-        else:
-            # We're below all pages - use last page
-            target_centerline_x = page_centerlines[-1][1]
-            target_page_width = page_centerlines[-1][2]
-            selected_page_index = len(page_centerlines) - 1
-
-        # Horizontal clamping to keep viewport reasonably centered on the page
-        # The centerline should ideally be at viewport_width / 2
-        ideal_pan_x = viewport_width / 2 - target_centerline_x
-
-        # Calculate how far we need to allow panning to see the full width of the page
-        # If page is wider than viewport, allow panning to see left and right edges
-        # If page is narrower, keep it centered
-        if target_page_width > viewport_width:
-            # Page wider than viewport - allow panning to see edges plus some margin
-            # Allow user to pan to see any part of the page, with reasonable overshoot
-            max_deviation = (target_page_width / 2) + (viewport_width / 4)
-        else:
-            # Page narrower than viewport - keep centered with small margin for stability
-            max_deviation = 100  # Small margin to avoid jitter
-
-        # Calculate bounds
-        min_pan_x = ideal_pan_x - max_deviation
-        max_pan_x = ideal_pan_x + max_deviation
-
-        old_pan_x = self.pan_offset[0]
-        self.pan_offset[0] = max(min_pan_x, min(max_pan_x, self.pan_offset[0]))
+        # Horizontal clamping
+        self._clamp_horizontal_pan(viewport_width, target_x, target_width)

pyPhotoAlbum/snapping.py

@@ -370,74 +370,6 @@ class SnappingSystem:
 
         return best_snap
 
-    def _snap_axis(
-        self, position: float, size: float, page_size_mm: float, dpi: int, snap_threshold_px: float, orientation: str
-    ) -> float:
-        """
-        Snap along a single axis
-
-        Args:
-            position: Current position along axis in pixels
-            size: Element size along axis in pixels
-            page_size_mm: Page size along axis in mm
-            dpi: DPI for conversion
-            snap_threshold_px: Snap threshold in pixels
-            orientation: 'vertical' for x-axis, 'horizontal' for y-axis
-
-        Returns:
-            Snapped position in pixels
-        """
-        snap_candidates: List[Tuple[float, float]] = []
-
-        # 1. Page edge snapping
-        if self.snap_to_edges:
-            # Snap to start edge (0)
-            snap_candidates.append((0.0, abs(position - 0)))
-
-            # Snap to end edge
-            page_size_px = page_size_mm * dpi / 25.4
-            snap_candidates.append((page_size_px - size, abs(position - (page_size_px - size))))
-
-            # Also snap element's far edge to page edge
-            snap_candidates.append((page_size_px - size, abs((position + size) - page_size_px)))
-
-        # 2. Grid snapping
-        if self.snap_to_grid:
-            grid_size_px = self.grid_size_mm * dpi / 25.4
-
-            # Snap to nearest grid line
-            nearest_grid = round(position / grid_size_px) * grid_size_px
-            snap_candidates.append((nearest_grid, abs(position - nearest_grid)))
-
-            # Also try snapping element's far edge to grid
-            element_end = position + size
-            nearest_grid_end = round(element_end / grid_size_px) * grid_size_px
-            snap_candidates.append((nearest_grid_end - size, abs(element_end - nearest_grid_end)))
-
-        # 3. Guide snapping
-        if self.snap_to_guides:
-            for guide in self.guides:
-                if guide.orientation == orientation:
-                    guide_pos_px = guide.position * dpi / 25.4
-
-                    # Snap start edge to guide
-                    snap_candidates.append((guide_pos_px, abs(position - guide_pos_px)))
-
-                    # Snap end edge to guide
-                    element_end = position + size
-                    snap_candidates.append((guide_pos_px - size, abs(element_end - guide_pos_px)))
-
-        # Find the best snap candidate within threshold
-        best_snap = None
-        best_distance = snap_threshold_px
-
-        for snap_pos, distance in snap_candidates:
-            if distance < best_distance:
-                best_snap = snap_pos
-                best_distance = distance
-
-        return best_snap if best_snap is not None else position
-
     def get_snap_lines(self, page_size: Tuple[float, float], dpi: int = 300) -> dict:
         """
         Get all snap lines for visualization

tests/test_snapping_system.py

@@ -462,51 +462,6 @@ class TestSnapEdgeToTargets:
         assert result is None
 
 
-class TestSnapAxis:
-    """Tests for _snap_axis method"""
-
-    def test_snap_axis_to_start(self):
-        """Test snapping axis to start edge"""
-        snap = SnappingSystem()
-        snap.snap_to_edges = True
-        snap.snap_to_grid = False
-        snap.snap_to_guides = False
-
-        dpi = 300
-        threshold_px = 50
-
-        result = snap._snap_axis(
-            position=10, size=50, page_size_mm=210, dpi=dpi, snap_threshold_px=threshold_px, orientation="vertical"
-        )
-
-        assert result == 0
-
-    def test_snap_axis_to_end(self):
-        """Test snapping axis so element end aligns with page end"""
-        snap = SnappingSystem()
-        snap.snap_to_edges = True
-        snap.snap_to_grid = False
-        snap.snap_to_guides = False
-
-        dpi = 300
-        page_size_mm = 210
-        page_size_px = page_size_mm * dpi / 25.4
-        element_size = 50
-        threshold_px = 50
-
-        result = snap._snap_axis(
-            position=page_size_px - element_size - 10,
-            size=element_size,
-            page_size_mm=page_size_mm,
-            dpi=dpi,
-            snap_threshold_px=threshold_px,
-            orientation="vertical",
-        )
-
-        expected = page_size_px - element_size
-        assert abs(result - expected) < 1
-
-
 class TestGetSnapLines:
     """Tests for get_snap_lines method"""