Skip to content

Main window: fixes from the UI finish-gate review - #25

Merged
tneohcl merged 2 commits into
masterfrom
finish-gate-fixes
Oct 7, 2026
Merged

tneohcl merged 2 commits into
masterfrom
finish-gate-fixes

Conversation

@tneohcl

@tneohcl tneohcl commented Oct 7, 2026 •

Copy link
Copy Markdown
Owner

A pre-release review of the main window (the ui-finish-gate-reviewer agent, rendering it offscreen in Dark and Light at default and minimum size) returned HOLD on six findings. This fixes all six.

  1. Long filenames no longer break the settings column. Settings for "…" widened the fixed 456 px column and silently clipped the segmented controls (Processing collapsed to one huge "Automatic"). The scope label (_ElidedLabel) now elides in the middle and asks for no width; the full name is in the tooltip.
  2. Convert acts only on unfinished videos. After a run it still said "Convert 4 Videos" and re-queued every row, re-encoding finished videos into name (1).mp4 copies. It now counts and queues only rows without _completed_output_path, so failed rows are retried; finished rows keep their result. With everything done it's disabled: "All videos converted — add more videos". The deferred start (Convert while analysis is running) uses the same check. The help already promised this behaviour.
  3. Queue columns fit their content. Size was 60 px ("11.2GB" → "11.2…") and Status lost its % at 960 px. Video now stretches and elides; Duration, Size and Status are sized from the font to their widest real value (DropTreeWidget.fit_columns) and refit on font or style changes. Trade-off: Video can no longer be dragged wider.
  4. Controls match the design decisions. No bold on selected segments or tabs (colour-only selection; the bold-width reservation is removed with it). Convert, Cancel and Open Folder are 28 px like the other toolbar buttons (Convert gets a 1 px border in its fill colour instead of border: none). Controls use the 4 px radius token instead of 6 px.
  5. The focus ring no longer sits on the label. Qt drew the QSS outline inside the button, around the text. Buttons now show keyboard focus as their own 2 px border, with padding compensated so nothing moves; the queue draws one 2 px ring around the whole current row instead of a 1 px box around a cell. Clicks still show no ring.
  6. Failures read without hovering. The worker now reports ffmpeg's last real error line instead of just "ffmpeg exited 1" (summarize_ffmpeg_failure). The row's second line shows it in red ("Failed: …"), and the finished outcome is a status headline: icon + sentence at section-title weight ("⚠ Completed with Issues").

Help (video-wont-convert, conversion-status) and the README are updated to match.

Also fixed: TestLeftPanelScrolling.test_no_scrolling_needed_at_default_size_with_expert_collapsed read the real config, so it failed whenever an earlier test had saved Expert as expanded. That showed up here because the new tests shifted the batch boundaries. It now uses _empty_qsettings(), like its sibling.

Tests: 28 new tests (6 classes in test_main.py, plus TestFailureReason in test_worker.py). 24 of them failed before the fixes and pass after; the other 4 are guards. TestSegmentedButtonBoldWidth is replaced by a plain fits-its-text check. All 32 batches of run_tests_chunked.py pass (621 tests).

Verified visually by re-rendering offscreen: long-name selection at 1186×720 and 960×640, the finished state with one failure, converting at minimum size, minimum size with 115% text, and zoomed Tab focus on Convert, a checked segment and a queue row. Not verified: the real desktop at 150% with the system accent and Breeze. In particular, with the default test accent the Convert focus ring (TEXT_ON_ACCENT) contrasts with the fill but not with the dark window behind it.

Not in this PR (the review's optional items): dropping Clear Queue's confirmation, the "Converting…" disabled primary slot, the "Automatic" button reading as a fifth segment, the repeated "Quality" label, the Expert box's collapsed padding, the "Preparing --" double hyphen, sentence case, menu mnemonics, the hard-coded 10pt body, and the two "open folder" buttons.

🤖 Generated with Claude Code

https://claude.ai/code/session_01E1Jny9Kg3YZX8GMCEStTvL

A pre-release review of the main window (rendered offscreen in dark and
light, at default and minimum size) returned HOLD on six findings:

1. A long filename in "Settings for ..." widened the fixed 456 px
   settings column and clipped the segmented controls. The scope label
   now elides in the middle and asks for no width; full name in the
   tooltip.
2. After a run, Convert still counted and queued every row, re-encoding
   finished videos into "name (1).mp4" copies. It now counts and queues
   only unfinished rows (failed ones are retried); with everything
   done it is disabled with a reason.
3. Queue columns were fixed px widths ("11.2GB" showed as "11.2..."; at
   960 px Status lost its %). Video now stretches and elides; Duration,
   Size and Status are sized from the font and refit on font/style
   changes.
4. Selected segments and tabs went bold (selection is colour-only),
   Convert/Cancel/Open Folder were 32 px next to 28 px buttons, and
   controls used a 6 px radius instead of the 4 px token. Fixed; the
   bold-width reservation is gone with the bold.
5. The keyboard focus ring (a QSS outline) was drawn inside the button,
   over the label, and the queue boxed a single cell. Buttons now show
   focus as their own 2 px border, padding-compensated so nothing
   moves; the queue draws one ring around the whole current row.
6. A failed row only said "Failed". The reason (ffmpeg's last real
   error line, not just "ffmpeg exited 1") now shows in red on the
   row's second line, and the finished outcome is a status headline
   (icon + sentence, section-title weight).

Also isolates TestLeftPanelScrolling's default-size test from the real
config: it read whatever Expert state an earlier test had saved, so it
failed depending on batch order.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E1Jny9Kg3YZX8GMCEStTvL
On a machine without a usable GPU (the CI runner) the Processing row is
hidden, and the hidden CPU button reported Qt's placeholder 640x480
geometry, so the test failed there although the column was fine. The
test now pins GPUs present, as the other layout tests do, and asserts each
button it measures is visible.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E1Jny9Kg3YZX8GMCEStTvL
@tneohcl
tneohcl merged commit 1235100 into master Oct 7, 2026
2 checks passed
@tneohcl
tneohcl deleted the finish-gate-fixes branch October 7, 2026 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant