[jussielo-amd]

Why

ROCMAI-817: after the dash-tui module split (#827) moved the popup geometry and the menu data out of crates/rocm-dash-tui/src/ui/modal.rs, the file still held the renderers for several unrelated modals (Help, global help, warnings, theme picker, Esc menu, command palette, Options) plus about 600 lines of their tests in one 1,483-line file. This PR splits it into focused modules as a pure, behaviour-preserving move, and delivers the whole ticket.

What changes

One commit per new module, each one compiling and passing clippy on its own:

• ui/theme_picker.rs: draw_theme_picker, draw_theme_list, draw_theme_preview, badge. It has no tests of its own.
• ui/help_modal.rs: draw_help, draw_global_help and their helpers (help_title, popup_height_for, key_line, help_columns, help_group_lines), with the six tests that exercise them.
• ui/warnings_modal.rs: draw_warnings, warning_lines, WARNING_MAX_LINES, with the eleven warnings tests (including the painted_multi_line_warning helper). It imports popup_height_for from help_modal.rs, which does not import it back, so there is no new cycle.
• ui/menu.rs: draw_menu, draw_palette, draw_options, draw_logo, opt_row, the menu sizing constants and menu_fits, with their tests. p4_overlays_render_key_content also renders the global help, so it imports draw_global_help from help_modal.rs; I kept it whole rather than splitting it so the test inventory stays identical.

ui/modal.rs is now its module doc and the pub use of the four popup helpers from #827 (centered_rect, centered_width, draw_popup_frame, grey_overlay); nothing else is re-exported. I used the grouping the ticket proposes, since it matched the code: help and global help share the sizing helpers, the warnings modal shares only popup_height_for with them, and the menu, palette and Options panel share the logo and the settings row.

Everything besides the moved code is one of these edits, each also listed in the commit message that makes it:

• Each new module is a private mod in ui/mod.rs, and the render functions ui/mod.rs calls are pub(super); the call sites in ui/mod.rs use the new paths (rustfmt re-wrapped the longer theme picker call). draw_theme_preview, draw_logo and opt_row, which only their own module calls, dropped from pub to private.
• popup_height_for is pub(super) because warnings_modal.rs calls it.
• The test fixtures that the help, warnings and menu tests share (flat, rows, bottom_border_row) moved to the ui/mod.rs test module as pub(in crate::ui), next to the helpers ROCMAI-758: split the dash-tui app module, event loop and ui modules into focused files #827 already widened there. The test modules are named tests rather than ported_chrome_tests.
• Imports repointed: the new modules import the popup helpers from ui/panel.rs rather than through the modal.rs re-export, and modal.rs lost the imports it no longer needs.
• Comments and doc links: the comment in draw_help that names the test module, the doc links between draw_help, draw_warnings and popup_height_for (now explicit paths), the banner above the overlays in menu.rs (it listed the global Help, which is now in help_modal.rs), the doc link in panel.rs, the module doc of panel.rs, the comment in ui/mod.rs that names where the grey_overlay test lives, and the comment in the e2e dash step that names where the Esc menu items are.
• docs/architecture.md: one entry per new file, the modal.rs entry rewritten to say it is only the re-export, the panel.rs entry lists the four renderer files as importers of the popup geometry, and the app/types.rs entry now says ui/menu.rs renders the palette destinations and options tabs. The old text said modal.rs renders MENU_ITEMS, which was never true (the menu draws its own item list and count), so I corrected it rather than carrying it over.

One point on the public surface: ui declares pub mod modal, so the functions that were pub in it (draw_help, draw_warnings, draw_theme_picker, draw_menu, and so on) were reachable as rocm_dash_tui::ui::modal::*. Because the new modules are private and only the four geometry helpers are re-exported, those paths no longer exist. Nothing in the workspace uses them (I grepped apps/, crates/, tests/ and xtask/; the only external users of ui::modal are the four helpers pinned by tests/dash_characterization.rs, which still resolve), and the draw_detail move in #827 set the same precedent, but it is a surface change I would rather state than leave implicit.

Verification

Run locally:

• Test inventory: cargo test -p rocm-dash-tui --all-targets -- --list and the same with --ignored, keeping lines ending : test, stripping the module path and sorting without -u, before and after the change. Both lists are identical (999 and 4 names).
• cargo test -p rocm-dash-tui --all-targets passes (960 lib tests, 4 ignored, plus the integration targets).
• cargo fmt --all --check, cargo clippy -p rocm-dash-tui --all-targets -- -D warnings, cargo check -p rocm, cargo xtask manifest --check and cargo xtask check-crate-edges all pass.
• A line-multiset comparison of the old modal.rs against the five resulting files shows only the edits listed above, and the leak scan of the diff against origin/main is clean. cargo doc --document-private-items reports no new warnings for the touched files (it still reports the unresolved gradient::lerp3_t link in the draw_logo doc, which was already unresolved in modal.rs).

Not run locally: the full workspace tests, the tests/e2e-cucumber suite and the Windows lane, which run in CI. The only e2e-visible change is a comment in a dash step. No test was added or changed in behaviour, so there is no new test to break-check; the move is covered by the existing tests, which are unchanged apart from their imports.

Scope

A pure move of the Modal::* renderer bodies and their tests, with the visibility, import, comment and architecture-doc edits above and no behaviour change. Out of scope: any behaviour change and the other dash-tui and workspace refactoring tickets.

[Copilot]

🟢 Approval recommended

No active review issues remain for the extraction of dashboard modal renderers into focused modules.

No open issues 🎉

What changed in this PR

Completes ROCMAI-817 by splitting the remaining dashboard modal renderers into focused modules, building on #827’s extraction of shared popup geometry. Rendering logic remains unchanged, while the former public renderer paths are deliberately removed.

Changes:

• Moves help, warnings, theme-picker, and menu renderers into four modules, alongside their existing tests.
• Updates dispatch paths and visibility, centralizes shared test fixtures, and preserves the four public geometry-helper re-exports.
• Updates architecture documentation and source comments to reflect the new layout.

File	Description
tests/​e2e-cucumber/​tests/​e2e/​dash_steps.rs	Updates a comment naming the Esc-menu renderer’s location.
docs/​architecture.md	Documents the extracted modules and their dependencies.
crates/​rocm-dash-tui/​src/​ui/​warnings_modal.rs	Houses warning rendering, truncation helpers, and existing tests.
crates/​rocm-dash-tui/​src/​ui/​theme_picker.rs	Houses the theme list and live-preview rendering.
crates/​rocm-dash-tui/​src/​ui/​panel.rs	Updates documentation references to the relocated renderers.
crates/​rocm-dash-tui/​src/​ui/​modal.rs	Retains only documentation and four geometry-helper re-exports.
crates/​rocm-dash-tui/​src/​ui/​mod.rs	Registers modules, redirects rendering calls, and shares test fixtures.
crates/​rocm-dash-tui/​src/​ui/​menu.rs	Houses menu, palette, Options, logo helpers, and existing tests.
crates/​rocm-dash-tui/​src/​ui/​help_modal.rs	Houses both help renderers, shared content-height sizing, and existing tests.

_{Review effort: Balanced}

[jussielo-amd]

ROCMAI-817 says the crate's public surface stays as it is after the earlier dash-tui split. On main, rocm_dash_tui::ui::modal exposes ten public items: draw_help, draw_warnings, draw_theme_picker, draw_theme_preview, draw_menu, draw_palette, draw_options, draw_global_help, draw_logo and opt_row. In this PR modal.rs re-exports only the four panel helpers, and the seven draw_* renderers are pub(super) in modules that are private to ui, so those paths stop resolving for anything outside the crate. ui is a pub mod in lib.rs, so this is a change to the public API. I found no user of those paths inside the workspace, but that is not the rule the ticket sets.

The simplest fix that keeps the move pure is to widen the renderers to pub in their new modules and add pub use lines for all ten items in modal.rs, next to the four panel re-exports. If narrowing is intended instead, the PR body should say so as a deliberate API change and the ticket's "surface stays as it is" line should be reconciled.

Separately, the branch is based on f52cc5b0, which is older than current main, so it needs a rebase before it can merge.