From ca2b3545ee324a8a19e75c5630cb26c1b2c54b0d Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sat, 22 Nov 2025 08:22:01 +0100 Subject: [PATCH] update page selection --- pyPhotoAlbum/mixins/base.py | 49 ++++++++++++-- pyPhotoAlbum/mixins/operations/page_ops.py | 75 ++++++++-------------- tests/test_base_mixin.py | 8 +++ tests/test_page_ops_mixin.py | 24 +++---- 4 files changed, 89 insertions(+), 67 deletions(-) diff --git a/pyPhotoAlbum/mixins/base.py b/pyPhotoAlbum/mixins/base.py index 1c341ce..98e2a86 100644 --- a/pyPhotoAlbum/mixins/base.py +++ b/pyPhotoAlbum/mixins/base.py @@ -54,21 +54,58 @@ class ApplicationStateMixin: return self._template_manager # Common helper methods - + + def _get_most_visible_page_index(self): + """ + Determine which page is most visible in the current viewport. + + Returns: + int: Index of the most visible page + """ + if not hasattr(self.gl_widget, '_page_renderers') or not self.gl_widget._page_renderers: + return self.gl_widget.current_page_index + + # Get viewport dimensions + viewport_height = self.gl_widget.height() + viewport_center_y = viewport_height / 2 + + # Find which page's center is closest to viewport center + min_distance = float('inf') + best_page_index = self.gl_widget.current_page_index + + for renderer, page in self.gl_widget._page_renderers: + # Get page center Y position in screen coordinates + page_height_mm = page.layout.size[1] + page_height_px = page_height_mm * self.project.working_dpi / 25.4 + page_center_y_offset = renderer.screen_y + (page_height_px * self.gl_widget.zoom_level / 2) + + # Calculate distance from viewport center + distance = abs(page_center_y_offset - viewport_center_y) + + if distance < min_distance: + min_distance = distance + # Find the page index in project.pages + try: + best_page_index = self.project.pages.index(page) + except ValueError: + pass + + return best_page_index + def get_current_page(self): """ - Get currently selected page. - + Get currently visible page (most visible in viewport). + Returns: Page instance or None if no page is selected """ if not self.project or not self.project.pages: return None - - index = self.gl_widget.current_page_index + + index = self._get_most_visible_page_index() if 0 <= index < len(self.project.pages): return self.project.pages[index] - + return None def get_current_page_index(self) -> int: diff --git a/pyPhotoAlbum/mixins/operations/page_ops.py b/pyPhotoAlbum/mixins/operations/page_ops.py index aad5c16..ec468b1 100644 --- a/pyPhotoAlbum/mixins/operations/page_ops.py +++ b/pyPhotoAlbum/mixins/operations/page_ops.py @@ -238,9 +238,12 @@ class PageOperationsMixin: # Connect page selection change page_combo.currentIndexChanged.connect(on_page_changed) - - # Initialize with first page - on_page_changed(0) + + # Initialize with most visible page + initial_page_index = self._get_most_visible_page_index() + if 0 <= initial_page_index < len(self.project.pages): + page_combo.setCurrentIndex(initial_page_index) + on_page_changed(initial_page_index if 0 <= initial_page_index < len(self.project.pages) else 0) # Buttons button_layout = QHBoxLayout() @@ -335,43 +338,6 @@ class PageOperationsMixin: if set_default_checkbox.isChecked(): status_msg += " (set as default)" self.show_status(status_msg, 2000) - - def _get_most_visible_page_index(self): - """ - Determine which page is most visible in the current viewport. - - Returns: - int: Index of the most visible page - """ - if not hasattr(self.gl_widget, '_page_renderers') or not self.gl_widget._page_renderers: - return self.gl_widget.current_page_index - - # Get viewport dimensions - viewport_height = self.gl_widget.height() - viewport_center_y = viewport_height / 2 - - # Find which page's center is closest to viewport center - min_distance = float('inf') - best_page_index = self.gl_widget.current_page_index - - for renderer, page in self.gl_widget._page_renderers: - # Get page center Y position in screen coordinates - page_height_mm = page.layout.size[1] - page_height_px = page_height_mm * self.project.working_dpi / 25.4 - page_center_y_offset = renderer.screen_y + (page_height_px * self.gl_widget.zoom_level / 2) - - # Calculate distance from viewport center - distance = abs(page_center_y_offset - viewport_center_y) - - if distance < min_distance: - min_distance = distance - # Find the page index in project.pages - try: - best_page_index = self.project.pages.index(page) - except ValueError: - pass - - return best_page_index @ribbon_action( label="Toggle Spread", @@ -427,26 +393,35 @@ class PageOperationsMixin: @ribbon_action( label="Remove Page", - tooltip="Remove the last page", + tooltip="Remove the currently selected page", tab="Layout", group="Page" ) def remove_page(self): - """Remove the last page""" + """Remove the currently selected page""" if len(self.project.pages) <= 1: self.show_warning("Cannot Remove", "Must have at least one page") print("Cannot remove page - must have at least one page") return - - # Remove last page - last_page = self.project.pages[-1] - self.project.remove_page(last_page) - + + # Get the most visible page in viewport + page_index = self._get_most_visible_page_index() + + # Ensure index is valid + if page_index < 0 or page_index >= len(self.project.pages): + page_index = len(self.project.pages) - 1 + + page_to_remove = self.project.pages[page_index] + page_name = self.project.get_page_display_name(page_to_remove) + + # Remove the selected page + self.project.remove_page(page_to_remove) + # Renumber remaining pages for i, page in enumerate(self.project.pages): page.page_number = i + 1 - + # Update display self.update_view() - - print(f"Removed page, now have {len(self.project.pages)} pages") + + print(f"Removed {page_name}, now have {len(self.project.pages)} pages") diff --git a/tests/test_base_mixin.py b/tests/test_base_mixin.py index 669c9de..aea24dd 100644 --- a/tests/test_base_mixin.py +++ b/tests/test_base_mixin.py @@ -103,12 +103,14 @@ class TestGetCurrentPage: # Setup project with pages project = Mock(spec=Project) + project.working_dpi = 96 page1 = Mock(spec=Page) page2 = Mock(spec=Page) project.pages = [page1, page2] gl_widget = Mock() gl_widget.current_page_index = 0 + gl_widget._page_renderers = [] # No renderers, so it will use current_page_index window._project = project window._gl_widget = gl_widget @@ -120,12 +122,14 @@ class TestGetCurrentPage: qtbot.addWidget(window) project = Mock(spec=Project) + project.working_dpi = 96 page1 = Mock(spec=Page) page2 = Mock(spec=Page) project.pages = [page1, page2] gl_widget = Mock() gl_widget.current_page_index = 1 + gl_widget._page_renderers = [] # No renderers, so it will use current_page_index window._project = project window._gl_widget = gl_widget @@ -158,10 +162,12 @@ class TestGetCurrentPage: qtbot.addWidget(window) project = Mock(spec=Project) + project.working_dpi = 96 project.pages = [Mock(spec=Page)] gl_widget = Mock() gl_widget.current_page_index = 5 # Out of range + gl_widget._page_renderers = [] # No renderers, so it will use current_page_index window._project = project window._gl_widget = gl_widget @@ -280,10 +286,12 @@ class TestRequirePage: qtbot.addWidget(window) project = Mock(spec=Project) + project.working_dpi = 96 project.pages = [Mock(spec=Page)] gl_widget = Mock() gl_widget.current_page_index = 0 + gl_widget._page_renderers = [] # No renderers, so it will use current_page_index window._project = project window._gl_widget = gl_widget diff --git a/tests/test_page_ops_mixin.py b/tests/test_page_ops_mixin.py index c7c92b2..07dca43 100644 --- a/tests/test_page_ops_mixin.py +++ b/tests/test_page_ops_mixin.py @@ -5,28 +5,30 @@ Tests for PageOperationsMixin import pytest from unittest.mock import Mock, MagicMock, patch from PyQt6.QtWidgets import QMainWindow +from pyPhotoAlbum.mixins.base import ApplicationStateMixin from pyPhotoAlbum.mixins.operations.page_ops import PageOperationsMixin from pyPhotoAlbum.project import Project, Page from pyPhotoAlbum.page_layout import PageLayout -class TestPageOpsWindow(PageOperationsMixin, QMainWindow): +class TestPageOpsWindow(PageOperationsMixin, ApplicationStateMixin, QMainWindow): """Test window with page operations mixin""" def __init__(self): super().__init__() - self.gl_widget = Mock() - self.gl_widget.current_page_index = 0 - self.gl_widget.zoom_level = 1.0 - self.gl_widget.pan_offset = [0, 0] - self.gl_widget._page_renderers = [] - self.gl_widget.width = Mock(return_value=800) - self.gl_widget.height = Mock(return_value=600) - self.project = Project(name="Test") - self.project.working_dpi = 96 - self.project.page_size_mm = (210, 297) + self._gl_widget = Mock() + self._gl_widget.current_page_index = 0 + self._gl_widget.zoom_level = 1.0 + self._gl_widget.pan_offset = [0, 0] + self._gl_widget._page_renderers = [] + self._gl_widget.width = Mock(return_value=800) + self._gl_widget.height = Mock(return_value=600) + self._project = Project(name="Test") + self._project.working_dpi = 96 + self._project.page_size_mm = (210, 297) self._update_view_called = False self._status_message = None + self._status_bar = Mock() def update_view(self): self._update_view_called = True