Add smart-fit as default photo fit mode#55
Merged
Merged
Conversation
Resolve fit per slide from media vs canvas aspect ratio so mismatched orientations letterbox instead of crop, while keeping cover for compatible pairs and manual overrides. Co-authored-by: Cursor <cursoragent@cursor.com>
Use object-contain in timeline blocks and per-slide aspect-ratio on filmstrip cards so portrait and landscape media preview without forced 16:9 cropping. Co-authored-by: Cursor <cursoragent@cursor.com>
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
smart-fitas the default global fit mode (themes included). At plan time it resolves per slide from media vs canvas aspect ratio: orientation/shape mismatches →blur-fill, compatible orientations →cover, unknown dimensions →contain.width/heightwhen media is loaded (image bitmap + Mediabunny video track) so the planner can make fit decisions.aspectRatiointoplan(); RenderPlan entries still carry only concrete fit modes (cover|contain|blur-fill). Per-slide overrides unchanged.Code review
Score: 4/5 — APPROVE
Blocking findings
None.
Non-blocking findings
media-loader.ts— video files open two separate MediabunnyInputinstances (duration + dimensions). Could merge into one parse in a follow-up.planner.ts—aspectRatiois a 7th positional parameter; consider an options bag if more plan inputs accumulate.mediaMetadatawidth/height fallback path (slide lacks dims, metadata has them).createImageBitmapis not stubbed in import tests; dimension extraction silently no-ops on invalid blobs in test env (acceptable for now).Strengths
smartFit.ts+ planner); composition unchanged.smartFit.test.tsplus planner integration tests for the key cases.fitMode: 'cover'are unaffected.Test plan
pnpm testpnpm lintpnpm buildMade with Cursor