Skip to content

fix/avatar crop scaling#1169

Merged
cha1latte merged 1 commit into
refactorfrom
fix/refactor-pr-55-avatar-crop-scaling
May 24, 2026
Merged

fix/avatar crop scaling#1169
cha1latte merged 1 commit into
refactorfrom
fix/refactor-pr-55-avatar-crop-scaling

Conversation

@munimunigamer

@munimunigamer munimunigamer commented May 24, 2026

Copy link
Copy Markdown
Collaborator

Migrated from Pasta-Devs/Marinara-Engine-Refactor#55: Pasta-Devs/Marinara-Engine-Refactor#55
Original author: @Promansis
Target base: Pasta-Devs/Marinara-Engine refactor
Source draft state: True
Port note: source updates/tracker/evidence files were omitted because the target refactor branch removed repo-local updates guidance.


Linked issue

Fixes #1158

Original migrated source issue: Pasta-Devs/Marinara-Engine-Refactor#35.

Why this change

Character library avatars should follow one explicit crop display contract. The old library and panel paths could pass a persisted JSON-string avatarCrop directly into the crop style helper, which produced invalid crop sizing and could render squished full-image avatars instead of the stored crop.

What changed

  • Added a shared CharacterAvatarImage helper in the React character feature.
  • Swapped the character library detail/card avatars and Characters panel avatars to use the shared crop-aware image path.
  • Preserved the normal uncropped object-cover fallback for missing or invalid crop data.
  • Included group-member avatars in the Characters panel so nested member rows use the same crop contract.

Validation

  • pnpm check
  • pnpm build
  • cargo check --manifest-path src-tauri/Cargo.toml
  • pnpm check:docs when docs, skills, or repo guidance changed
  • Ran pnpm tauri dev and tested the app path manually
  • Verified the affected feature in the right mode(s): chat, roleplay, game, settings/providers, imports/exports, or assets

Codex verification notes

  • Vite + Playwright proof harness passed for the affected render path. It reproduced the old string-crop failure as 160x160 object-fit: fill, then verified the shared helper renders the same persisted string crop as 320x160 absolute crop geometry.
  • Edge case verified: invalid persisted crop string falls back to uncropped object-cover.
  • pnpm check passed locally: architecture, frontend typecheck, Rust cargo check, and docs check.
  • pnpm build passed locally. Vite reported existing chunk-size/dynamic-import warnings.
  • Marinara proof gate passed for scratch/bugfix-verification.json.
  • Bunny review pass: no code blockers after PR wording/evidence update.

Manual verification notes

No manual verification needed for the core claim. The local proof uses a Vite-served React harness importing the production CharacterAvatarImage helper because the browser-only dev app cannot list local Tauri storage rows without the Tauri runtime.

Docs impact

  • No docs changes needed
  • Updated docs/developer/* or repo guidance as needed

UI evidence

Avatar crop proof

Summary by CodeRabbit

  • Refactor
    • Consolidated character avatar rendering logic into a reusable component for improved code maintainability across character display views.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 24, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a8f51629-91de-4306-a154-2024a1549a8c

📥 Commits

Reviewing files that changed from the base of the PR and between 99c21dc and cd1240f.

📒 Files selected for processing (3)
  • src/features/catalog/characters/components/CharacterAvatarImage.tsx
  • src/features/catalog/characters/components/CharacterLibraryView.tsx
  • src/features/catalog/characters/components/CharactersPanel.tsx

📝 Walkthrough

Walkthrough

A new CharacterAvatarImage component consolidates avatar crop resolution and rendering logic. The component validates loosely-typed crop data, parses JSON strings, enforces bounds constraints, and applies crop styles uniformly. Three files refactor to use this shared component, passing crop data from character extensions through charMap lookups and direct character properties.

Changes

Avatar Component Consolidation

Layer / File(s) Summary
CharacterAvatarImage component with crop resolution
src/features/catalog/characters/components/CharacterAvatarImage.tsx
New component renders <img> with lazy loading and validates optional crop data by resolving JSON strings, checking numeric bounds via type guard (isAvatarCrop), and passing resolved crop to getAvatarCropStyle for inline style computation.
CharacterLibraryView avatar integration
src/features/catalog/characters/components/CharacterLibraryView.tsx
Imports updated to use CharacterAvatarImage; detail card header and grid card image rendering replaced to pass crop from character.parsed.extensions?.avatarCrop through the new component.
CharactersPanel avatar and charMap refactoring
src/features/catalog/characters/components/CharactersPanel.tsx
charMap lookup extended to carry avatarCrop from parsed.extensions; group-member and character-row avatar rendering replaced with CharacterAvatarImage component. Imports updated accordingly.

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly Related PRs

  • Pasta-Devs/Marinara-Engine#661: Aligns with updated AvatarCrop/LegacyAvatarCrop model and getAvatarCropStyle behavior that the new component depends upon.
  • Pasta-Devs/Marinara-Engine#770: Modifies the same character-card and detail-panel avatar rendering containers where CharacterAvatarImage integrates.

Suggested Labels

client


Ah, splendid precision...
The scattered brushstrokes now converge—
One canvas for the face,
Crop and frame aligned at last.
A most elegant consolidation.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix/avatar crop scaling' directly addresses the core issue: resolving avatar crop scaling inconsistencies in the character library.
Description check ✅ Passed The PR description comprehensively covers linked issue, rationale, changes made, and validation evidence including proof harness verification and manual checks.
Linked Issues check ✅ Passed The implementation fully addresses issue #1158 by introducing CharacterAvatarImage shared component and applying it consistently across all avatar rendering paths.
Out of Scope Changes check ✅ Passed All changes are scoped to the avatar crop rendering issue: CharacterAvatarImage component creation and refactoring avatar usage in library/panel views.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/refactor-pr-55-avatar-crop-scaling

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed: lockfile failed supply-chain policy check. Run pnpm install locally to update the lockfile.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the bugfix Bug fix label May 24, 2026
@cha1latte

Copy link
Copy Markdown
Contributor

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 24, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot added the client label May 24, 2026
@github-actions github-actions Bot removed the client label May 24, 2026
@cha1latte cha1latte marked this pull request as ready for review May 24, 2026 19:59
@cha1latte cha1latte merged commit 50822cf into refactor May 24, 2026
10 checks passed
@cha1latte cha1latte deleted the fix/refactor-pr-55-avatar-crop-scaling branch May 24, 2026 20:02
@cha1latte cha1latte self-assigned this May 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants