From fb111dcdc9e6ba53a30f944a48bc4b907bc82d7d Mon Sep 17 00:00:00 2001 From: omigamedev Date: Wed, 3 Jun 2026 12:42:23 +0200 Subject: [PATCH] Add main toolbar service boundary --- docs/modernization/debt.md | 2 +- docs/modernization/roadmap.md | 9 ++- src/app_core/main_toolbar.h | 50 ++++++++++++ src/app_layout.cpp | 112 +++++++++++++++++++------- tests/app_core/main_toolbar_tests.cpp | 77 ++++++++++++++++++ 5 files changed, 218 insertions(+), 32 deletions(-) diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index d0bf61b..2bd93b8 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -52,7 +52,7 @@ agent or engineer to remove them without reconstructing context from chat. | DEBT-0032 | Open | Modernization | Layer menu command planning now consumes pure `pp_app_core` through `App::init_menu_layer` and `pano_cli plan-layer-menu`, but live execution still calls legacy `Canvas::clear`, `App::dialog_layer_rename`, `NodePanelLayer::merge`, and reads `Canvas::I` animation/layer state directly | Preserve existing Layer menu behavior while layer commands move toward document/app services | `pp_app_core_document_layer_tests`; `pano_cli plan-layer-menu --command merge --current-index 2 --lower-name Paint`; `pano_cli plan-layer-menu --command rename --no-current-layer`; `ctest --preset desktop-fast --build-config Debug` | Layer clear, rename, merge-down execution, animation gating, and selected-layer state are owned by document/app services with Layer-menu callbacks acting only as UI adapters | | DEBT-0033 | Open | Modernization | Tools menu and floating-panel planning now consumes pure `pp_app_core` through `App::init_menu_tools`, `pano_cli plan-tools-menu`, and `pano_cli plan-tools-panel`, but live execution still constructs legacy `NodePanelFloating` panels, mutates legacy panel nodes, clears `CanvasModeGrid`, resets `NodeCanvas` camera state, opens legacy shortcuts UI, and calls the iOS SonarPen bridge directly | Preserve current Tools menu behavior while UI shell actions move toward app/UI/platform services | `pp_app_core_tools_menu_tests`; `pano_cli plan-tools-menu --command shortcuts`; `pano_cli plan-tools-panel --panel layers`; `pano_cli plan-tools-panel --panel animation --already-visible`; `ctest --preset desktop-fast --build-config Debug` | Tools panel creation, submenu routing, grid clear, camera reset, shortcuts dialog, and SonarPen dispatch are owned by app/UI/platform services with `App::init_menu_tools` acting only as a UI adapter | | DEBT-0034 | Open | Modernization | About menu command planning now consumes pure `pp_app_core` through `App::init_menu_about` and `pano_cli plan-about-menu`, but live execution still opens legacy About/manual/what's-new dialogs, invokes the injected crash hook, and runs the legacy Canvas stroke performance test directly | Preserve About menu behavior while dialogs and diagnostics move toward app/UI/platform services | `pp_app_core_about_menu_tests`; `pano_cli plan-about-menu --command news --version-major 2 --version-minor 5 --version-fix 7`; `pano_cli plan-about-menu --command performance --no-canvas`; `ctest --preset desktop-fast --build-config Debug` | About/manual/what's-new dialog dispatch, crash-test dispatch, and performance-test execution are owned by app/UI/platform services with `App::init_menu_about` acting only as a UI adapter | -| DEBT-0035 | Open | Modernization | Main toolbar/status command planning now consumes pure `pp_app_core` through `App::init_toolbar_main` and `pano_cli plan-main-toolbar`, but live execution still opens legacy open/save/settings/message-box dialogs, mutates legacy `ActionManager` history, and clears the legacy `Canvas` directly | Preserve reachable toolbar/status behavior while app shell commands move toward app/document/UI services | `pp_app_core_main_toolbar_tests`; `pano_cli plan-main-toolbar --command undo --undo-count 2`; `pano_cli plan-main-toolbar --command clear-canvas --no-canvas`; `ctest --preset desktop-fast --build-config Debug` | Open/save/settings/message-box routing, undo/redo/clear-history execution, and canvas-clear execution are owned by app/document/UI services with `App::init_toolbar_main` acting only as a UI adapter | +| DEBT-0035 | Open | Modernization | Main toolbar/status command planning and execution dispatch now consume pure `pp_app_core` through `App::init_toolbar_main`, `pano_cli plan-main-toolbar`, and the `MainToolbarServices` boundary, but the live adapter still opens legacy open/save/settings/message-box dialogs, mutates legacy `ActionManager` history, and clears the legacy `Canvas` directly | Preserve reachable toolbar/status behavior while app shell commands move toward app/document/UI services | `pp_app_core_main_toolbar_tests`; `pano_cli plan-main-toolbar --command undo --undo-count 2`; `pano_cli plan-main-toolbar --command clear-canvas --no-canvas`; `ctest --preset desktop-fast --build-config Debug` | Open/save/settings/message-box routing, undo/redo/clear-history execution, and canvas-clear execution are owned by injected app/document/UI services with `App::init_toolbar_main` acting only as a UI adapter and no legacy toolbar adapter | ## Closed Debt diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index 167d55a..2fa57cd 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -537,8 +537,10 @@ legacy `ActionManager` stack execution continues. `pano_cli plan-main-toolbar` exposes app-core planning for the live main toolbar/status-bar shell, including open/save dialogs, undo/redo availability, clear-history availability, clear-canvas no-canvas blocking, message-box -creation, and settings dialog routing before legacy dialogs, `ActionManager`, -and `Canvas` execution continue. +creation, and settings dialog routing. `pp_app_core` now also owns a +`MainToolbarServices` executor boundary, so `App::init_toolbar_main` dispatches +through a legacy adapter before legacy dialogs, `ActionManager`, and `Canvas` +execution continue. `pano_cli plan-quick-operation` exposes app-core planning for quick brush/color slot selection versus popup opening, plus quick mini-state restore/reset validation used by the live quick panel before legacy `Brush`, color picker, @@ -1293,7 +1295,8 @@ Results: planning as JSON automation. - `pp_app_core_main_toolbar_tests` passed, covering live toolbar/status direct dialog routing, undo/redo availability, clear-history availability, no-canvas - clear blocking, and negative history metric rejection. + clear blocking, negative history metric rejection, and dispatch through the + `MainToolbarServices` executor boundary without invoking no-op actions. - `pano_cli_plan_main_toolbar_undo_smoke`, `pano_cli_plan_main_toolbar_redo_empty_smoke`, `pano_cli_plan_main_toolbar_clear_canvas_no_canvas_smoke`, and diff --git a/src/app_core/main_toolbar.h b/src/app_core/main_toolbar.h index 6455280..eed66f6 100644 --- a/src/app_core/main_toolbar.h +++ b/src/app_core/main_toolbar.h @@ -43,6 +43,20 @@ struct MainToolbarPlan { bool no_op = false; }; +class MainToolbarServices { +public: + virtual ~MainToolbarServices() = default; + + virtual void show_open_dialog() = 0; + virtual void show_save_dialog() = 0; + virtual void invoke_undo() = 0; + virtual void invoke_redo() = 0; + virtual void clear_history() = 0; + virtual void clear_canvas() = 0; + virtual void show_message_box() = 0; + virtual void show_settings_dialog() = 0; +}; + [[nodiscard]] inline pp::foundation::Result plan_main_toolbar_command( MainToolbarCommand command, int undo_count = 0, @@ -143,4 +157,40 @@ struct MainToolbarPlan { pp::foundation::Status::invalid_argument("unknown main toolbar command")); } +[[nodiscard]] inline pp::foundation::Status execute_main_toolbar_plan( + const MainToolbarPlan& plan, + MainToolbarServices& services) +{ + switch (plan.action) { + case MainToolbarAction::show_open_dialog: + services.show_open_dialog(); + return pp::foundation::Status::success(); + case MainToolbarAction::show_save_dialog: + services.show_save_dialog(); + return pp::foundation::Status::success(); + case MainToolbarAction::invoke_undo: + services.invoke_undo(); + return pp::foundation::Status::success(); + case MainToolbarAction::invoke_redo: + services.invoke_redo(); + return pp::foundation::Status::success(); + case MainToolbarAction::clear_history: + services.clear_history(); + return pp::foundation::Status::success(); + case MainToolbarAction::clear_canvas: + services.clear_canvas(); + return pp::foundation::Status::success(); + case MainToolbarAction::show_message_box: + services.show_message_box(); + return pp::foundation::Status::success(); + case MainToolbarAction::show_settings_dialog: + services.show_settings_dialog(); + return pp::foundation::Status::success(); + case MainToolbarAction::no_op_unavailable: + return pp::foundation::Status::success(); + } + + return pp::foundation::Status::invalid_argument("unknown main toolbar action"); +} + } // namespace pp::app diff --git a/src/app_layout.cpp b/src/app_layout.cpp index 1fbed08..fec5ca2 100644 --- a/src/app_layout.cpp +++ b/src/app_layout.cpp @@ -249,6 +249,74 @@ void apply_tools_panel_chrome(NodePanelFloating& panel, const pp::app::ToolsPane panel.m_droppable = plan.droppable; } +class LegacyMainToolbarServices final : public pp::app::MainToolbarServices { +public: + explicit LegacyMainToolbarServices(App& app) noexcept + : app_(app) + { + } + + void show_open_dialog() override + { + app_.dialog_open(); + } + + void show_save_dialog() override + { + app_.dialog_save(); + } + + void invoke_undo() override + { + ActionManager::undo(); + } + + void invoke_redo() override + { + ActionManager::redo(); + } + + void clear_history() override + { + ActionManager::clear(); + } + + void clear_canvas() override + { + if (!app_.canvas || !app_.canvas->m_canvas) + return; + + app_.canvas->m_canvas->clear({ 0.0F, 0.0F, 0.0F, 0.0F }); + } + + void show_message_box() override + { + app_.msgbox = new NodeMessageBox(); + app_.msgbox->set_manager(&app_.layout); + app_.msgbox->init(); + app_.layout[app_.main_id]->add_child(app_.msgbox); + } + + void show_settings_dialog() override + { + app_.settings = new NodeSettings(); + app_.settings->set_manager(&app_.layout); + app_.settings->init(); + app_.layout[app_.main_id]->add_child(app_.settings); + } + +private: + App& app_; +}; + +void execute_main_toolbar_plan(App& app, const pp::app::MainToolbarPlan& plan) +{ + LegacyMainToolbarServices services(app); + const auto status = pp::app::execute_main_toolbar_plan(plan, services); + if (!status.ok()) + LOG("Main toolbar action failed: %s", status.message); +} + } // namespace void App::title_update() @@ -284,8 +352,8 @@ void App::init_toolbar_main() button->on_click = [this, button](Node*) { const auto plan = pp::app::plan_main_toolbar_command( pp::app::MainToolbarCommand::open_document); - if (plan && plan.value().action == pp::app::MainToolbarAction::show_open_dialog) - dialog_open(); + if (plan) + execute_main_toolbar_plan(*this, plan.value()); }; } if (auto* button = layout[main_id]->find("btn-save")) @@ -293,8 +361,8 @@ void App::init_toolbar_main() button->on_click = [this, button](Node*) { const auto plan = pp::app::plan_main_toolbar_command( pp::app::MainToolbarCommand::save_document); - if (plan && plan.value().action == pp::app::MainToolbarAction::show_save_dialog) - dialog_save(); + if (plan) + execute_main_toolbar_plan(*this, plan.value()); }; } if (auto* button = layout[main_id]->find("btn-undo")) @@ -303,8 +371,8 @@ void App::init_toolbar_main() const auto plan = pp::app::plan_main_toolbar_command( pp::app::MainToolbarCommand::undo, static_cast(ActionManager::I.m_actions.size())); - if (plan && plan.value().action == pp::app::MainToolbarAction::invoke_undo) - ActionManager::undo(); + if (plan) + execute_main_toolbar_plan(*this, plan.value()); }; } if (auto* button = layout[main_id]->find("btn-redo")) @@ -314,8 +382,8 @@ void App::init_toolbar_main() pp::app::MainToolbarCommand::redo, 0, static_cast(ActionManager::I.m_redos.size())); - if (plan && plan.value().action == pp::app::MainToolbarAction::invoke_redo) - ActionManager::redo(); + if (plan) + execute_main_toolbar_plan(*this, plan.value()); }; } if (auto* button = layout[main_id]->find("btn-clean-memory")) @@ -326,8 +394,8 @@ void App::init_toolbar_main() static_cast(ActionManager::I.m_actions.size()), static_cast(ActionManager::I.m_redos.size()), static_cast(ActionManager::I.m_memory)); - if (plan && plan.value().action == pp::app::MainToolbarAction::clear_history) - ActionManager::clear(); + if (plan) + execute_main_toolbar_plan(*this, plan.value()); }; } if (auto* button = layout[main_id]->find("btn-clear")) @@ -340,12 +408,8 @@ void App::init_toolbar_main() 0, 0, static_cast(canvas)); - if (plan && plan.value().action == pp::app::MainToolbarAction::clear_canvas) - canvas->m_canvas->clear({ - 0.0F, - 0.0F, - 0.0F, - 0.0F }); + if (plan) + execute_main_toolbar_plan(*this, plan.value()); }; } if (auto* button = layout[main_id]->find("btn-popup")) @@ -353,12 +417,8 @@ void App::init_toolbar_main() button->on_click = [this](Node*) { const auto plan = pp::app::plan_main_toolbar_command( pp::app::MainToolbarCommand::show_message_box); - if (!plan || plan.value().action != pp::app::MainToolbarAction::show_message_box) - return; - msgbox = new NodeMessageBox(); - msgbox->set_manager(&layout); - msgbox->init(); - layout[main_id]->add_child(msgbox); + if (plan) + execute_main_toolbar_plan(*this, plan.value()); }; } if (auto* button = layout[main_id]->find("btn-settings")) @@ -366,12 +426,8 @@ void App::init_toolbar_main() button->on_click = [this](Node*) { const auto plan = pp::app::plan_main_toolbar_command( pp::app::MainToolbarCommand::show_settings); - if (!plan || plan.value().action != pp::app::MainToolbarAction::show_settings_dialog) - return; - settings = new NodeSettings(); - settings->set_manager(&layout); - settings->init(); - layout[main_id]->add_child(settings); + if (plan) + execute_main_toolbar_plan(*this, plan.value()); }; } } diff --git a/tests/app_core/main_toolbar_tests.cpp b/tests/app_core/main_toolbar_tests.cpp index dbdd8a2..1f269c5 100644 --- a/tests/app_core/main_toolbar_tests.cpp +++ b/tests/app_core/main_toolbar_tests.cpp @@ -3,6 +3,39 @@ namespace { +class FakeMainToolbarServices final : public pp::app::MainToolbarServices { +public: + void show_open_dialog() override { open_dialogs += 1; } + void show_save_dialog() override { save_dialogs += 1; } + void invoke_undo() override { undo_calls += 1; } + void invoke_redo() override { redo_calls += 1; } + void clear_history() override { clear_history_calls += 1; } + void clear_canvas() override { clear_canvas_calls += 1; } + void show_message_box() override { message_boxes += 1; } + void show_settings_dialog() override { settings_dialogs += 1; } + + [[nodiscard]] int total_calls() const noexcept + { + return open_dialogs + + save_dialogs + + undo_calls + + redo_calls + + clear_history_calls + + clear_canvas_calls + + message_boxes + + settings_dialogs; + } + + int open_dialogs = 0; + int save_dialogs = 0; + int undo_calls = 0; + int redo_calls = 0; + int clear_history_calls = 0; + int clear_canvas_calls = 0; + int message_boxes = 0; + int settings_dialogs = 0; +}; + void direct_dialog_commands_are_available(pp::tests::Harness& harness) { const auto open = pp::app::plan_main_toolbar_command(pp::app::MainToolbarCommand::open_document); @@ -101,6 +134,49 @@ void rejects_negative_history_metrics(pp::tests::Harness& harness) PP_EXPECT(harness, !pp::app::plan_main_toolbar_command(pp::app::MainToolbarCommand::clear_history, 0, 0, -1)); } +void executor_dispatches_to_service_boundary(pp::tests::Harness& harness) +{ + FakeMainToolbarServices services; + + auto open = pp::app::plan_main_toolbar_command(pp::app::MainToolbarCommand::open_document); + PP_EXPECT(harness, open); + if (open) { + const auto status = pp::app::execute_main_toolbar_plan(open.value(), services); + PP_EXPECT(harness, status.ok()); + PP_EXPECT(harness, services.open_dialogs == 1); + } + + auto undo = pp::app::plan_main_toolbar_command(pp::app::MainToolbarCommand::undo, 1); + PP_EXPECT(harness, undo); + if (undo) { + const auto status = pp::app::execute_main_toolbar_plan(undo.value(), services); + PP_EXPECT(harness, status.ok()); + PP_EXPECT(harness, services.undo_calls == 1); + } + + auto clear_canvas = pp::app::plan_main_toolbar_command( + pp::app::MainToolbarCommand::clear_canvas, + 0, + 0, + 0, + true); + PP_EXPECT(harness, clear_canvas); + if (clear_canvas) { + const auto status = pp::app::execute_main_toolbar_plan(clear_canvas.value(), services); + PP_EXPECT(harness, status.ok()); + PP_EXPECT(harness, services.clear_canvas_calls == 1); + } + + auto redo_empty = pp::app::plan_main_toolbar_command(pp::app::MainToolbarCommand::redo); + PP_EXPECT(harness, redo_empty); + if (redo_empty) { + const int calls_before = services.total_calls(); + const auto status = pp::app::execute_main_toolbar_plan(redo_empty.value(), services); + PP_EXPECT(harness, status.ok()); + PP_EXPECT(harness, services.total_calls() == calls_before); + } +} + } // namespace int main() @@ -110,5 +186,6 @@ int main() harness.run("history commands reuse history breakpoints", history_commands_reuse_history_breakpoints); harness.run("canvas clear requires live canvas", canvas_clear_requires_live_canvas); harness.run("rejects negative history metrics", rejects_negative_history_metrics); + harness.run("executor dispatches to service boundary", executor_dispatches_to_service_boundary); return harness.finish(); }