diff --git a/src/cv4pve_tui/app.py b/src/cv4pve_tui/app.py index 8984d52..a474540 100644 --- a/src/cv4pve_tui/app.py +++ b/src/cv4pve_tui/app.py @@ -4,6 +4,7 @@ import subprocess import sys from datetime import datetime +from rich.text import Text from textual import work from textual.app import App, ComposeResult, SuspendNotSupported from textual.containers import Horizontal, Vertical @@ -15,6 +16,7 @@ from textual.widgets import ( Footer, Header, Input, + Label, OptionList, Select, Static, @@ -30,17 +32,47 @@ def _message(exc: Exception) -> str: ) -class ConfirmScreen(ModalScreen[bool]): +class DialogScreen(ModalScreen): + """A centred, titled box (#dialog) with labelled rows and right-aligned buttons.""" + + # every widget in a row gets an explicit width: Horizontal alone squeezes Select to 1 + # and stretches Input to 100 %, pushing the rest off-screen + DEFAULT_CSS = """ + DialogScreen { align: center middle; } + DialogScreen #dialog { + width: 80; max-width: 100%; height: auto; max-height: 100%; overflow-y: auto; + border: round $accent; border-title-style: bold; padding: 0 1; + background: $surface; + } + DialogScreen .row, DialogScreen .buttons { height: auto; } + DialogScreen .buttons { align-horizontal: right; } + DialogScreen .row Label { width: 14; height: 3; content-align: left middle; } + DialogScreen .row Select, DialogScreen .row Input { width: 1fr; } + DialogScreen .row Select.mode { width: 16; } + DialogScreen .sub { + height: auto; border: round $accent; border-title-style: bold; padding: 0 1; + } + """ + + def box(self, title: str) -> Vertical: + """The #dialog container; use as `with self.box(title):` inside compose.""" + box = Vertical(id="dialog") + # titles carry guest names: never parse them as markup + box.border_title = Text(title) + return box + + +class ConfirmScreen(DialogScreen): BINDINGS = [("escape", "dismiss(False)", "No")] # noqa: RUF012 - Textual idiom - def __init__(self, message: str) -> None: + def __init__(self, message: str, title: str = "Confirm") -> None: super().__init__() - self.message = message + self.message, self.title_text = message, title def compose(self) -> ComposeResult: - with Vertical(): - yield Static(self.message) - with Horizontal(): + with self.box(self.title_text): + yield Static(self.message, markup=False) + with Horizontal(classes="buttons"): yield Button("Yes", id="yes", variant="error") yield Button("No", id="no") @@ -48,7 +80,7 @@ class ConfirmScreen(ModalScreen[bool]): self.dismiss(event.button.id == "yes") -class EditScreen(ModalScreen): +class EditScreen(DialogScreen): """Dismisses with a dict of validated values, or None when cancelled.""" BINDINGS = [("escape", "dismiss(None)", "Cancel")] # noqa: RUF012 - Textual idiom @@ -58,24 +90,29 @@ class EditScreen(ModalScreen): self.guest, self.config, self.disks = guest, config, disks def compose(self) -> ComposeResult: - with Vertical(): - yield Static( - f"Edit {self.guest['name']} ({self.guest['vmid']}); empty = unchanged" - ) - yield Input( - placeholder=f"cores (now {self.config.get('cores', '?')})", id="cores" - ) - yield Input( - placeholder=f"memory MiB (now {self.config.get('memory', '?')})", - id="memory", - ) - yield Select( - [(f"{k} ({v})", k) for k, v in self.disks.items()], - prompt="disk", - id="disk", - ) - yield Input(placeholder="grow by GiB", id="grow") - with Horizontal(): + with self.box(f"Edit resources: {self.guest['name']} ({self.guest['vmid']})"): + yield Static("Empty = unchanged") + with Horizontal(classes="row"): + yield Label("Cores") + yield Input( + placeholder=f"now {self.config.get('cores', '?')}", id="cores" + ) + with Horizontal(classes="row"): + yield Label("Memory (MiB)") + yield Input( + placeholder=f"now {self.config.get('memory', '?')}", id="memory" + ) + with Horizontal(classes="row"): + yield Label("Disk") + yield Select( + [(f"{k} ({v})", k) for k, v in self.disks.items()], + prompt="disk", + id="disk", + ) + with Horizontal(classes="row"): + yield Label("Grow by (GiB)") + yield Input(placeholder="grow by GiB", id="grow") + with Horizontal(classes="buttons"): yield Button("Apply", id="apply", variant="primary") yield Button("Cancel", id="cancel") @@ -104,7 +141,7 @@ class EditScreen(ModalScreen): return int(text) -class ContextScreen(ModalScreen[str | None]): +class ContextScreen(DialogScreen): """Dismisses with the chosen context name, or None when cancelled.""" BINDINGS = [("escape", "dismiss(None)", "Cancel")] # noqa: RUF012 - Textual idiom @@ -114,7 +151,7 @@ class ContextScreen(ModalScreen[str | None]): self.contexts = contexts def compose(self) -> ComposeResult: - with Vertical(): + with self.box("Switch context"): yield OptionList( *( Option( @@ -130,7 +167,7 @@ class ContextScreen(ModalScreen[str | None]): self.dismiss(event.option.id) -class BackupScreen(ModalScreen[str | None]): +class BackupScreen(DialogScreen): """Dismisses with the chosen storage, or None when cancelled.""" BINDINGS = [("escape", "dismiss(None)", "Cancel")] # noqa: RUF012 - Textual idiom @@ -140,17 +177,20 @@ class BackupScreen(ModalScreen[str | None]): self.guest, self.storages, self.chosen = guest, storages, chosen def compose(self) -> ComposeResult: - with Vertical(): + with self.box(f"Back up {self.guest['name']} ({self.guest['vmid']})"): yield Static( - f"Back up {self.guest['name']} ({self.guest['vmid']}) now: mode snapshot, zstd" + f"Back up {self.guest['name']} ({self.guest['vmid']}) now: mode snapshot, zstd", + markup=False, ) - yield Select( - [(s, s) for s in self.storages], - value=self.chosen, - allow_blank=False, - id="storage", - ) - with Horizontal(): + with Horizontal(classes="row"): + yield Label("Storage") + yield Select( + [(s, s) for s in self.storages], + value=self.chosen, + allow_blank=False, + id="storage", + ) + with Horizontal(classes="buttons"): yield Button("Yes", id="yes", variant="primary") yield Button("No", id="no") @@ -165,11 +205,10 @@ _IPV4_MODES = [("DHCP", "dhcp"), ("Static", "static"), ("None", "none")] _IPV6_MODES = [("SLAAC", "auto"), *_IPV4_MODES] -class NetScreen(ModalScreen): +class NetScreen(DialogScreen): """Edit bridge, VLAN, firewall and IP of one interface; confirms, then applies.""" BINDINGS = [("escape", "dismiss", "Cancel")] # noqa: RUF012 - Textual idiom - DEFAULT_CSS = "NetScreen Horizontal { height: auto; }" def __init__( self, guest: dict, config: dict, bridges: list[str], running: bool @@ -184,32 +223,38 @@ class NetScreen(ModalScreen): self.bridges = sorted(set(bridges) | used) def compose(self) -> ComposeResult: - with Vertical(): - yield Static( - f"Network of {self.guest['name']} ({self.guest['vmid']})", markup=False - ) - yield Select( - [(k, k) for k in self.nets], - value=next(iter(self.nets)), - allow_blank=False, - id="iface", - ) - with Horizontal(): + with self.box(f"Edit network: {self.guest['name']} ({self.guest['vmid']})"): + with Horizontal(classes="row"): + yield Label("Interface") + yield Select( + [(k, k) for k in self.nets], + value=next(iter(self.nets)), + allow_blank=False, + id="iface", + ) + with Horizontal(classes="row"): + yield Label("Bridge") yield Select( [(b, b) for b in self.bridges], allow_blank=False, id="bridge" ) + with Horizontal(classes="row"): + yield Label("VLAN") yield Input(placeholder="VLAN tag (empty = none)", id="tag") - yield Checkbox("Firewall", id="firewall") + with Horizontal(classes="row"): + yield Label("Firewall") + yield Checkbox("", id="firewall") if not self.ip_enabled: yield Static( "IP is configured inside the VM (no cloud-init drive)", id="ip_note" ) for fam, modes in (("ipv4", _IPV4_MODES), ("ipv6", _IPV6_MODES)): - with Horizontal(): - yield Select(modes, allow_blank=False, id=fam) - yield Input(placeholder="address/prefix", id=f"{fam}_cidr") - yield Input(placeholder="gateway", id=f"{fam}_gw") - with Horizontal(): + with Vertical(id=f"{fam}_section", classes="sub") as section: + section.border_title = fam.replace("ip", "IP") + with Horizontal(classes="row"): + yield Select(modes, allow_blank=False, id=fam, classes="mode") + yield Input(placeholder="address/prefix", id=f"{fam}_cidr") + yield Input(placeholder="gateway", id=f"{fam}_gw") + with Horizontal(classes="buttons"): yield Button("Apply", id="apply", variant="primary") yield Button("Cancel", id="cancel") @@ -314,7 +359,10 @@ class NetScreen(ModalScreen): notes.append("IP settings (cloud-init) apply at next boot.") who = f"{self.guest['name']} ({self.guest['vmid']})" self.app.push_screen( - ConfirmScreen("\n".join([f"Change network of {who}?", *lines, *notes])), + ConfirmScreen( + "\n".join([f"Change network of {who}?", *lines, *notes]), + "Network change", + ), lambda ok: ( self.app.start_net_change(self.guest, changes, old, self) if ok @@ -447,7 +495,8 @@ class InfoScreen(ModalScreen): ip = cli.parse_net(str(config.get(f"ipconfig{name[3:]}", ""))) else: mac, ip = net.get("hwaddr", "-"), net - table.add_row( + _add_plain( + table, name, net.get("bridge", "-"), net.get("tag", "-"), @@ -461,8 +510,12 @@ class InfoScreen(ModalScreen): note.display = bool(unavailable) # no empty line when the addresses are known table = self.query_one("#addresses", DataTable).clear() for i in addresses: - table.add_row( - i["name"], i["mac"] or "-", ", ".join(i["ipv4"]), ", ".join(i["ipv6"]) + _add_plain( + table, + i["name"], + i["mac"] or "-", + ", ".join(i["ipv4"]), + ", ".join(i["ipv6"]), ) def _show_backups(self) -> None: @@ -472,7 +525,8 @@ class InfoScreen(ModalScreen): ] table = self.query_one("#backups", DataTable).clear() for b in shown: - table.add_row( + _add_plain( + table, datetime.fromtimestamp(b["ctime"]).strftime("%Y-%m-%d %H:%M") if b.get("ctime") else "-", @@ -528,7 +582,8 @@ class InfoScreen(ModalScreen): lines = [f"{k} = {v or '(remove)'}" for k, v in old.items()] self.app.push_screen( ConfirmScreen( - "\n".join(["Restore the previous network settings?", *lines]) + "\n".join(["Restore the previous network settings?", *lines]), + "Undo network change", ), lambda ok: self.app.start_net_change(self.guest, old, None) if ok else None, ) @@ -589,12 +644,23 @@ class GuestApp(App): self.generation = 0 self.switching = False + CSS = """ + #guests_section { + border: round $accent; border-title-style: bold; padding: 0 1; + } + #guests_section DataTable { height: 1fr; } + """ + def compose(self) -> ComposeResult: yield Header() - yield DataTable(id="guests", cursor_type="row") + with Vertical(id="guests_section") as section: + section.border_title = "Guests" + yield DataTable(id="guests", cursor_type="row") yield Footer() def on_mount(self) -> None: + # one place for every sub_title change: mount, switch and failed switch + self.watch(self, "sub_title", self._show_context_frame) table = self.query_one("#guests", DataTable) table.add_columns( "VMID", @@ -609,6 +675,11 @@ class GuestApp(App): self._show_context() self.action_reload() + def _show_context_frame(self, sub_title: str) -> None: + self.query_one("#guests_section").border_subtitle = sub_title.removeprefix( + "context: " + ) + @work(thread=True) def _show_context(self) -> None: self._read_context() @@ -645,7 +716,7 @@ class GuestApp(App): table.clear() self.guests = {str(g["vmid"]): g for g in guests} for g in guests: - table.add_row( + _add_plain(table, g["vmid"], g["name"], g["type"], g["node"], g["status"], g.get("maxcpu", "-"), _scaled(g.get("maxmem"), 2**20), _scaled(g.get("maxdisk"), 2**30), key=str(g["vmid"]), @@ -766,7 +837,7 @@ class GuestApp(App): if name == "stop": text = f"Hard power-off {who}? The guest is killed without shutting down." self.push_screen( - ConfirmScreen(text), + ConfirmScreen(text, name.capitalize()), lambda ok: self._start_job(guest, name, run) if ok else None, ) @@ -913,7 +984,8 @@ class GuestApp(App): who = f"{guest['name']} ({guest['vmid']})" self.push_screen( ConfirmScreen( - f"Grow {values['disk']} of {who} by {values['grow']} GiB? This cannot be undone." + f"Grow {values['disk']} of {who} by {values['grow']} GiB? This cannot be undone.", + "Grow disk", ), lambda ok: ( self._apply(guest, values) if ok else None @@ -952,6 +1024,11 @@ def _reset_terminal() -> None: sys.stdout.flush() +def _add_plain(table: DataTable, *cells, **kwargs) -> None: + """DataTable parses str cells as markup; names and notes like "a[/b]" must stay literal.""" + table.add_row(*(Text(c) if isinstance(c, str) else c for c in cells), **kwargs) + + def _scaled(value, unit: int): return "-" if value is None else round(value / unit) diff --git a/tests/test_app.py b/tests/test_app.py index 9ca8dd5..5ca54fd 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -1883,3 +1883,106 @@ async def test_info_has_separate_titled_backup_and_network_sections(fake): "unavailable", "addresses", } + + +# --- boxed design: every dialog a titled box, main table in a titled frame ---------- + +from textual.content import Content +from textual.widgets import Label + +SIZE = (100, 40) + + +def visible(app, widget): + r = widget.region + return ( + r.width >= 3 + and r.height >= 1 + and r.right <= app.size.width + and r.bottom <= app.size.height + ) + + +def label_texts(screen): + return {str(w.render()).strip() for w in screen.query(Label)} + + +async def test_net_screen_shows_every_field_inside_the_screen(fake): + # bug: in the old rows Select got width 1 and Input 100 %, pushing fields off-screen + fake.net_configs[100] = dict(CT_CONFIG) + app = make_app() + async with app.run_test(size=SIZE) as pilot: + await open_net(app, pilot) + for wid in ("iface", "bridge", "tag", "firewall", "ipv4", "ipv4_cidr", "ipv4_gw", + "ipv6", "ipv6_cidr", "ipv6_gw", "apply", "cancel"): # fmt: skip + assert visible(app, app.screen.query_one(f"#{wid}")), wid + + +async def test_net_screen_labels_and_ip_sections(fake): + fake.net_configs[100] = dict(CT_CONFIG) + app = make_app() + async with app.run_test(size=SIZE) as pilot: + await open_net(app, pilot) + assert {"Interface", "Bridge", "VLAN"} <= label_texts(app.screen) + assert "IPv4" in str(app.screen.query_one("#ipv4_section").border_title) + assert "IPv6" in str(app.screen.query_one("#ipv6_section").border_title) + + +async def test_edit_screen_fields_have_labels(fake): + app = make_app() + async with app.run_test(size=SIZE) as pilot: + await open_edit(app, pilot) + assert {"Cores", "Memory (MiB)", "Disk", "Grow by (GiB)"} <= label_texts( + app.screen + ) + + +async def open_each_dialog(app, pilot): + """Yields after each dialog is open; closes it with escape afterwards.""" + await settle(app, pilot) + for keys in (["S"], ["e"], ["c"], ["i", "b"], ["i", "n"]): + for k in keys: + await pilot.press(k) + await settle(app, pilot) + yield keys + for _ in keys: + await pilot.press("escape") + await settle(app, pilot) + + +async def test_every_dialog_is_a_titled_centred_box(fake): + fake.net_configs[108] = dict(VM_CONFIG) + app = make_app() + async with app.run_test(size=SIZE) as pilot: + async for keys in open_each_dialog(app, pilot): + box = app.screen.query_one("#dialog") + assert str(box.border_title).strip(), keys + assert box.region.width < app.size.width, keys # a box, not full screen + assert box.region.x > 0, keys # centred + + +async def test_main_table_is_framed_with_title_and_context(fake): + app = make_app() + async with app.run_test(size=SIZE) as pilot: + await settle(app, pilot) + frame = app.query_one("#guests_section") + assert "Guests" in str(frame.border_title) + assert "mainframe" in str(frame.border_subtitle) + + +@pytest.mark.parametrize("keys", [["e"], ["i", "b"], ["i", "n"]]) +async def test_guest_name_with_markup_characters_does_not_break_dialogs(fake, keys): + # border titles are parsed as markup; a name like "a[/b]" must be shown literally + fake.guests = [dict(VM, name="a[/b]")] + fake.net_configs[108] = dict(VM_CONFIG) + app = make_app() + async with app.run_test(size=SIZE) as pilot: + await settle(app, pilot) + for k in keys: + await pilot.press(k) + await settle(app, pilot) + title = app.screen.query_one("#dialog").border_title + assert ( + "a[/b]" in Content.from_markup(title).plain + ) # what is shown, not how it is stored + assert app.is_running