feat(editor): show what a trim keeps, while the handles are still moving - #640
Open
My-Denia wants to merge 2 commits into
Open
feat(editor): show what a trim keeps, while the handles are still moving#640My-Denia wants to merge 2 commits into
My-Denia wants to merge 2 commits into
Conversation
The Edit clip dialog printed Start / End / Duration and then repeated that same range a second time inside the selection bar. Nothing said how long the source was, so there was no figure to read the kept length against, and the kept length itself was gone the moment the dialog closed. The stats row is now Original duration / Trim range / Final duration, and each clip card carries the length that clip contributes to the film. Original duration is the asset's own length and reads an em dash when the document carries none: the track's scale falls back to the out-point so it can always hold the selection, and that fallback must never be shown as the source length — a 0:20-1:45 selection would have claimed a 1:45 source. Two things the running window turned up: The dimmed head and tail are painted after the selection, so the tail sat above the 6px by which the end handle overhangs it. As soon as the range was narrower than the handle, the tail swallowed the grab: a range dragged down to the 0.05s minimum could then only be recovered with Reset. Both are decoration and now take no pointer events. The card timecode does not shrink, and the label pill is capped at calc(100% - 50px) to clear the delete button. Between the 120px narrow gate and about 131px the timecode escaped the pill and came to rest on that button, so a card only carries it from 132px up. Every duration here is a raw-ruler length, the clock the timeline, the transport readout and the cards already share. A speed region changes how long a span plays, not how long it is, and only the export and audio paths integrate that; scaling this one number alone would put it at odds with the ruler above it.
The gate added with the card duration was one number, derived from a six-character `0:12.0`. `formatSec` never prints an hour field, so the string grows without bound — `16:40.0` past ten minutes, `100:00.0` past a hundred — and each extra character is another 6px the label pill has to find. A card wide enough for the short form was therefore still admitting the long one, which is the overlap onto the delete button the gate exists to prevent. The room is now measured against this card's own text. The comparison also subtracts the gutter, which is taken out of the card's width but not out of the span the gate was reading, so the gate had been judging every card 6px wider than it is. Measured in the running window: a 137px card withholds `10:00.0` where the fixed gate rendered it, and 144px, 161px, 1202px and 1358px carry seven-, eight- and nine-character codes inside the pill with the delete button clear.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (18)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe trim dialog now shows original duration, trim range, and final duration. Handle changes update the final duration. Timeline cards show edited durations when space allows. Editor translations use the new duration labels across supported locales. ChangesTrim duration workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant EditClipModal
participant TrimTrack
participant DurationStats
User->>EditClipModal: Drag a trim handle
EditClipModal->>TrimTrack: Update selected range
TrimTrack-->>EditClipModal: Return start and end values
EditClipModal->>DurationStats: Display updated final duration
Merge Risk: ⚪ Minimal · up to The trim dialog and timeline now show accurate edited durations with interaction and width handling covered. No concrete merge-blocking risk is evident. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 4 files. (14 skipped: 14 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
MinhOmega
added a commit
to MinhOmega/Capturia
that referenced
this pull request
Sep 11, 2026
…models and upstream fixes (#13) Capturia 2.1: new editor and capture features, plus the upstream open PRs that were worth taking. ## New features - **Record an area of the screen**: an Area tab in the source picker opens an overlay on the chosen display. You drag, move and resize a rectangle, and it shows the live size in physical pixels. The rectangle is validated and clamped in the main process, and the recording opens already cropped to it. Auto-zoom stays inside the area. Not offered on Wayland. - **Saved looks**: save the current appearance (background and frame, camera layout, cursor, caption style, and optionally the format) as a named preset, apply it in one undo step, and star one as the default for new projects. Regions, trims, zooms, crop and the transcript are never touched. - **Zooms at flagged moments**: Auto-enhance adds a zoom at every moment flagged while recording, using the same placement rules as auto-zoom. A flag that falls in a trim or on an existing zoom is reported, not duplicated. - **Right-click menu** on region pills and clips: Copy, Paste at playhead, Split, Delete. These call the same functions as the keyboard shortcuts. Also fixes Ctrl+C on audio pills, which did nothing before. - **Poster frames**: the project list and media cards show real thumbnails. They are generated by ffmpeg in the main process, cached, and made one at a time. - **Speech model choice**: Fast / Balanced / Accurate in AI settings. Each model is pinned to a SHA-256 digest and verified before it becomes active, and a failed switch keeps the previous model. - **Recordings folder**: choose where new takes are saved. The folder is set only through the OS picker. In that folder, only files Capturia itself names are reachable, after resolving symlinks, and it is never auto-cleaned. If the folder is unavailable, the app offers to use the default before the take starts. - **Pre-release update channel**: opt-in, and it never downgrades (`allowDowngrade` stays false). ## Taken from upstream open PRs Each one was rebuilt on our code where it no longer applied, and each carries its `Upstream-PR:` trailer: getopenscreen/openscreen#302, #386, #519, #520, #571, #617, #632, #640, #641, #642, #644. - #617 drops `node_modules` from `app.asar`. Verified: every npm dependency is bundled by Vite, since externals are Node builtins plus `electron`. `electron-updater` is a bundled chunk, and native addons load from `resourcesPath`. ## Fixes - **Windows Store verify step**: it looked the package up by the pre-rename name, `EtienneLescot.OpenScreen`, which is what failed the RC.3 Store job. It now reads the name from the generated `AppxManifest.xml`. - **Linux export on Intel Arc**: iHD accepts the dmabuf and then returns EIO on every encode, so every hardware export died at the first frame. Each export now probes one real frame and falls back to software if it fails. The mapped frame is also freed when `send_frame` fails. - **Windows microphone drift**: the 44.1→48 kHz path rounded every packet on its own, which added up to 3.75 s/h of growing mic lag. It now carries the position across packets with exact integer totals. 88.2/176.4/352.8 kHz devices now snap to 44.1 kHz, so they go through the anti-alias decimator. - **PipeWire test**: the vendored SPA 1.0.5 compares 64-bit values through an `int`, so the old probe modifier matched Intel X_TILED. The test now uses a modifier that cannot collide. CI now runs this crate's tests. ## Review and audit The integrated branch got an independent security audit and a separate bug hunt. Both were read-only, and every finding was verified by tracing the code. Fixed here: - **Self-update**: it could install a version other than the one the dialog named, or error out instead of falling back to "View Release". It now self-updates only when electron-updater's version matches. - **Recordings folder**: - The writable check always passed on Windows, because libuv ignores directory ACLs. It now creates and deletes a real probe file. - Renderer-named writes are contained after resolving symlinks. - A take keeps the path it opened with, so changing the folder's availability mid-take no longer reports "missing on disk". - The folder cannot be changed while a take is running. - **Poster cache**: one entry per source file, with no flicker when the duration arrives. - **Speech models**: switching is single-flight, and a settings dialog reopened mid-download joins the running download. - **Timeline**: a shift-click that deselects a pill no longer leaves it focused, which had made the menu delete the wrong pill. - **Area recording**: a flag zoom with no telemetry now centres on the recorded area. - **Saved looks**: applying a look is optimistic, so an edit made during its save is no longer lost. - **Saved-looks probe document**: it was invalid at import time. Caught in review before it could crash the editor. ## Verification - Both tsc projects exit 0. Biome is clean; the 26 warnings are the same as on main. The i18n check passes, with real translations in all 13 locales. - Vitest: 255 files, 3099 passed, 1 skipped, on the integrated branch. - Rust: compositor 216 lib tests plus integration tests, and pipewire-capture 84 tests. Both pass locally. - C++ `audio_sample_utils_test`: 97/97 under g++ on Linux, using stub headers. MSVC coverage comes from the `build.yml` dispatch on this branch, which never publishes without `release_tag`. - Every agent-reported claim was re-checked independently. For example, the model digests were checked against Hugging Face's LFS oids, and the electron-updater downgrade path was read in 6.8.9. ## Release note The speech-model change adds a `--dtw-preset` flag to the whisper helper. An older helper ignores unknown flags, so Balanced keeps working. The 2.1 release must still be cut **after** `build-whisper-stt.yml` has finished on main, so the installers stage a helper that understands the flag. Upstream-PR: getopenscreen/openscreen#302 Upstream-PR: getopenscreen/openscreen#386 Upstream-PR: getopenscreen/openscreen#519 Upstream-PR: getopenscreen/openscreen#520 Upstream-PR: getopenscreen/openscreen#571 Upstream-PR: getopenscreen/openscreen#617 Upstream-PR: getopenscreen/openscreen#632 Upstream-PR: getopenscreen/openscreen#640 Upstream-PR: getopenscreen/openscreen#641 Upstream-PR: getopenscreen/openscreen#642 Upstream-PR: getopenscreen/openscreen#644
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Edit clip dialog printed Start / End / Duration and repeated the same range a
second time inside the selection bar. Nothing said how long the source was, so
there was no figure to read the kept length against, and the kept length itself
was gone the moment the dialog closed.
The stats row is now Original duration / Trim range / Final duration, all three
moving while a handle is dragged, and each clip card carries the length that clip
contributes to the film so the figure survives closing the dialog:
Original duration is the asset's own length. When the document carries none it
reads an em dash: the track's scale falls back to the out-point so it can always
hold the selection, and showing that fallback as the source length would tell a
0:20–1:45 selection its source was 1:45 long.
Two adjacent defects turned up while driving this in a running window, both small
and both in the code this change touches:
the tail sat above the 6px by which the end handle overhangs it. Once the range
was narrower than the handle, the tail swallowed the grab, and a range dragged
down to the 0.05s minimum could then only be recovered with Reset. Both are
decoration and now take no pointer events.
delete button, so a card could be admitted at a width that fitted the shortest
timecode but not this clip's.
formatSechas no hour field, so the string growsto 16:40.0 past ten minutes and 100:00.0 past a hundred; the affordance is now
measured against the card's own text, and it also subtracts the card gutter,
which the first cut had not accounted for.
A note on speed regions, since Final duration invites the question: every
duration here is a raw-ruler length, the clock the timeline, the transport
readout and the cards already share. A speed region changes how long a span
plays, not how long it is, and only the export and audio paths integrate that.
Adding two 1.5x regions leaves the transport total, the card and the dialog
unchanged, so scaling this one number alone would put it at odds with the ruler
directly above the dialog.
Related issue
Fixes #558
Type of change
Release impact
Desktop impact
Renderer-only: React, CSS and locale strings. Exercised on Windows.
Screenshots / video
Two things are visible. The Edit clip dialog's stats row reads Original duration
2:35.0, Trim range 0:20.0–1:45.0, Final duration 1:25.0 on the example from the
issue, all three moving as a handle is dragged, and the selection bar under it
no longer repeats the range it used to print. And a clip card now carries its
own edited length beside its name, so after Apply the trimmed figure is still
on screen.
Testing
Unit and type gates:
Both typechecks and the i18n check are clean, and CI is green on the suite for
both commits on this branch.
Driven in a real Electron window rather than jsdom alone, on a seeded project
whose asset is 155s and whose first clip keeps source 20–105s:
elementFromPointat each handle now returns the handleinstead of the dimmed overlay, and the range drags back open
first cut rendered it, and cards from 144px up carry seven-, eight- and
nine-character codes inside the label pill with the delete button clear
the range and the kept length stay right
New tests:
EditClipModal.test.tsxcovers the three readouts, live update on bothhandles, the single statement of the range, the em-dash fallback and the
pointer-transparent overlays;
V4Timeline.geometry.test.tsxcovers the cardduration and both width gates.
Not covered: macOS and Linux, the packaged build, and export — this changes what
the editor reports about a trim, not what the trim does, and no export path is
touched. The trim handles have no keyboard affordance today, so the readout's
aria-live region only ever announces during a pointer drag; giving them one is
out of scope here.