CAMEL-23720: Add emoji icons to camel-jbang TUI tab headers and centralize glyphs#24412
CAMEL-23720: Add emoji icons to camel-jbang TUI tab headers and centralize glyphs#24412ammachado wants to merge 2 commits into
Conversation
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 2 tested, 1 compile-only — current: 2 all testedMaveniverse Scalpel detected 3 affected modules (current approach: 2).
|
davsclaus
left a comment
There was a problem hiding this comment.
Review: CAMEL-23720 — Add emoji icons to TUI tab headers and centralize glyphs
Well-structured refactoring across 5 commits. TuiIcons as single source of truth, MoreTab record consolidation, and mnemonic-derived shortcuts are all sound. Excellent test coverage (icon width, VS16 absence, shortcut uniqueness, historical sequence oracle, Browse/Configuration regression guard).
Minor suggestions (non-blocking)
-
TuiHelper.fileEmoji()/fileEmojiByName()still return raw emoji literals —FilesBrowser.fileType()now switches onTuiIconsconstants, but the methods producing the values it compares against haven't been migrated. Works because the values are identical, but contradicts the "single source of truth" principle this PR establishes. Good follow-up candidate. -
VS16 in some constants —
DEV_PROFILE = "🛠️"andWARN = "⚠️"contain VS16 (U+FE0F), while the class Javadoc and CAMEL-23818 reference say to avoid it. TheprimaryTabEmojisHaveNoVariationSelectortest only checksPRIMARY_TAB_ICONS. These were pre-existing values; consider cleaning them up or documenting as exceptions. -
Icon alias naming —
TuiIcons.OTEL(🔗) is reused for MCP connection doctor checks (not OTel-related), andTAB_STARTUPreusesQUARKUS(🚀) though Startup isn't Quarkus-specific. Naming nitpicks, not bugs.
None of these are blocking. Nice contribution!
This review does not replace specialized tools such as CodeRabbit, Sourcery, or SonarCloud.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
|
|
||
| // ---- Brand & runtime ---- | ||
| static final String CAMEL = "🐪"; | ||
| static final String SPRING_BOOT = "🍃"; |
There was a problem hiding this comment.
Minor: DEV_PROFILE = "🛠️" and WARN = "⚠️" (lines 35, 39) contain VS16 variation selectors, while the class Javadoc says to use "plain 2-column-wide emoji without VS16". The primaryTabEmojisHaveNoVariationSelector test only checks PRIMARY_TAB_ICONS. These were pre-existing values being centralized — consider either stripping VS16 or adding a doc note that these are intentional exceptions.
| @@ -117,7 +117,7 @@ private boolean loadDirectory(Path dir) { | |||
|
|
|||
There was a problem hiding this comment.
The switch in fileType() (line 315) now matches against TuiIcons.CAMEL, TuiIcons.JAVA, etc., but TuiHelper.fileEmoji() which produces the values being compared still returns raw emoji literals. Works because the values are identical, but a follow-up migrating TuiHelper.fileEmoji()/fileEmojiByName() to use TuiIcons would complete the centralization.
Introduce TuiIcons as the single source of truth for emoji and UI symbols across the camel-jbang TUI: tab bar, More submenu, footer navigation hints, and the F2 Actions/Go-to popups. Consolidate the More submenu into a MoreTab record with & mnemonics, add a compact tab-bar threshold, and strengthen TUI icon tests (TuiIconsTest, tab bar render tests) validating icon width constraints and MoreTab invariants. Co-authored-by: Cursor <cursoragent@cursor.com>
Migrate raw emoji literals that lived in popup/browser classes to the TuiIcons registry, completing the single-source-of-truth goal: - ExampleBrowserPopup: bundled/online/Docker/infra/Citrus icons and legend - InfraBrowserPopup: infra service row prefix - FolderBrowser: directory icon - DoctorPopup: MCP connection check now uses TuiIcons.MCP instead of reusing TuiIcons.OTEL (which stays for OTel Spans) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Description
Adds emoji icons to the
camel-jbang-plugin-tuimonitor and centralizes all glyph handling:TuiIconsas the single source of truth for emoji and UI symbols. Add icons to every primary tab header, the More submenu, and the Go to… popup; replace hardcoded footer navigation hints (↑↓,↑↓←→, etc.) withTuiIconsconstants.TabRegistry.MoreTabrecord (icon, name, label, tab), replacing the fragile parallel arrays and theMORE_TAB_KEY_INDEXoffset table.PopupManager,McpFacade, andActionsPopupnow read from this one list.&mnemonic markers embedded in each label, so the underline offset and the trigger key can no longer drift apart. This fixes the Browse/Configuration mis-highlight where the wrong letter was underlined. Mnemonic triggers are case-insensitive (wandWboth fire).TABS_FULL_MIN_WIDTH(157) now only selects tight vs spaced dividers.PROD_PROFILEis 📦 (matching the currentmainglyph), not the padlock 🔒.Hardening of the
MoreTabmodel:MoreTabthat validates the&mnemonic marker, so a malformed label fails loudly at construction with the offending label instead of throwingStringIndexOutOfBoundsfromshortcut()during the eagerallTabEntries()call at startup.selectMoreTab(12)) with a name lookup via a newmoreTabIndex("SQL Query"), removing the final piece of index coupling the refactor left behind.PRIMARY_TAB_ICONSan unmodifiableList(was a mutable array) and dropTuiIconsto package-private to match its members.Remaining glyph centralization (addresses review feedback):
TuiIcons:ExampleBrowserPopup(bundled/online/Docker/infra/Citrus icons and legend),InfraBrowserPopup(service row prefix), andFolderBrowser(directory icon).DoctorPopupMCP connection check now usesTuiIcons.MCPinstead of reusingTuiIcons.OTEL(which stays for OTel Spans).Tests:
TuiIconsTest,TabBarRenderTest, and a registry-backedTabRegistryTestguard icon width, mnemonic/shortcut derivation, the Browse/Configuration regression, Route/Top width parity, and case-insensitive triggers. A tautological shortcut test (it re-derivedshortcut(), so it could not fail) is replaced with a literal historical-sequence oracle (B,W,C,A,G,N,D,H,I,M,K,E,Q,R,O,P,S,T), with added coverage for shortcut uniqueness, the constructor validation,moreTabIndex,allTabEntries, andPRIMARY_TAB_ICONSordering. Thecamel-jbang-plugin-tuimodule test suite passes (the twoThemeTest/OverviewTabRenderTestflakes are pre-existing onmain, caused by order-dependentThemestatic state).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.AI-generated PR description on behalf of Adriano Machado, via Claude Code.