added compat actions to navigation controller

[igorkorsukov]

No description provided.

[coderabbitai]

📝 Walkthrough

Walkthrough

NavigationController now implements actions::Actionable, receives an actions dispatcher, and registers legacy navigation action strings that dispatch mapped commands through the main dispatcher. UI tests link and initialize the actions module, construct an ActionsDispatcher, and inject it into the controller fixture.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is missing; the required issue reference, summary, checklist items, and build configuration details are not provided.	Add the template sections: issue reference, short summary/motivation, completed checklist items, and any relevant build/test notes.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately summarizes the main change: adding compat actions to the navigation controller.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@framework/ui/internal/navigationcontroller.cpp`:
- Around line 325-329: Set the dispatcher immediately after obtaining it in the
NavigationController initialization flow, before manually registering actions,
so Actionable can unregister them during destruction. Specifically update
framework/ui/internal/navigationcontroller.cpp lines 325-329 around
actionsDispatcher() and reg(); framework/ui/internal/navigationcontroller.h
lines 41-45 only documents the Actionable inheritance and requires no direct
change.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: cfb98277-f314-4e2b-b6ac-91cc0620a6d6

📥 Commits

Reviewing files that changed from the base of the PR and between aee9f67 and 8ce151e.

📒 Files selected for processing (5)

• framework/ui/internal/navigationcontroller.cpp
• framework/ui/internal/navigationcontroller.h
• framework/ui/tests/CMakeLists.txt
• framework/ui/tests/environment.cpp
• framework/ui/tests/navigationcontroller_tests.cpp

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Ensure Actionable::setDispatcher is called to guarantee cleanup.

The NavigationController inherits from actions::Actionable, which relies on its m_dispatcher member to automatically unregister actions in ~Actionable(). Since you are manually registering actions with ad->reg(...), you need to explicitly set the dispatcher so the base class can properly clean up these registrations when the controller is destroyed.

• framework/ui/internal/navigationcontroller.cpp#L325-L329: Call setDispatcher(ad); right after retrieving the dispatcher to ensure the base class has the pointer for cleanup.
• framework/ui/internal/navigationcontroller.h#L41-L45: NavigationController inherits actions::Actionable here, inherently taking on this lifecycle requirement.

🛠️ Proposed fix

         auto ad = actionsDispatcher();
+        setDispatcher(ad);
         for (const auto& [action, command] : compatActionToCommand) {
             ad->reg(this, action, [d, command]() { d->dispatch(command); });
         }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	auto ad = actionsDispatcher();
	for (const auto& [action, command] : compatActionToCommand) {
	ad->reg(this, action, [d, command]() { d->dispatch(command); });
	}
	}
	auto ad = actionsDispatcher();
	setDispatcher(ad);
	for (const auto& [action, command] : compatActionToCommand) {
	ad->reg(this, action, [d, command]() { d->dispatch(command); });
	}
	}

🧰 Tools

🪛 Clang (14.0.6)

[warning] 325-325: variable name 'ad' is too short, expected at least 3 characters

(readability-identifier-length)

📍 Affects 2 files

• framework/ui/internal/navigationcontroller.cpp#L325-L329 (this comment)
• framework/ui/internal/navigationcontroller.h#L41-L45

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@framework/ui/internal/navigationcontroller.cpp` around lines 325 - 329, Set
the dispatcher immediately after obtaining it in the NavigationController
initialization flow, before manually registering actions, so Actionable can
unregister them during destruction. Specifically update
framework/ui/internal/navigationcontroller.cpp lines 325-329 around
actionsDispatcher() and reg(); framework/ui/internal/navigationcontroller.h
lines 41-45 only documents the Actionable inheritance and requires no direct
change.