ammachado commented on PR #24365: URL: https://github.com/apache/camel/pull/24365#issuecomment-4859727463
## Comparison with #24351 Both PRs implement **CAMEL-23831: mouse support for the `camel-tui` monitor**, developed independently and in parallel. ### At a glance | | **#24365** (this PR) | **#24351** (@gnodet) | |---|---|---| | Files changed | 29 | 44 | | Additions / deletions | **+1,484 / −7** | **+1,859 / −976** | | New files | 2 (both tests) | 4 (`AbstractTab`, `AbstractTableTab`, `DragSplit`, `PanelAnimation`) | | Net footprint | Purely additive | Add + heavy refactor | | Commits | 10, `CAMEL-23831:` prefixed, granular per-feature | 6, `chore:` prefixed, refactor-oriented | ### Different core architecture - **#24365 — central dispatch, widgets stay pure renderers.** Everything routes through a single `MouseEvent` branch in `CamelMonitor`. Widgets capture their geometry at render time and expose static hit-testing helpers (`tableRowAt`, `tabIndexAt`, `morePopupItemAt`, `listItemAt`, `footerKeyEvent`, `footerRegionAt`) that the app calls at dispatch time. The `−7` deletions confirm it only adds. - **#24351 — inheritance refactor.** Introduces `AbstractTab` (+107) and `AbstractTableTab` (+142) base classes, then pulls shared table logic up into them. This is why the table tabs show large deletions (`BeansTab −61`, `ConsumersTab −58`, `DataSourceTab −58`, `InflightTab −68`, `HeapHistogramTab −77`): duplicated code is centralized. Mouse behavior lives in the base classes via `renderTableScrollbar()` and `nearestPresetIndex()`. ### Different feature scope - **#24351 does more breadth:** drag-to-resize for shell/AI panels (`DragSplit`, `PanelAnimation`, `ShellPanel`, `AiPanel`), diagram/EIP node clicks (`DiagramSupport`, `DiagramTab`), vertical scrollbars on every table tab, and extra tabs (`ClasspathTab`, `ConfigurationTab`, `DataSourceTab`, `SqlQueryTab`, `SqlTraceTab`, `StartupTab`, `MemoryTab`, `HeapHistogramTab`, `ProcessTab`). - **#24365 covers areas #24351 does not:** clickable footer key bindings (clicking `F1`/`F2`/`Enter` synthesizes the keystroke), a clickable Actions popup / doc picker (`ActionsPopup +81`), and Overview integration-list click-to-select. It also carries one unrelated fix (`PluginAdd` confirmation message). ### Testing - **#24365:** 2 new test files + 6 extended (`MonitorTabTest +223`, `ActionsPopupTest +70`, render tests). Static hit-testing helpers unit-tested directly. 514+ tests green. - **#24351:** 18 tests in `MonitorTabMouseTest` covering `nearestPresetIndex`, `handleTableClick`, `setSplitPercent`. 493 tests green. ### Overlap / conflict The two PRs collide on ~19 files, including the hardest to reconcile (`CamelMonitor`, `MonitorTab`), every table tab, `PopupManager`, `HistoryTab`, `OverviewTab`, and `OverviewTabRenderTest`. Because #24351 *restructures* the class hierarchy that #24365 *builds on top of*, they cannot merge cleanly together. ### Recommendation Let **#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: - Clickable footer key bindings - Clickable Actions popup / doc picker - Overview integration-list click-to-select - The unrelated `PluginAdd` confirmation fix On rebase, those would re-target the new `AbstractTab` / `AbstractTableTab` hit-testing surface instead of the current standalone static helpers. --- _Comparison prepared by Claude Code (Anthropic) on behalf of Adriano Machado._ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
