feat: adaptive top bar with corvid mark replaces the stock footer - #155
Conversation
The one-line footer key legend becomes a top bar with the corvid mark: - collapsed by default: logo, current view, the top-4 priority keys for that view, and a '~ more' hint - one line, minimal noise - '~' (remappable action toggle_topbar) expands the full legend grouped Nav / Sort / Actions / Logs / Panes / Agent; group members update dynamically per view from screen.active_bindings, so only keys that act on the resource kind on screen appear (same single source of visibility as before - check_action stays authoritative) - the expanded/collapsed choice persists to ui.topbar in config.yaml (save_topbar_state read-modify-write, wired from __main__.py); terminals narrower than 80 columns render collapsed automatically - pure legend-building helpers in widgets/top_bar.py keep the widget thin and the logic unit-testable without a pilot Closes #142 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Replaces Textual’s footer with an adaptive, persistent top-bar legend.
Changes:
- Adds grouped collapsed/expanded top-bar rendering.
- Wires remappable toggling and config persistence.
- Updates keybinding documentation and UI tests.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/korvid/ui/widgets/top_bar.py |
Implements top-bar rendering. |
src/korvid/ui/app.py |
Integrates and refreshes the top bar. |
src/korvid/core/config.py |
Loads and saves top-bar preference. |
src/korvid/__main__.py |
Wires preference persistence. |
src/korvid/ui/widgets/help_screen.py |
Registers the toggle in help. |
tests/ui/test_top_bar.py |
Tests rendering, behavior, and persistence. |
tests/ui/test_adaptive_footer.py |
Adapts view-scoped legend tests. |
docs/keybindings.md |
Documents the top bar and toggle. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
이슈 #142 상단바 전환 리뷰입니다.
총평: 설계가 깔끔합니다. 순수 헬퍼(group_of/collapsed_entries/build_*)를 위젯에서 분리해 pilot 없이 단위 테스트한 점, 가시성 판단을 기존 check_action/_ACTION_VIEWS 단일 소스(screen.active_bindings)에 그대로 위임한 점, bindings_updated_signal 구독으로 stock Footer와 동일한 갱신 경로를 유지한 점, save_topbar_state의 read-modify-write + atomic write, 콜백 주입으로 app이 저장 함수를 직접 import하지 않는 점 모두 좋습니다. 기존 test_adaptive_footer.py 18개를 _legend_entries() 기준으로 포팅해 뷰별 필터링 불변을 핀한 것도 적절합니다.
인라인으로 Warning 1건(active_bindings["~"] 조회는 실제로는 절대 매치되지 않는 dead lookup — textual은 키를 "tilde"로 저장), Suggestion 1건(레이아웃 전 width=0 폴백) 남겼습니다. 둘 다 크래시성은 아니어서 APPROVE 합니다.
APPROVE
…ded flash Review round 1 on #155: - the toggle hint was resolved by the literal key "~" - a dead lookup (active_bindings is keyed by declared names like "tilde") that also went stale after a toggle_topbar remap. _topbar_toggle_key resolves by action, so 'keybindings: {toggle_topbar: f8}' moves the hint too (test_toggle_hint_reflects_a_remapped_key). - build_collapsed now takes the width as a hard budget: keys that would wrap the height:auto bar are dropped, the logo/view/more-hint always survive (test_collapsed_line_budgets_keys_to_the_width). - pre-layout size.width=0 no longer falls back to MIN_EXPANDED_WIDTH: an unknown width renders collapsed, so a 60-column terminal started with ui.topbar: expanded never flashes a wrapped legend (test_prelayout_render_is_collapsed_even_when_configured_expanded). - PRIORITY_ACTIONS gains 'help' as a generic fallback for views with few specific verbs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
재리뷰 (신규 커밋 1개, round 1 지적사항 반영 확인) — APPROVE
두 건의 이전 지적이 모두 테스트와 함께 해결되었습니다:
- remap-aware toggle hint —
active_bindings["~"]dead lookup이_topbar_toggle_key()로 교체되어 action 기준(toggle_topbar)으로 해석합니다.keybindings: {toggle_topbar: f8}remap 시 힌트도 함께 이동하며,test_toggle_hint_reflects_a_remapped_key가 표시 + 실제 토글 동작까지 검증합니다 (vacuous 아님). - pre-layout expanded flash —
size.width or MIN_EXPANDED_WIDTHfallback 제거. width=0(첫 layout 이전)이면build_legend가 항상 collapsed를 반환하므로 60컬럼 터미널에서ui.topbar: expanded로 시작해도 wrapped legend가 한 프레임도 노출되지 않습니다.
추가로 build_collapsed의 width 하드 버짓(초과 키 drop, logo/view/more-hint 항상 생존)은 cell_len 기반 계산이 선행 구분자 2칸까지 정확히 반영하고, PRIORITY_ACTIONS의 help fallback도 합리적입니다. 신규 지적사항 없음.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/korvid/ui/widgets/top_bar.py:88
enter: drillis injected into every expanded legend, buton_data_table_row_selectedexplicitly leaves Enter unconsumed when the current kind has neither a hierarchy nor adrill_child(app.py:2085-2101), so views such as ConfigMaps advertise a shortcut that does nothing. Derive these handler entries from the current view (or model drill as a gated binding) instead of adding it unconditionally.
("Nav", "enter", "drill"),
src/korvid/ui/app.py:7556
- Suppressing every persistence exception makes a successful-looking toggle silently revert after restart when the config is unwritable or malformed. Surface a warning as the existing agent config paths do (
app.py:2783-2792) so users know the in-memory preference was not saved.
if self._save_topbar is not None:
with contextlib.suppress(Exception):
self._save_topbar(self._topbar_expanded)
Review round 2 on #155 (both suppressed findings were credible): - 'enter: drill' was injected into every expanded legend, but on_data_table_row_selected leaves Enter unconsumed on views with neither a hierarchy nor a drill child - nodes/configmaps advertised a dead key. _topbar_can_drill mirrors that handler's gating and the entry only renders where Enter drills (test_enter_drill_hint_is_gated_by_drill_capability). - a failed save_topbar no longer disappears into contextlib.suppress: the in-memory toggle stays applied and a warning names the consequence, matching the agent-config save paths (test_failed_persistence_warns_instead_of_silently_reverting). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Round 2's two suppressed findings were both credible and are fixed in 82b112c:
Full gate green (2896 passed). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/korvid/ui/widgets/top_bar.py:67
- On a Helm releases view,
helm_historyis active, but because it is absent here the four collapsed slots become Describe, Install, Upgrade, and the generic Help action. That contradicts the stated rule that view-specific verbs outrank generic fillers; place History before the generic tail.
"helm_install",
"helm_upgrade",
"helm_rollback",
src/korvid/ui/widgets/top_bar.py:142
- The width guard budgets only optional key entries. If the fixed logo + view name + toggle hint already exceeds
width(for example on a very narrow terminal or with a long CRD plural), the function still returns text wider than the widget andheight: autowraps it. The claimed one-physical-line guarantee therefore still fails; the fixed segments also need a truncation/degradation policy.
hint_len = 2 + cell_len(f" {toggle_key} ") + cell_len(" more")
used = cell_len(text.plain)
for entry in collapsed_entries(entries):
entry_len = 2 + cell_len(f" {entry.key} ") + cell_len(f" {entry.description}")
if width > 0 and used + entry_len + hint_len > width:
…last resort Review round 3 on #155 (suppressed advisories, both credible): - helm_history and uncordon_node were missing from PRIORITY_ACTIONS, so generic help could outrank a view verb on helm/node views (test_view_specific_verbs_outrank_generic_fillers_everywhere). - the width budget only dropped optional keys: a terminal narrower than logo + view + hint still wrapped. cell-level truncation with ellipsis is the last-resort floor - one physical line, always (test_collapsed_line_truncates_when_even_the_fixed_parts_overflow). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Round 3's two suppressed advisories are also fixed in cd367ca:
Rounds 2 and 3 both contained only suppressed low-confidence findings and no blocking ones — per the review-loop policy this closes the loop; merging once required checks are green. |
There was a problem hiding this comment.
재리뷰 (신규 커밋 2건) — 이전 라운드의 잠재 지적 사항들을 모두 해결한 커밋입니다.
-
enter: drill힌트가 drill 능력에 따라 게이팅됨:_topbar_can_drill()이on_data_table_row_selected의 게이팅(pods, hierarchy root, drill_child)을 그대로 미러링해서 nodes/configmaps처럼 Enter가 아무것도 하지 않는 뷰에서 죽은 키를 광고하지 않게 됨._DRILL_ENTRY분리 +can_drill파라미터가 build_legend→build_expanded→TopBar까지 일관되게 스레딩됨. 게이팅 테스트(pods에는 표시, nodes에는 미표시)도 실제 뷰 전환으로 검증하는 진짜 테스트. -
save_topbar실패가contextlib.suppress로 사라지지 않고 warning notify로 표면화됨 — 인메모리 토글은 유지되고 재시작 시 이전 상태로 돌아간다는 결과를 명시. broken_save 테스트가 경고 발생 + 토글 유지 둘 다 단언. -
helm_history/uncordon_node가 PRIORITY_ACTIONS에 추가되어 뷰 동사가 generic filler(help 등)에 슬롯을 뺏기지 않음, 테스트로 순서 고정. -
collapsed 라인의 최후 수단 하드 트렁케이션(
text.truncate(width, overflow="ellipsis")): logo+view+hint조차 안 들어가는 초협소 터미널에서도 한 줄 유지,cell_len ≤ width단언 테스트 포함.
지적 사항 없음. APPROVE
Closes #142
What
Replaces the stock Textual
Footerwith an adaptive top bar carrying the corvid mark, per the decisions recorded on the issue.Collapsed (default)
One line: logo, current view, the top-4 priority keys for that view,
~ morehint. Priority is the fixedPRIORITY_ACTIONSorder with view-specific verbs outranking generic ones (a pods view surfaces describe/logs/shell/port-forward; generichelp/filter/cmdfill in on views with fewer verbs). Keys that would wrap the line on a narrow terminal are budget-dropped — the bar never exceeds one physical line. The~hint trackstoggle_topbarremaps.Expanded (
~)Full legend grouped Nav / Sort / Actions / Logs / Panes / Agent — fixed group order, dynamic members per view: entries come from
screen.active_bindings, so the samecheck_actionfiltering that governed the footer stays the single source of key-visibility truth (helm'si/u/ronly on helm views, node ops only on nodes, …).Decisions implemented (from #142 discussion)
~— remappable actiontoggle_topbarPRIORITY_ACTIONS), top 4( o> korvidon the leftui.topbar: expanded|collapsedin config.yaml, saved on toggleHow
ui/widgets/top_bar.py(new): pure helpers (group_of,collapsed_entries,build_collapsed,build_expanded,build_legend) + a thinTopBar(Static)widget — legend logic is unit-testable without a pilot.ui/app.py: composesTopBarinstead ofFooter; subscribes toscreen.bindings_updated_signal(the same signal the stock footer used) so the legend refreshes whenever bindings change;action_toggle_topbarflips state and persists via an injectedsave_topbarcallback.core/config.py:ui_topbar_expandedfield,ui.topbarparsing,save_topbar_stateread-modify-write (atomic).__main__.py: wiressave_topbar(app never imports save functions directly).~row +toggle_topbarin the remappable list.Testing
tests/ui/test_top_bar.py(new, 16 tests): pure helper behavior (grouping, priority selection, width collapse), app wiring (logo present, collapsed default,~toggles, per-view dynamic members change when the view changes, configexpandedstart, persistence callback fired), config parse/save round-trip.tests/ui/test_adaptive_footer.py: ported from Footer internals toapp._legend_entries()— all 18 still pass, pinning that per-view key filtering is unchanged.