feat: add column visibility selector popup

[amir20]

Summary

• Add a popup dialog (v key) to show/hide and reorder table columns in the container list view
• Container list rendering is now dynamic based on column visibility configuration
• Column config can be persisted to the YAML config file with a save-on-close prompt (y/n/Esc)
• New columns config key supports setting default visible columns and their order

Details

Popup controls:

• ↑/↓ or j/k — navigate columns
• Enter/Space — toggle column visibility (Name is always visible)
• PageUp/PageDown — reorder columns
• Esc or v — close (prompts to save if changed)

Config example:

columns:
  - status
  - name
  - cpu
  - memory

Test plan

• Unit tests for Column enum and ColumnConfig (toggle, reorder, serialize/deserialize)
• Config round-trip tests (YAML serialization with/without columns)
• Snapshot tests for column selector popup and hidden columns
• All 108 tests pass
• clippy and fmt clean
• Perf: reduced per-frame allocations (Cow headers, &str icons, no String::new())

🤖 Generated with Claude Code

[github-actions]

Docker Image Built Successfully

docker pull ghcr.io/amir20/dtop:pr-261

[claude]

Review

Good feature overall — well-structured code with solid test coverage. A few issues worth addressing:

Bug: Help popup regression

Adding the v line to the help popup has pushed the sort keybindings row (u/U n/N c/C m/M) off-screen. The updated snapshot confirms this — the line is simply gone:

-            │   u/U         Uptime       n/N         Name           c/C             CPU                    │

The help popup appears to have a fixed height and doesn't scroll, so users can no longer see sort help. Either expand the popup height or consolidate the v entry onto an existing line (e.g., alongside a).

UX issue: Host column toggle is misleading

The Host column can be toggled in the column selector, but visibility is also gated by show_host_column (only true when >1 host is connected). When running a single host, the user can mark Host as visible in the selector, save it to config, and the column still won't appear. The toggle appears effective but has no visible effect.

Options: either disable the Host row in the selector when only one host is connected, or decouple the show_host_column logic entirely and let column config drive it.

Non-atomic config write (columns.rs:377)

if let Err(e) = std::fs::write(config_path, yaml_string) {

A direct write can leave a partially-written (corrupted) config file if the process is killed mid-write. The standard safe pattern is write-to-temp-file + fs::rename (atomic on the same filesystem):

let tmp = config_path.with_extension("tmp");
std::fs::write(&tmp, yaml_string)?;
std::fs::rename(&tmp, config_path)?;

Silent save failure

save_column_config is fire-and-forget — the user presses y, the prompt disappears, but if the write fails they get no indication. The error only goes to tracing::error!. Consider sending an AppEvent to surface the error in the UI (the existing connection_errors pattern could be reused).

Minor: visible_columns() allocates per render frame

visible_columns() builds a Vec<Column> on every call and render_container_list calls it every 500ms frame. With only 9 columns this is negligible, but if this is called multiple times per frame (header, rows, constraints — currently 3x), caching it in the render function (already done via the visible_columns local variable) is the right pattern. Looks fine as-is.

[claude]

Review: Nice feature overall - modular design in columns.rs looks good. Issues: (1) save_column_config ignores config_path:None intent - silently creates ~/.config/dtop/config.yaml when CLI flags are used. (2) Silent disk write failures - spawn_blocking JoinHandle dropped, write errors not surfaced in UI. (3) YAML comments stripped on save via serde_yaml round-trip. (4) visible_columns() allocates fresh Vec on every 500ms render frame - should be cached. (5) Minor: hidden column order not preserved in config round-trip.

[claude]

Code Review - Overall this is a well-structured feature with good test coverage. Four issues worth addressing: 1) Help popup lost sorting shortcuts - the u/n/c/m key bindings row was silently dropped from the snapshot when the v line was added (the popup ran out of height). Either increase popup height or combine lines to fit. 2) Non-atomic config write - write_column_config writes directly to the file which can corrupt it if interrupted mid-write; prefer writing to a temp file then renaming over the destination. 3) Config comments silently erased on save - serde_yaml drops all YAML comments when round-tripping; consider warning the user in the save prompt. 4) visible_columns allocates every frame (noted in PR description as known) - since ColumnConfig only changes on user interaction, caching in AppState like sorted_container_keys would be cleaner. Minor: Column Status label shows Status Icon in the popup but the table header is empty; renaming to Status avoids confusion.

[claude]

test

[claude]

Review

This is a well-structured feature with good test coverage. A few issues worth addressing:

Bugs:

Host column confusion with single-host setups (src/ui/container_list.rs, src/ui/render.rs): The Host column is included in visible_columns() but filtered out by show_host_column in rendering. If a user has one host and Host is toggled visible in the selector, the checkbox shows [X] but the column never appears in the table. Either hide the Host entry from the selector when there is only one host, or explain that it auto-hides.

from_config_strings does not enforce Name visibility (src/core/types.rs:644): The toggle() method prevents hiding Name interactively, but from_config_strings does not enforce it. A user who edits their config file to omit name from the columns list will end up with Name hidden. Should add a guard after building result to ensure Name is always visible.

Help popup regression (src/ui/snapshots/...help_popup_enabled.snap): The snapshot diff shows the sort keys row (u/U Uptime n/N Name c/C CPU) was dropped from the help popup. Looks like an unintentional removal.

Performance:

visible_columns() allocates a Vec on every render frame (src/ui/container_list.rs:40): visible_columns() is called three times per render (rows, header, constraints) and allocates a new Vec each time. Since column config changes rarely, this could be cached in AppState and invalidated only on config change - same pattern as sorted_container_keys.

UX:

Save prompt Esc is confusing (src/core/app_state/columns.rs:274): When the save prompt is shown, pressing Esc dismisses the prompt and returns to the column selector. But since the user already pressed Esc/v to close, they likely want to exit entirely. Consider making Esc in the save prompt discard changes and close (the n behavior) rather than returning to the selector.

No user feedback on save (src/core/app_state/columns.rs:347): When columns are saved, there is no visible confirmation. The tracing::debug! is not visible to users. A brief status line (like how connection errors are shown) would help.

Minor:

config_path is None when CLI hosts are provided (src/main.rs:880): This silently disables saving when --host flags are used. Worth surfacing in the UI - either grey out the save option or show a message like Save unavailable (CLI hosts in use).

spawn_blocking result is dropped (src/core/app_state/columns.rs:359): The JoinHandle from spawn_blocking is dropped silently. If the write task panics, it would be unobserved. At minimum use let _ = to make the intent explicit.

---

Overall this is solid work - the Cow<'static, str> header optimization, avoiding .to_string() on icons, the modular split, and the snapshot test coverage are all well done.