From 535cd4fea77c6248a5a2da04d5620ed1f412229b Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Mon, 12 Oct 2026 08:32:01 +0200 Subject: [PATCH 1/2] feat(toolbar): Tools menu arms any canvas tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every canvas tool has its own toolbar row, but it armed only from its sidebar card or a key. Now: - a Tools menu on the row lists them under their panel section (Geometry, Metering, Filtration, Dodge & Burn, Retouch), each with its sidebar icon and key; an item runs the tool's shortcut action, so its tab opens and a gated tool grays out - the armed tool's name on its row opens the same menu, so Heal goes to Clone with no Done between them - the ⋯ menu carries a Tools submenu - a stored row gets Tools once, before Undo; a removed one stays removed - Tools and Overlays are icon-only - the Print and Overlays menus show their icons; an item that is on takes the accent, since the icon sits in the checkmark's place - "Open Tools menu" shortcut action, unbound by default --- docs/USER_GUIDE.md | 2 +- negpy/desktop/view/canvas/toolbar.py | 157 +++++++++++++++++++++-- negpy/desktop/view/keyboard_shortcuts.py | 1 + negpy/desktop/view/shortcut_registry.py | 1 + tests/test_canvas_toolbar.py | 120 ++++++++++++++--- 5 files changed, 247 insertions(+), 34 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index d48a6277b..54acca7d6 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -15,7 +15,7 @@ If Windows blocks the default data folder, NegPy suggests `%LOCALAPPDATA%\NegPy\ ### Screen layout * **Left, the film strip**: your frames as a contact sheet, with import, sorting and triage tools. -* **Center, the canvas**: the live preview. Scroll or pinch to zoom, drag to pan. The bottom toolbar holds navigation, zoom (**Fit to Window**, 25–400%, **HQ Preview**), rotate/flip (on every selected frame), undo/redo, the **Print** menu (Before / After and the peeks below) and **Overlays** (Zone Overlay, Clipping Overlay, Printing Notes, Grain Focuser). An armed canvas tool shows its use, its slider and **Done** on the toolbar. The **⋯** menu holds every action, including **Preferences…** (§15). Right-click the toolbar for **Edit Toolbar…**. Right-click the image for **Reset View**, **Sticky Zoom**, the pickers, copy/paste settings, **Reset Settings**, **Reset to Roll Settings** and **Unload** (drop the frame from the session; its edit stays). +* **Center, the canvas**: the live preview. Scroll or pinch to zoom, drag to pan. The bottom toolbar holds navigation, zoom (**Fit to Window**, 25–400%, **HQ Preview**), rotate/flip (on every selected frame), **Tools** (arms any canvas tool: crop, straighten, the pickers, masks, retouching), undo/redo, the **Print** menu (Before / After and the peeks below) and **Overlays** (Zone Overlay, Clipping Overlay, Printing Notes, Grain Focuser). An armed canvas tool shows its use, its slider and **Done** on the toolbar; its name opens the same menu to switch tools. The **⋯** menu holds every action, including **Preferences…** (§15). Right-click the toolbar for **Edit Toolbar…**. Right-click the image for **Reset View**, **Sticky Zoom**, the pickers, copy/paste settings, **Reset Settings**, **Reset to Roll Settings** and **Unload** (drop the frame from the session; its edit stays). * **Right, the controls**: tabs **Roll** / **Frame** / **Metadata** / **Gear** / **Export** / **Scan**. **Frame** has a pinned **Analysis** readout and its own row of tabs. Only Roll and Frame change the render. With nothing open, the canvas offers **Load Scans…** and **Take the Tour**. diff --git a/negpy/desktop/view/canvas/toolbar.py b/negpy/desktop/view/canvas/toolbar.py index 313aabfc4..7d2e47ad6 100644 --- a/negpy/desktop/view/canvas/toolbar.py +++ b/negpy/desktop/view/canvas/toolbar.py @@ -2,6 +2,7 @@ import qtawesome as qta from PyQt6.QtCore import QSize, Qt +from PyQt6.QtGui import QCursor, QIcon from PyQt6.QtWidgets import ( QFrame, QHBoxLayout, @@ -10,6 +11,7 @@ QToolButton, QVBoxLayout, QWidget, + QWidgetAction, ) from negpy.desktop.controller import AppController @@ -18,8 +20,15 @@ from negpy.desktop.view.widgets.granular_settings_dialog import open_paste_dialog, open_sync_bounds_dialog from negpy.desktop.view.shortcut_registry import label_with_shortcut, tooltip_with_shortcut from negpy.desktop.view.widgets.choice_button import ChoiceButton, MenuButton, ToggleMenuButton -from negpy.desktop.view.widgets.collapsible import roll_revert_icon -from negpy.desktop.view.styles.templates import EditedDot, default_button_height, hint_label, labeled_action, wrap_tooltip +from negpy.desktop.view.widgets.collapsible import hidden_by_gating, roll_revert_icon +from negpy.desktop.view.styles.templates import ( + EditedDot, + default_button_height, + hint_label, + labeled_action, + section_subheader, + wrap_tooltip, +) from negpy.desktop.view.styles.theme import THEME CANVAS_COLORS = [ @@ -48,6 +57,7 @@ class ToolbarItem: ToolbarItem("rot_r", "Geometry", "Rotate CW"), ToolbarItem("flip_h", "Geometry", "Flip Horizontal"), ToolbarItem("flip_v", "Geometry", "Flip Vertical"), + ToolbarItem("tools", "Tools", "Tools"), ToolbarItem("undo", "Edit", "Undo"), ToolbarItem("redo", "Edit", "Redo"), ToolbarItem("source", "View", "Print or Peek"), @@ -77,6 +87,7 @@ class ToolbarItem: "rot_r", "flip_h", "flip_v", + "tools", "undo", "redo", "source", @@ -99,6 +110,10 @@ class ToolbarItem: _TOOLBAR_SETTING_KEY = "toolbar_items" +# A new item reaches a stored row once, before its anchor (else last); the record keeps a removed one out. +_ADDED_IDS = {"tools": "undo"} +_ADDED_SETTING_KEY = "toolbar_added_items" + # A narrow canvas hides these first to last; items not listed (menu actions put on the row) go # before them, from the end of the user's order. _COLLAPSE_ORDER = ( @@ -111,6 +126,7 @@ class ToolbarItem: "overlays", "redo", "undo", + "tools", "source", "zoom", "position", @@ -131,7 +147,13 @@ class ToolbarItem: "toggle_negative_peek", "The source as it was loaded, un-inverted and unedited, at your crop and rotation (no color management)", ), - ("flat_peek", "fa5s.eye", "Flat Scan", "toggle_flat_peek", "The flat master. Turn on Flat output in the Export tab to use it."), + ( + "flat_peek", + "fa5s.eye", + "Flat Scan", + "toggle_flat_peek", + "The flat master. Turn on Flat output in the Export tab to use it.", + ), ( "embedded_peek", "fa5s.camera", @@ -179,6 +201,34 @@ class ToolbarItem: } _CLONE_SOURCE_HINT = "Click the photo to pick the area to copy from" +# The Tools menu, in panel order: a panel section's title, then its tools as (tool, action id, sidebar button +# path from MainWindow). Zone Placement has no entry: it arms only with a zone picked on the zone strip. +_CANVAS_TOOLS: tuple[tuple[ToolMode, str, str] | str, ...] = ( + "Geometry", + (ToolMode.CROP_MANUAL, "manual_crop", "controls_panel.geometry_sidebar.manual_crop_btn"), + (ToolMode.STRAIGHTEN, "straighten", "controls_panel.geometry_sidebar.straighten_btn"), + (ToolMode.KEYSTONE_LINES, "keystone_lines", "controls_panel.geometry_sidebar.keystone_lines_btn"), + "Metering", + (ToolMode.ANALYSIS_DRAW, "analysis_draw", "controls_panel.process_sidebar.analysis_region_btn"), + "Filtration", + (ToolMode.WB_PICK, "pick_wb", "controls_panel.color_sidebar.pick_wb_btn"), + "Dodge & Burn", + (ToolMode.LOCAL_DRAW, "local_draw", "controls_panel.local_sidebar.draw_btn"), + (ToolMode.LOCAL_OVAL, "local_oval", "controls_panel.local_sidebar.oval_btn"), + (ToolMode.LOCAL_GRADIENT, "local_gradient", "controls_panel.local_sidebar.gradient_btn"), + "Retouch", + (ToolMode.DUST_PICK, "pick_dust", "controls_panel.retouch_sidebar.pick_dust_btn"), + (ToolMode.SCRATCH_PICK, "pick_scratch", "controls_panel.retouch_sidebar.pick_scratch_btn"), + (ToolMode.SCRATCH_LINE, "pick_scratch_line", "controls_panel.retouch_sidebar.pick_line_btn"), + (ToolMode.CLONE, "clone_tool", "controls_panel.retouch_sidebar.clone_btn"), +) +_TOOL_BUTTON_PATHS = {spec[0]: spec[2] for spec in _CANVAS_TOOLS if not isinstance(spec, str)} + + +def _checkable_menu_icon(name: str): + """The icon takes the checkmark's place in the menu, so the accent marks the item on.""" + return qta.icon(name, color=THEME.text_primary, color_on=THEME.accent_primary, color_disabled=THEME.text_muted) + def frame_position(model, actual_idx: int) -> str: """n / total in the Film Strip's order and filter, the one Next and Previous follow.""" @@ -195,6 +245,15 @@ def _migrate_retired(stored: list[str]) -> list[str]: return out +def _add_new_items(stored: list[str], added: list[str]) -> list[str]: + out = list(stored) + for item_id, anchor in _ADDED_IDS.items(): + if item_id in added or item_id in out: + continue + out.insert(out.index(anchor) if anchor in out else len(out), item_id) + return out + + def load_toolbar_items(repo) -> list[str]: """No stored row means the stock one; unknown ids drop so a retired button degrades quietly.""" stored = repo.get_global_setting(_TOOLBAR_SETTING_KEY) @@ -204,6 +263,11 @@ def load_toolbar_items(repo) -> list[str]: if any(item_id in _RETIRED_IDS for item_id in stored): stored = _migrate_retired(stored) repo.save_global_setting(_TOOLBAR_SETTING_KEY, stored) + added = repo.get_global_setting(_ADDED_SETTING_KEY, []) + added = added if isinstance(added, list) else [] + if any(item_id not in added for item_id in _ADDED_IDS): + stored = _add_new_items(stored, added) + repo.save_global_settings({_TOOLBAR_SETTING_KEY: stored, _ADDED_SETTING_KEY: sorted(set(added) | set(_ADDED_IDS))}) return [item_id for item_id in stored if item_id in TOOLBAR_ITEM_BY_ID] @@ -330,16 +394,20 @@ def _init_ui(self) -> None: tuple((icon, label) for _flag, icon, label, _key, _tip in _VIEW_SOURCES), "What the canvas shows: the print, a before/after split, or a peek that leaves your edit alone", ) - for i, (_flag, _icon, label, key, tip) in enumerate(_VIEW_SOURCES): + for i, (_flag, icon, label, key, tip) in enumerate(_VIEW_SOURCES): + self.source_choice.choice_action(i).setIcon(_checkable_menu_icon(icon)) if key: self._label(self.source_choice.choice_action(i), label, key) self.source_choice.set_choice_tooltip(i, tip) - self.overlays_menu = ToggleMenuButton("fa5s.layer-group", "Overlays", "Marks drawn over the frame; none of them changes the edit") + self.overlays_menu = ToggleMenuButton( + "fa5s.layer-group", "", "Overlays — marks drawn over the frame; none of them changes the edit" + ) self._overlay_actions = {} - for flag, label, key, tip, toggle in ( + for flag, icon, label, key, tip, toggle in ( ( "zones_overlay", + "mdi6.numeric-5-box-outline", "Zone Overlay", "toggle_zones", "Label each region of the print with its Adams zone", @@ -347,6 +415,7 @@ def _init_ui(self) -> None: ), ( "clipping_overlay", + "fa5s.adjust", "Clipping Overlay", "toggle_clipping_overlay", "Mark paper black and paper white on the frame", @@ -354,6 +423,7 @@ def _init_ui(self) -> None: ), ( "printing_notes", + "mdi.playlist-edit", "Printing Notes", "toggle_printing_notes", "The dodge and burn map and the print recipe", @@ -361,6 +431,7 @@ def _init_ui(self) -> None: ), ( "grain_focuser", + "fa5s.search-plus", "Grain Focuser", "toggle_grain_focuser", "A loupe at the cursor showing the frame's own pixels, with an acutance figure for comparing sharpness across the frame (reads true on HQ)", @@ -368,10 +439,16 @@ def _init_ui(self) -> None: ), ): action = self.overlays_menu.add_toggle(label, tip) + action.setIcon(_checkable_menu_icon(icon)) self._label(action, label, key) action.triggered.connect(lambda checked, toggle=toggle: toggle(force=checked)) self._overlay_actions[flag] = action + self.btn_tools = MenuButton("Tools — crop, straighten, the pickers, masks and retouching; each opens its own row here") + self.btn_tools.setIcon(qta.icon("fa5s.tools", color=icon_color, color_disabled=THEME.text_muted)) + self.btn_tools.setMinimumWidth(self.btn_tools.sizeHint().width()) + self._fill_tools_menu(self.btn_tools.choice_menu) + # One-shot actions that otherwise live only in the ⋯ menu. self.btn_reference = self._action_button( qta.icon("fa5s.thumbtack", color=icon_color), @@ -462,6 +539,7 @@ def _init_ui(self) -> None: find_action = overflow_menu.addAction(qta.icon("fa5s.search", color=icon_color), "Find Control or Action…", self._show_palette) self._label(find_action, "Find Control or Action…", "command_palette") find_action.setToolTip("Find any slider, card or action by name, and open it") + self._fill_tools_menu(overflow_menu.addMenu(qta.icon("fa5s.tools", color=icon_color), "Tools")) overflow_menu.addSeparator() self._ov_hq_action = overflow_menu.addAction("HQ Preview") @@ -636,7 +714,7 @@ def _init_ui(self) -> None: btn.setFixedHeight(btn_height) btn.setCursor(Qt.CursorShape.PointingHandCursor) btn.setToolTip(wrap_tooltip(btn.toolTip())) - for btn in (self.btn_zoom, self.source_choice, self.overlays_menu): + for btn in (self.btn_zoom, self.btn_tools, self.source_choice, self.overlays_menu): btn.setCursor(Qt.CursorShape.PointingHandCursor) self._row_layout = row_layout @@ -649,6 +727,7 @@ def _init_ui(self) -> None: "rot_r": self.btn_rot_r, "flip_h": self.btn_flip_h, "flip_v": self.btn_flip_v, + "tools": self.btn_tools, "undo": self.btn_undo, "redo": self.btn_redo, "source": self.source_choice, @@ -690,9 +769,9 @@ def _build_tool_row(self, v_layout: QVBoxLayout, icon_color: str, icon_size: QSi self._tool_slider = None self._tool_slider_source = None - self.tool_name = QLabel() - self.tool_name.setFixedHeight(btn_height) - self.tool_name.setStyleSheet(f"color: {THEME.text_primary}; font-size: {THEME.font_size_base}px; font-weight: 600; padding: 0 4px;") + # Names the armed tool and switches to another one, with no Done in between. + self.tool_name = MenuButton("Switch to another canvas tool") + self._fill_tools_menu(self.tool_name.choice_menu) self.tool_hint = hint_label() self.tool_hint.setWordWrap(False) self.btn_tool_undo = QToolButton() @@ -732,7 +811,10 @@ def _sync_tool_row(self) -> None: spec = _TOOL_ROWS.get(mode) if spec is not None: name, hint, _path = spec - self.tool_name.setText(name) + button = self._find_widget(_TOOL_BUTTON_PATHS.get(mode, "")) + self.tool_name.setIcon(button.icon() if button is not None else QIcon()) + self.tool_name.setText(f" {name}" if button is not None else name) + self.tool_name.setMinimumWidth(self.tool_name.sizeHint().width()) self.tool_hint.setText(_CLONE_SOURCE_HINT if mode == ToolMode.CLONE and state.clone_picking else hint) self.tool_name.setToolTip(self.tool_hint.text()) self.btn_tool_undo.setEnabled(state.undo_index > 0) @@ -749,7 +831,7 @@ def _sync_tool_row(self) -> None: if self._tool_slider is not None: self._tool_slider.setEnabled(self._tool_slider_source.isEnabled()) - def _find_tool_slider(self, path: str): + def _find_widget(self, path: str): from negpy.desktop.view.main_window import MainWindow win = self.window() @@ -769,7 +851,7 @@ def _swap_tool_slider(self, path: str) -> None: self._tool_slider.setParent(None) self._tool_slider.deleteLater() self._tool_slider = self._tool_slider_source = None - source = self._find_tool_slider(path) + source = self._find_widget(path) if source is None: return mirror = clone_slider(source) @@ -783,6 +865,55 @@ def _swap_tool_slider(self, path: str) -> None: self._tool_layout.insertWidget(self._tool_slider_index, mirror) self._tool_slider, self._tool_slider_source = mirror, source + def _fill_tools_menu(self, menu: QMenu) -> None: + """Each item runs the tool's own shortcut action, which shows its tab and refuses a gated tool.""" + menu.setToolTipsVisible(True) + entries = [] + for spec in _CANVAS_TOOLS: + if isinstance(spec, str): + header = section_subheader(spec) + header.setContentsMargins(THEME.space_lg, 0, THEME.space_lg, 0) + section = QWidgetAction(menu) + section.setDefaultWidget(header) + section.setEnabled(False) + menu.addAction(section) + continue + mode, action_id, path = spec + name, hint, _slider = _TOOL_ROWS[mode] + action = menu.addAction(name) + self._label(action, name, action_id) + action.setToolTip(hint) + action.triggered.connect(lambda _checked=False, a=action_id: self._arm_tool(a)) + entries.append((mode, path, action)) + + def refresh() -> None: + current = self.session.state.active_tool + for mode, path, action in entries: + button = self._find_widget(path) + usable = button is None or (button.isEnabled() and not hidden_by_gating(button)) + if button is not None: + action.setIcon(button.icon()) + # The armed tool's own item would toggle it off. + action.setEnabled(mode != current and usable) + font = action.font() + font.setBold(mode == current) + action.setFont(font) + + menu.aboutToShow.connect(refresh) + + def _arm_tool(self, action_id: str) -> None: + from negpy.desktop.view.main_window import MainWindow + + win = self.window() + if isinstance(win, MainWindow): + action = win.shortcut_manager.action_for(action_id) + if action is not None: + action() + + def open_tools_menu(self) -> None: + """At the cursor: the row may have hidden the Tools button.""" + self.btn_tools.choice_menu.exec(QCursor.pos()) + def _on_tool_done(self) -> None: overlay = getattr(self.parentWidget(), "overlay", None) if overlay is not None: diff --git a/negpy/desktop/view/keyboard_shortcuts.py b/negpy/desktop/view/keyboard_shortcuts.py index 31816dc57..54e793671 100644 --- a/negpy/desktop/view/keyboard_shortcuts.py +++ b/negpy/desktop/view/keyboard_shortcuts.py @@ -364,6 +364,7 @@ def _build_actions(self) -> dict[str, Callable[[], None]]: "local_oval": lambda: _toggle_tool_button(self.window, "tone", controls.local_sidebar.oval_btn), "local_gradient": lambda: _toggle_tool_button(self.window, "tone", controls.local_sidebar.gradient_btn), "analysis_draw": lambda: _toggle_tool_button(self.window, "roll", controls.process_sidebar.analysis_region_btn), + "open_tools_menu": lambda: self.window.toolbar.open_tools_menu(), "toggle_flat_peek": controller.toggle_flat_peek, "toggle_negative_peek": controller.toggle_negative_peek, "toggle_embedded_peek": controller.toggle_embedded_peek, diff --git a/negpy/desktop/view/shortcut_registry.py b/negpy/desktop/view/shortcut_registry.py index 1ed4dd9e4..d79bbfa16 100644 --- a/negpy/desktop/view/shortcut_registry.py +++ b/negpy/desktop/view/shortcut_registry.py @@ -74,6 +74,7 @@ class ShortcutEntry: "local_oval": ShortcutEntry("", "Toggle dodge & burn oval mask draw", "Tools"), "local_gradient": ShortcutEntry("", "Toggle dodge & burn card-edge mask draw", "Tools"), "analysis_draw": ShortcutEntry("Ctrl+R", "Toggle analysis region draw", "Tools"), + "open_tools_menu": ShortcutEntry("", "Open the Tools menu: every canvas tool", "Tools"), "toggle_flat_peek": ShortcutEntry("M", "Peek flat scan (digital intermediate)", "Tools"), "toggle_negative_peek": ShortcutEntry("N", "Peek negative (source as loaded)", "Tools"), "toggle_embedded_peek": ShortcutEntry("P", "Peek the camera's embedded preview", "Tools"), diff --git a/tests/test_canvas_toolbar.py b/tests/test_canvas_toolbar.py index 4bd32ca27..1e10f4ff7 100644 --- a/tests/test_canvas_toolbar.py +++ b/tests/test_canvas_toolbar.py @@ -47,6 +47,14 @@ def _make_toolbar() -> ActionToolbar: return ActionToolbar(controller) +def _repo(stored, added=("tools",)) -> MagicMock: + """A stored row whose new items were already offered, unless *added* says otherwise.""" + repo = MagicMock() + settings = {"toolbar_items": stored, "toolbar_added_items": list(added)} + repo.get_global_setting.side_effect = lambda key, default=None: settings.get(key, default) + return repo + + def _visible_item_count(tb: ActionToolbar) -> int: return sum(1 for item_id in tb._item_ids if not tb._row_widgets[item_id].isHidden()) @@ -136,6 +144,7 @@ def test_every_visible_control_shares_one_row_height(self): tb.btn_rot_r, tb.btn_flip_h, tb.btn_flip_v, + tb.btn_tools, tb.btn_undo, tb.btn_redo, tb.source_choice, @@ -263,34 +272,48 @@ def test_default_row_is_used_when_nothing_is_stored(self): self.assertEqual(_row_order(tb), list(DEFAULT_TOOLBAR_IDS)) def test_load_drops_unknown_ids(self): - repo = MagicMock() - repo.get_global_setting.return_value = ["overlays", "retired_button", "prev"] + repo = _repo(["overlays", "retired_button", "prev"]) self.assertEqual(load_toolbar_items(repo), ["overlays", "prev"]) def test_folded_buttons_become_their_menu_and_the_row_is_saved_once(self): - repo = MagicMock() - repo.get_global_setting.return_value = [ - "prev", - "next", - "zoom_label", - "zoom_fit", - "hq", - "undo", - "compare", - "negative_peek", - "zones", - "loupe", - ] + repo = _repo( + [ + "prev", + "next", + "zoom_label", + "zoom_fit", + "hq", + "undo", + "compare", + "negative_peek", + "zones", + "loupe", + ] + ) migrated = ["prev", "position", "next", "zoom", "undo", "source", "overlays"] self.assertEqual(load_toolbar_items(repo), migrated) repo.save_global_setting.assert_called_once_with("toolbar_items", migrated) def test_a_removed_frame_counter_stays_removed(self): - repo = MagicMock() - repo.get_global_setting.return_value = ["prev", "next", "zoom"] + repo = _repo(["prev", "next", "zoom"]) self.assertEqual(load_toolbar_items(repo), ["prev", "next", "zoom"]) repo.save_global_setting.assert_not_called() + def test_a_stored_row_gets_tools_once_before_undo(self): + repo = _repo(["prev", "undo", "redo"], added=()) + self.assertEqual(load_toolbar_items(repo), ["prev", "tools", "undo", "redo"]) + repo.save_global_settings.assert_called_once_with( + {"toolbar_items": ["prev", "tools", "undo", "redo"], "toolbar_added_items": ["tools"]} + ) + + def test_a_stored_row_without_undo_gets_tools_last(self): + self.assertEqual(load_toolbar_items(_repo(["prev", "next"], added=())), ["prev", "next", "tools"]) + + def test_a_removed_tools_button_stays_removed(self): + repo = _repo(["prev", "undo"]) + self.assertEqual(load_toolbar_items(repo), ["prev", "undo"]) + repo.save_global_settings.assert_not_called() + def test_load_tolerates_a_malformed_setting(self): repo = MagicMock() repo.get_global_setting.return_value = "prev" @@ -298,8 +321,7 @@ def test_load_tolerates_a_malformed_setting(self): def test_stored_order_drives_the_row(self): tb = _make_toolbar() - tb.controller.session.repo.get_global_setting.return_value = ["overlays", "prev", "undo"] - tb._item_ids = load_toolbar_items(tb.session.repo) + tb._item_ids = load_toolbar_items(_repo(["overlays", "prev", "undo"])) tb._rebuild_row() self.assertEqual(_row_order(tb), ["overlays", "prev", "undo"]) @@ -668,7 +690,7 @@ def test_the_slider_mirrors_the_sidebar_both_ways(self): source = CompactSlider("Brush Size", 1, 50, 10, step=1) source.setValue(10) tb = _make_toolbar() - with unittest.mock.patch.object(tb, "_find_tool_slider", return_value=source): + with unittest.mock.patch.object(tb, "_find_widget", side_effect=lambda path: source if path.endswith("slider") else None): tb.session.state.active_tool = ToolMode.DUST_PICK tb._sync_tool_row() mirror = tb._tool_slider @@ -692,3 +714,61 @@ def test_a_narrow_canvas_drops_the_hint_before_done(self): QApplication.processEvents() self.assertTrue(tb.tool_hint.isHidden()) self.assertFalse(tb.btn_tool_done.isHidden()) + + +class TestToolsMenu(unittest.TestCase): + def _items(self, menu): + from PyQt6.QtWidgets import QWidgetAction + + return [a for a in menu.actions() if not a.isSeparator() and not isinstance(a, QWidgetAction)] + + def test_every_menu_tool_has_a_row_and_a_registry_action(self): + from negpy.desktop.view.canvas.toolbar import _CANVAS_TOOLS, _TOOL_ROWS + from negpy.desktop.view.shortcut_registry import REGISTRY + + tools = [spec for spec in _CANVAS_TOOLS if not isinstance(spec, str)] + self.assertEqual({mode for mode, _a, _p in tools}, set(_TOOL_ROWS) - {ToolMode.ZONE_PLACE}) + for _mode, action_id, _path in tools: + self.assertIn(action_id, REGISTRY) + self.assertIn("open_tools_menu", REGISTRY) + + def test_the_menu_is_grouped_under_panel_section_titles(self): + from PyQt6.QtWidgets import QWidgetAction + + tb = _make_toolbar() + headers = [a.defaultWidget().text() for a in tb.btn_tools.choice_menu.actions() if isinstance(a, QWidgetAction)] + self.assertEqual(headers, ["GEOMETRY", "METERING", "FILTRATION", "DODGE & BURN", "RETOUCH"]) + + def test_tools_is_on_the_stock_row_and_collapses(self): + from negpy.desktop.view.canvas.toolbar import _COLLAPSE_ORDER + + self.assertIn("tools", DEFAULT_TOOLBAR_IDS) + self.assertIn("tools", _COLLAPSE_ORDER) + + def test_an_item_runs_the_tools_shortcut_action(self): + tb = _make_toolbar() + with unittest.mock.patch.object(tb, "_arm_tool") as arm: + heal = next(a for a in self._items(tb.btn_tools.choice_menu) if a.text().startswith("Heal")) + heal.trigger() + arm.assert_called_once_with("pick_dust") + + def test_a_gated_tool_and_the_armed_tool_are_grayed_out(self): + from PyQt6.QtWidgets import QPushButton + + tb = _make_toolbar() + disabled = QPushButton() + disabled.setEnabled(False) + tb.session.state.active_tool = ToolMode.CROP_MANUAL + with unittest.mock.patch.object( + tb, "_find_widget", side_effect=lambda path: disabled if path.endswith("clone_btn") else QPushButton() + ): + tb.tool_name.choice_menu.aboutToShow.emit() + enabled = {a.text().split()[0]: a.isEnabled() for a in self._items(tb.tool_name.choice_menu)} + self.assertFalse(enabled["Clone"]) + self.assertFalse(enabled["Crop"]) + self.assertTrue(enabled["Heal"]) + + def test_the_overflow_menu_carries_the_tools(self): + tb = _make_toolbar() + submenu = next(a.menu() for a in tb.btn_overflow.menu().actions() if a.menu() is not None and a.text() == "Tools") + self.assertEqual(len(self._items(submenu)), len(self._items(tb.btn_tools.choice_menu))) From b90b3634708c890bbdb324b2e481fe7ae6cb83b8 Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Mon, 12 Oct 2026 08:32:08 +0200 Subject: [PATCH 2/2] fix(toolbar): Flat Scan peek no longer waits on Flat output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Print menu grayed out Flat Scan until Flat output was on in the Export tab, yet the peek renders the flat master from any frame and its key (M) always worked. The menu and the ⋯ item now match the key. --- negpy/desktop/view/canvas/toolbar.py | 6 +----- tests/test_canvas_toolbar.py | 9 +++++---- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/negpy/desktop/view/canvas/toolbar.py b/negpy/desktop/view/canvas/toolbar.py index 7d2e47ad6..c55887a99 100644 --- a/negpy/desktop/view/canvas/toolbar.py +++ b/negpy/desktop/view/canvas/toolbar.py @@ -152,7 +152,7 @@ class ToolbarItem: "fa5s.eye", "Flat Scan", "toggle_flat_peek", - "The flat master. Turn on Flat output in the Export tab to use it.", + "The flat master: the scan as a log-encoded digital intermediate, with no print curve", ), ( "embedded_peek", @@ -927,9 +927,6 @@ def _sync_view_state(self, *_args) -> None: self.source_choice.blockSignals(True) self.source_choice.setCurrentIndex(active) self.source_choice.blockSignals(False) - flat_ok = bool(state.flat_output) - self.source_choice.set_choice_enabled(_SOURCE_INDEX["flat_peek"], flat_ok) - self._ov_flat_peek_action.setEnabled(flat_ok) self.source_choice.set_choice_enabled(_SOURCE_INDEX["flatfield_peek"], bool(state.config.flatfield.profile_id)) for flag, action in self._overlay_actions.items(): action.setChecked(bool(getattr(state, flag))) @@ -990,7 +987,6 @@ def _connect_signals(self) -> None: controller.negative_peek_changed, controller.embedded_peek_changed, controller.flatfield_peek_changed, - controller.flat_output_changed, controller.zones_overlay_changed, controller.clipping_overlay_changed, controller.printing_notes_changed, diff --git a/tests/test_canvas_toolbar.py b/tests/test_canvas_toolbar.py index 1e10f4ff7..cdd28ed39 100644 --- a/tests/test_canvas_toolbar.py +++ b/tests/test_canvas_toolbar.py @@ -612,13 +612,14 @@ def test_a_refused_peek_falls_back_to_print(self): self._choose(tb, "Embedded") self.assertEqual(tb.source_choice.text().strip(), "Print") - def test_flat_scan_waits_on_flat_output(self): + def test_flat_scan_needs_no_flat_output(self): + """The peek renders the flat master from any frame, like its key.""" tb = _make_toolbar() - flat = next(i for i in range(tb.source_choice.count()) if tb.source_choice.choice_action(i).text().startswith("Flat Scan")) - self.assertFalse(tb.source_choice.is_choice_enabled(flat)) - tb.session.state.flat_output = True + tb.session.state.flat_output = False tb._sync_view_state() + flat = next(i for i in range(tb.source_choice.count()) if tb.source_choice.choice_action(i).text().startswith("Flat Scan")) self.assertTrue(tb.source_choice.is_choice_enabled(flat)) + self.assertTrue(tb._ov_flat_peek_action.isEnabled()) class TestOverlaysMenu(unittest.TestCase):