diff --git a/docs/modernization/debt.md b/docs/modernization/debt.md index 2bd93b8..6671cac 100644 --- a/docs/modernization/debt.md +++ b/docs/modernization/debt.md @@ -51,7 +51,7 @@ agent or engineer to remove them without reconstructing context from chat. | DEBT-0031 | Open | Modernization | Top-level File menu command planning now consumes pure `pp_app_core` through `App::init_menu_file` and `pano_cli plan-file-menu`, but live execution still invokes legacy dialogs, platform pickers, cloud code, share code, and canvas import/export paths directly | Preserve File menu behavior while app workflows move toward app/document/platform command services | `pp_app_core_file_menu_tests`; `pano_cli plan-file-menu --command save-as`; `pano_cli plan-file-menu --command import`; `pano_cli plan-file-menu --command cloud-upload`; `ctest --preset desktop-fast --build-config Debug` | File menu routing, picker dispatch, save/share/cloud/resize/export execution, and image/project import execution are owned by app/document/platform services with `App::init_menu_file` acting only as a UI adapter | | 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-0034 | Open | Modernization | About menu command planning and execution dispatch now consume pure `pp_app_core` through `App::init_menu_about`, `pano_cli plan-about-menu`, and the `AboutMenuServices` boundary, but the live adapter 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 injected app/UI/platform services with `App::init_menu_about` acting only as a UI adapter and no legacy About 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 2fa57cd..7f3431f 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -555,8 +555,9 @@ grid panel by mistake. `pano_cli plan-about-menu` exposes app-core planning for About menu help, about, what's-new, crash-test, and performance-test commands, including versioned what's-new labels, diagnostic gating, and no-canvas performance-test -blocking before legacy dialogs, platform crash hooks, and Canvas performance -strokes continue. +blocking. `pp_app_core` now also owns an `AboutMenuServices` executor boundary, +so `App::init_menu_about` dispatches through a legacy adapter before legacy +dialogs, platform crash hooks, and Canvas performance strokes continue. `pp_platform_api` now owns a headless `PlatformServices` interface for startup storage path preparation, clipboard text, cursor visibility, virtual-keyboard visibility, UI-thread lifecycle hooks, render-context @@ -1287,7 +1288,8 @@ Results: menu/panel planning as JSON automation. - `pp_app_core_about_menu_tests` passed, covering About/help/what's-new dialog routing, versioned what's-new labels, crash diagnostic gating, performance - workload metadata, and no-canvas performance-test blocking. + workload metadata, no-canvas performance-test blocking, dispatch through the + `AboutMenuServices` executor boundary, and no-op unavailable actions. - `pano_cli_plan_about_menu_news_smoke`, `pano_cli_plan_about_menu_performance_no_canvas_smoke`, `pano_cli_plan_about_menu_crash_disabled_smoke`, and diff --git a/src/app_core/about_menu.h b/src/app_core/about_menu.h index afe76c5..60a6c38 100644 --- a/src/app_core/about_menu.h +++ b/src/app_core/about_menu.h @@ -1,5 +1,7 @@ #pragma once +#include "foundation/result.h" + #include namespace pp::app { @@ -31,6 +33,17 @@ struct AboutMenuPlan { int performance_updates_per_iteration = 0; }; +class AboutMenuServices { +public: + virtual ~AboutMenuServices() = default; + + virtual void show_user_manual() = 0; + virtual void show_about_dialog() = 0; + virtual void show_whats_new_dialog() = 0; + virtual void trigger_crash_test() = 0; + virtual void run_performance_test(const AboutMenuPlan& plan) = 0; +}; + [[nodiscard]] inline AboutMenuPlan plan_about_menu_command( AboutMenuCommand command, int version_major, @@ -83,4 +96,31 @@ struct AboutMenuPlan { return plan; } +[[nodiscard]] inline pp::foundation::Status execute_about_menu_plan( + const AboutMenuPlan& plan, + AboutMenuServices& services) +{ + switch (plan.action) { + case AboutMenuAction::show_user_manual: + services.show_user_manual(); + return pp::foundation::Status::success(); + case AboutMenuAction::show_about_dialog: + services.show_about_dialog(); + return pp::foundation::Status::success(); + case AboutMenuAction::show_whats_new_dialog: + services.show_whats_new_dialog(); + return pp::foundation::Status::success(); + case AboutMenuAction::trigger_crash_test: + services.trigger_crash_test(); + return pp::foundation::Status::success(); + case AboutMenuAction::run_performance_test: + services.run_performance_test(plan); + return pp::foundation::Status::success(); + case AboutMenuAction::no_op_unavailable: + return pp::foundation::Status::success(); + } + + return pp::foundation::Status::invalid_argument("unknown about menu action"); +} + } diff --git a/src/app_layout.cpp b/src/app_layout.cpp index fec5ca2..e703cad 100644 --- a/src/app_layout.cpp +++ b/src/app_layout.cpp @@ -309,6 +309,73 @@ private: App& app_; }; +class LegacyAboutMenuServices final : public pp::app::AboutMenuServices { +public: + explicit LegacyAboutMenuServices(App& app) noexcept + : app_(app) + { + } + + void show_user_manual() override + { + app_.dialog_usermanual(); + } + + void show_about_dialog() override + { + app_.dialog_about(); + } + + void show_whats_new_dialog() override + { + app_.dialog_whatsnew(true); + } + + void trigger_crash_test() override + { + LOG("crashing"); + app_.crash_test(); + } + + void run_performance_test(const pp::app::AboutMenuPlan& plan) override + { + if (!Canvas::I) + return; + + LOG("perf"); + std::string message; + const int performance_iterations = plan.performance_iterations; + app_.render_task([&] + { + auto start = std::chrono::high_resolution_clock::now(); + Canvas::I->stroke_start({ 0, 0, 0 }, 0.9f); + for (int i = 0; i < performance_iterations; i++) + { + Canvas::I->stroke_update({ 100, 100, 0 }, 0.9f); + Canvas::I->stroke_update({ 200, 200, 0 }, 0.9f); + Canvas::I->stroke_update({ 200, 100, 0 }, 0.9f); + Canvas::I->stroke_update({ 100, 200, 0 }, 0.9f); + Canvas::I->stroke_update({ 300, 300, 0 }, 0.9f); + Canvas::I->stroke_update({ 200, 500, 0 }, 0.9f); + Canvas::I->stroke_update({ 500, 500, 0 }, 0.9f); + Canvas::I->stroke_update({ 400, 400, 0 }, 0.9f); + Canvas::I->stroke_update({ 0, 200, 0 }, 0.9f); + Canvas::I->stroke_update({ 200, 0, 0 }, 0.9f); + Canvas::I->stroke_draw(); + } + Canvas::I->stroke_end(); + auto diff = std::chrono::high_resolution_clock::now() - start; + auto ms = std::chrono::duration_cast(diff).count(); + LOG("%lld ms", ms); + message = "Time " + std::to_string(ms) + " ms"; + }); + app_.message_box("Performance test", message); + } + +private: + App& app_; +}; + void execute_main_toolbar_plan(App& app, const pp::app::MainToolbarPlan& plan) { LegacyMainToolbarServices services(app); @@ -317,6 +384,14 @@ void execute_main_toolbar_plan(App& app, const pp::app::MainToolbarPlan& plan) LOG("Main toolbar action failed: %s", status.message); } +void execute_about_menu_plan(App& app, const pp::app::AboutMenuPlan& plan) +{ + LegacyAboutMenuServices services(app); + const auto status = pp::app::execute_about_menu_plan(plan, services); + if (!status.ok()) + LOG("About menu action failed: %s", status.message); +} + } // namespace void App::title_update() @@ -1583,8 +1658,7 @@ void App::init_menu_about() g_version_major, g_version_minor, g_version_fix); - if (plan.action == pp::app::AboutMenuAction::show_about_dialog) - dialog_about(); + execute_about_menu_plan(*this, plan); if (plan.closes_root_popup) { popup->mouse_release(); @@ -1600,8 +1674,7 @@ void App::init_menu_about() g_version_major, g_version_minor, g_version_fix); - if (plan.action == pp::app::AboutMenuAction::show_user_manual) - dialog_usermanual(); + execute_about_menu_plan(*this, plan); if (plan.closes_root_popup) { popup->mouse_release(); @@ -1626,8 +1699,7 @@ void App::init_menu_about() g_version_major, g_version_minor, g_version_fix); - if (plan.action == pp::app::AboutMenuAction::show_whats_new_dialog) - dialog_whatsnew(true); + execute_about_menu_plan(*this, plan); if (plan.closes_root_popup) { popup->mouse_release(); @@ -1644,11 +1716,7 @@ void App::init_menu_about() g_version_major, g_version_minor, g_version_fix); - if (plan.action == pp::app::AboutMenuAction::trigger_crash_test) - { - LOG("crashing"); - crash_test(); - } + execute_about_menu_plan(*this, plan); if (plan.closes_root_popup) { popup->mouse_release(); @@ -1667,37 +1735,7 @@ void App::init_menu_about() g_version_fix, true, Canvas::I != nullptr); - if (plan.action == pp::app::AboutMenuAction::run_performance_test) - { - LOG("perf"); - std::string message; - const int performance_iterations = plan.performance_iterations; - render_task([&] - { - auto start = std::chrono::high_resolution_clock::now(); - Canvas::I->stroke_start({ 0, 0, 0 }, 0.9f); - for (int i = 0; i < performance_iterations; i++) - { - Canvas::I->stroke_update({ 100, 100, 0 }, 0.9f); - Canvas::I->stroke_update({ 200, 200, 0 }, 0.9f); - Canvas::I->stroke_update({ 200, 100, 0 }, 0.9f); - Canvas::I->stroke_update({ 100, 200, 0 }, 0.9f); - Canvas::I->stroke_update({ 300, 300, 0 }, 0.9f); - Canvas::I->stroke_update({ 200, 500, 0 }, 0.9f); - Canvas::I->stroke_update({ 500, 500, 0 }, 0.9f); - Canvas::I->stroke_update({ 400, 400, 0 }, 0.9f); - Canvas::I->stroke_update({ 0, 200, 0 }, 0.9f); - Canvas::I->stroke_update({ 200, 0, 0 }, 0.9f); - Canvas::I->stroke_draw(); - } - Canvas::I->stroke_end(); - auto diff = std::chrono::high_resolution_clock::now() - start; - auto ms = std::chrono::duration_cast(diff).count(); - LOG("%lld ms", ms); - message = "Time " + std::to_string(ms) + " ms"; - }); - message_box("Performance test", message); - } + execute_about_menu_plan(*this, plan); if (plan.closes_root_popup) { popup->mouse_release(); diff --git a/tests/app_core/about_menu_tests.cpp b/tests/app_core/about_menu_tests.cpp index 583ab11..a8ec159 100644 --- a/tests/app_core/about_menu_tests.cpp +++ b/tests/app_core/about_menu_tests.cpp @@ -3,6 +3,33 @@ namespace { +class FakeAboutMenuServices final : public pp::app::AboutMenuServices { +public: + void show_user_manual() override { manuals += 1; } + void show_about_dialog() override { about_dialogs += 1; } + void show_whats_new_dialog() override { whats_new_dialogs += 1; } + void trigger_crash_test() override { crash_tests += 1; } + void run_performance_test(const pp::app::AboutMenuPlan& plan) override + { + performance_tests += 1; + performance_iterations = plan.performance_iterations; + performance_updates_per_iteration = plan.performance_updates_per_iteration; + } + + [[nodiscard]] int total_calls() const noexcept + { + return manuals + about_dialogs + whats_new_dialogs + crash_tests + performance_tests; + } + + int manuals = 0; + int about_dialogs = 0; + int whats_new_dialogs = 0; + int crash_tests = 0; + int performance_tests = 0; + int performance_iterations = 0; + int performance_updates_per_iteration = 0; +}; + void about_menu_maps_user_facing_dialog_actions(pp::tests::Harness& harness) { const auto help = pp::app::plan_about_menu_command(pp::app::AboutMenuCommand::help_guide, 1, 2, 3); @@ -67,6 +94,70 @@ void about_menu_performance_test_declares_workload_and_canvas_requirement(pp::te PP_EXPECT(harness, !no_canvas.closes_root_popup); } +void about_menu_executor_dispatches_to_services(pp::tests::Harness& harness) +{ + FakeAboutMenuServices services; + + const auto help = pp::app::plan_about_menu_command(pp::app::AboutMenuCommand::help_guide, 1, 2, 3); + PP_EXPECT(harness, pp::app::execute_about_menu_plan(help, services).ok()); + PP_EXPECT(harness, services.manuals == 1); + + const auto about = pp::app::plan_about_menu_command(pp::app::AboutMenuCommand::about_app, 1, 2, 3); + PP_EXPECT(harness, pp::app::execute_about_menu_plan(about, services).ok()); + PP_EXPECT(harness, services.about_dialogs == 1); + + const auto news = pp::app::plan_about_menu_command(pp::app::AboutMenuCommand::whats_new, 1, 2, 3); + PP_EXPECT(harness, pp::app::execute_about_menu_plan(news, services).ok()); + PP_EXPECT(harness, services.whats_new_dialogs == 1); + + const auto crash = pp::app::plan_about_menu_command( + pp::app::AboutMenuCommand::induce_crash, + 1, + 2, + 3, + true); + PP_EXPECT(harness, pp::app::execute_about_menu_plan(crash, services).ok()); + PP_EXPECT(harness, services.crash_tests == 1); + + const auto performance = pp::app::plan_about_menu_command( + pp::app::AboutMenuCommand::performance_test, + 1, + 2, + 3, + true, + true); + PP_EXPECT(harness, pp::app::execute_about_menu_plan(performance, services).ok()); + PP_EXPECT(harness, services.performance_tests == 1); + PP_EXPECT(harness, services.performance_iterations == 100); + PP_EXPECT(harness, services.performance_updates_per_iteration == 10); +} + +void about_menu_executor_preserves_no_op_unavailable(pp::tests::Harness& harness) +{ + FakeAboutMenuServices services; + + const auto disabled_crash = pp::app::plan_about_menu_command( + pp::app::AboutMenuCommand::induce_crash, + 1, + 2, + 3, + false); + PP_EXPECT(harness, disabled_crash.action == pp::app::AboutMenuAction::no_op_unavailable); + PP_EXPECT(harness, pp::app::execute_about_menu_plan(disabled_crash, services).ok()); + PP_EXPECT(harness, services.total_calls() == 0); + + const auto no_canvas_perf = pp::app::plan_about_menu_command( + pp::app::AboutMenuCommand::performance_test, + 1, + 2, + 3, + true, + false); + PP_EXPECT(harness, no_canvas_perf.action == pp::app::AboutMenuAction::no_op_unavailable); + PP_EXPECT(harness, pp::app::execute_about_menu_plan(no_canvas_perf, services).ok()); + PP_EXPECT(harness, services.total_calls() == 0); +} + } int main() @@ -77,5 +168,7 @@ int main() harness.run( "about menu performance test declares workload and canvas requirement", about_menu_performance_test_declares_workload_and_canvas_requirement); + harness.run("about menu executor dispatches to services", about_menu_executor_dispatches_to_services); + harness.run("about menu executor preserves no op unavailable", about_menu_executor_preserves_no_op_unavailable); return harness.finish(); }