CAMEL-23831: Add footer and Actions popup mouse support to camel-tui#24365
Conversation
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
davsclaus
left a comment
There was a problem hiding this comment.
CAMEL-23831: Add mouse support to camel-tui
Well-structured PR that adds mouse support (click-to-select, scroll wheel, popup interaction) to the TUI monitor. The architecture is clean: widgets stay pure renderers capturing geometry at render time, and CamelMonitor hit-tests at dispatch time. Commits are logically split into focused changes.
Observations (non-blocking)
-
Unrelated
PluginAdd.javachange — Theprinter().printf("Plugin %s added%n", name)confirmation message is unrelated to mouse support. The PR description acknowledges it. Minor scope mixing but trivial. -
MORE_ITEM_COUNT = 17hardcoded constant — Correct today (matches the 17ListItem.from()entries). Will silently go stale when someone adds a new dropdown entry. Already an improvement over the prior bare17inselectNext(17), since the constant is named and scoped. -
Minor blank line removals — A few files have cosmetic blank line removals between fields (
BrowseTab,HttpTab,ThreadsTab,HistoryTab,SpansTab). Harmless but unnecessary churn.
Checklist
- JIRA ticket linked (CAMEL-23831)
- Commit format follows conventions
- Tests for new functionality (2 new test classes + additions to 5 existing ones)
- AI attribution declared
- No new dependencies
- JUnit 5, no Thread.sleep(), no Lombok, no FQCNs
- Targeting correct branch (main)
CI builds are still pending.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
|
No, I haven't seen it. I'm working on this since the weekend. Let's keep this one on hold, and I rebase it once @gnodet's PR merge. |
Comparison with #24351Both PRs implement CAMEL-23831: mouse support for the At a glance
Different core architecture
Different feature scope
Testing
Overlap / conflictThe two PRs collide on ~19 files, including the hardest to reconcile ( RecommendationLet #24351 merge first (it does the deeper refactor and wider tab coverage; rebasing additive work onto it is far easier than the reverse). This PR then becomes a follow-up for the features #24351 lacks:
On rebase, those would re-target the new Comparison prepared by Claude Code (Anthropic) on behalf of Adriano Machado. |
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 2 tested, 1 compile-only — current: 3 all testedMaveniverse Scalpel detected 3 affected modules (current approach: 3). Skip-tests mode would test 2 modules (1 direct + 1 downstream), skip tests for 1 (generated code, meta-modules) Modules Scalpel would test (2)
Modules with tests skipped (1)
All tested modules (3 modules)
|
|
Thanks for the review, @davsclaus. Follow-up in
About the first observation (the Full Claude Code (Anthropic) on behalf of Adriano Machado |
… clickable Follow-up to apache#24351 (merged), which already added mouse wheel scrolling, table click-to-select, tab-bar clicks and the More/Switch dropdown mouse handling. This adds the two interactions apache#24351 did not cover, re-fitted onto its architecture. - Footer key bindings: renderFooter captures the on-screen column range of every clickable hint (a Theme.hintKey()-styled key span plus its label span) after the overflow-drop logic runs. handleMouseEvent routes a left click on the footer row through handleFooterClick, which hit-tests the click and feeds the synthesized KeyEvent back through the normal key path so it behaves exactly like the key press. footerKeyEvent maps a hint token to a KeyEvent for unambiguous single keys only (F1-F12, Enter, Esc, Tab, single characters); ambiguous multi-key hints (Up/Down, PgUp/PgDn, arrow glyphs) stay non-clickable. Overlay states (screenshot flash, help, caption) leave the footer non-clickable. - Actions popup and doc picker: ActionsPopup.handleMouseEvent handles the F2 Actions menu and the doc picker with the same modal approach as the More dropdown. A click on an entry selects and activates it via a synthetic Enter, a click outside goes back one level via a synthetic Esc, and the wheel moves the highlight. Divider rows are ignored. Other sub-popups stay modal. CamelMonitor now routes mouse events to ActionsPopup when it is visible instead of dropping them. Tests: CamelMonitorTest covers footerKeyEvent parsing and footerRegionAt geometry; ActionsPopupTest covers listItemAt geometry including scroll offset. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
apache#24351 added click-to-select on the Overview table, but a click on the "Dev/Infra Services" divider row moved the visual selection onto it (the selected pid was left unchanged). Restore the prior selection when the divider row is clicked so it stays non-selectable, matching keyboard navigation which already skips it. Tests: OverviewTabRenderTest covers clicking an integration row (selection change plus pid-changed callback) and clicking the divider row (no change). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
davsclaus
left a comment
There was a problem hiding this comment.
Review: CAMEL-23831 — Footer and Actions popup mouse support
CI Status: All checks pass.
Clean, well-scoped follow-up to #24351. Three features (footer clicks, Actions popup mouse, Overview divider fix) are logically coherent and consistent with the established mouse handling architecture.
Highlights
- Re-entrancy is safe —
handleFooterClickcallshandleEvent(KeyEvent)from insidehandleEvent(MouseEvent). The synthesized event goes through a separate dispatch branch; no recursion risk. - Footer invalidation is correct —
renderFooterresetsfooterRowY = -1before any early-return path (screenshot flash, help overlay, caption), so overlay states leave the footer non-clickable. - Divider fix mirrors keyboard behavior — The Overview tab already skips the divider in
navigateUp/navigateDown; the mouse click now does the same by restoring the prior selection. - Good separation — ActionsPopup gets mouse routing while FilesBrowser stays keyboard-only (variable-height rows prevent reliable hit-testing), matching the PR description.
- Test coverage — All new static helpers (
footerKeyEvent,footerRegionAt,listItemAt) are unit-tested. OverviewTab render tests cover both happy path and divider edge case. - Conventions — Commits follow
CAMEL-XXXX:format with detailed bodies andCo-Authored-Bytrailers.
No issues found. LGTM.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Description
Follow-up to #24351 (merged), which added the bulk of mouse support to the
camel-tuimonitor (camel-jbang-plugin-tui): mouse-wheel scrolling,click-to-select on table tabs, tab-bar clicks, the More/Switch dropdown mouse
handling, scrollbar drag, panel-border resize and node selection in diagram views.
This PR adds the two pointer interactions #24351 did not cover, re-fitted onto its
architecture:
F1/help,F2/actions,Enter/open or a single-letter action (d,f,p,r,x, ...) fires thatkey.
renderFootercaptures the on-screen column range of every clickable hint(the
Theme.hintKey()-styled key span plus its label span) after theoverflow-drop logic runs, so clicking either the key or its label works.
handleMouseEventroutes a left click on the footer row throughhandleFooterClick, which hit-tests the click and feeds the synthesizedKeyEventback through the normal key path, so it behaves exactly like the keypress (including popups and tab switching).
footerKeyEventmaps a hint tokento a
KeyEventfor unambiguous single keys only (F1-F12, Enter, Esc, Tab, singlecharacters); ambiguous multi-key hints (
Up/Down,PgUp/PgDn, arrow glyphs) staynon-clickable. Overlay states (screenshot flash, help, caption) leave the footer
non-clickable.
ActionsPopup.handleMouseEventhandles the F2Actions menu and the doc picker with the same modal approach Improve mouse support in TUI: scroll, tab clicks, node selection, panel resize #24351 used for the
More dropdown: a click on an entry selects and activates it via a synthetic
Enter, a click outside goes back one level via a syntheticEsc, and the wheelmoves the highlight. Divider rows are ignored.
CamelMonitornow routes mouseevents to
ActionsPopupwhen it is visible instead of dropping them. The exampleand infra browsers stay keyboard-only for now (their wrapped, variable-height rows
cannot be reliably hit-tested).
a click on the "Dev/Infra Services" divider row moved the visual selection onto it
while leaving the selected pid unchanged. The prior selection is now restored when
the divider row is clicked, keeping it non-selectable and matching keyboard
navigation which already skips it.
Static hit-testing helpers (
footerKeyEvent,footerRegionAt,listItemAt) areunit-tested directly; render tests cover the footer click-to-key path, the Actions
popup entry selection, and the Overview divider-vs-row click behavior.
Target
mainbranch)Tracking
Apache Camel coding standards and style
mvn clean install -DskipTestslocally from root folder and I have committed all auto-generated changes.AI-assisted contributions
Co-authored-bytrailers) and the PR description identifies the AI tool used.This PR was prepared by Claude Code (Anthropic) on behalf of Adriano Machado. Commits carry
Co-Authored-Bytrailers.