fix(attachment_engine): default TextAttachmentRenderer.showSearch to false - #65
Merged
Conversation
… from a bare URL Adds a background cache-warming entry point where the caller supplies only a URL (and optionally id/name) — no pre-built Attachment, no render/open needed. Reuses the existing resolve() pipeline end to end (cache-hit check, in-flight dedup, download, cache write), so it automatically inherits every fix from the prior audit pass: 64 KB chunked reads, debounced+durable metadata writes, serialized cache mutations, dispose()/teardown. Best-effort by design: a failed prefetch (network error, 404, etc.) does not throw, but is still reported through the configured AttachmentDiagnosticsSink, same as a failed open(). Caller must pass a stable id when url is a short-lived signed URL — using the URL itself as the cache identity (the default when id is omitted) only works for URLs that don't rotate; a signed URL's cache identity needs to come from somewhere stable, same constraint Attachment.stableIdentity already documents for remoteUrl. Documented in the dartdoc and CHANGELOG. New tests: prefetch() downloads+caches a bare URL; a second prefetch() for an already-cached id is a no-op (no re-download); a failed prefetch() completes without throwing. 184/184 tests pass. flutter analyze clean. license-check and dart pub publish --dry-run clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Callers need to know whether a prefetch actually downloaded something or the content was already cached (fromCache), and need the resolved attachment/localPath to show or use the now-cached file elsewhere in the app (e.g. a cached indicator, opening it from a different screen) without triggering a second download. void gave them neither. prefetch() now returns Future<ResolvedAttachment?> — the resolved result on success, null on failure (still best-effort: no throw, still reported to AttachmentDiagnosticsSink). Updated tests assert fromCache is false on first prefetch, true on a second prefetch of the same id, and that the returned localPath actually exists on disk. 184/184 tests pass. flutter analyze clean. license-check and dart pub publish --dry-run clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ility AttachmentManager.pinForOffline(attachment) caches (if needed) and exempts an attachment from every automatic cache-cleanup path — size-cap LRU eviction, clearUnused(), clearExpired() — so a file the user explicitly marked 'save for offline'/'keep downloaded' survives storage pressure instead of silently disappearing under cache limits. unpinFromOffline()/isPinnedForOffline() reverse/query it. Pinning is deliberately scoped: it only exempts a file from *automatic* cleanup, not from an explicit, targeted removal. deleteCache() (and AttachmentCacheManager.clearAttachment()/clearAll()) still remove a pinned file on request — a host app always has a way to actually delete one when it needs to, pinned or not. lookup() now also serves a pinned entry past its remote expiresAt/retention window: those describe when the *remote* source is considered stale, not whether the already-downloaded bytes are usable — and the entire point of pinning is guaranteed offline availability regardless of the remote/network state. New CacheEntry.pinned field (default false; absent in metadata written before this change, read back as false — no migration needed). New tests: CachePolicy skips pinned entries for both size-cap eviction and expiry selection (and documents that enough pinned content can genuinely leave the cache over its cap — deliberate); pin() throws for an uncached attachment; unpin() restores normal eviction eligibility; isPinned() is false for uncached content; clearAttachment() still removes a pinned entry; clearUnused() skips one. Plus AttachmentManager-level pinForOffline()/unpinFromOffline() integration tests through the full write -> pin -> eviction-pressure -> lookup path. 195/195 tests pass. flutter analyze clean. license-check and dart pub publish --dry-run clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…s, not presentation - attachment_engine_ios: OfficePreviewChannel now implements QLPreviewControllerDelegate; the openOfficePreview future completes only on previewControllerDidDismiss instead of right after presenting the modal, so callers finally learn when the preview actually closes. - attachment_engine: OfficeAttachmentRenderer gains an onDismissed callback fired once that future resolves, so a host app can react to the QuickLook close (e.g. pop the screen behind it) instead of being left showing a bare placeholder. - Version bumps: attachment_engine 0.0.1-dev.3, attachment_engine_ios 0.0.1-dev.2, with CHANGELOG entries for both.
…aint tightening Live pub.dev only has attachment_engine_ios up to 0.0.1-dev.1; dev.2 exists locally but isn't published yet. Tightening the constraint to >=0.0.1-dev.2 broke pub resolution (confirmed via pana, which resolves against the real published registry, not local pubspec_overrides.yaml). Reverted to ^0.0.1-dev.0 until dev.2 is actually published.
…false The in-file search bar was on by default for every plain-text preview. Consuming apps that don't want it had to override the renderer at the call site. Flip the default off; opt in explicitly via showSearch: true.
Contributor
Reviewer's GuideThis PR releases attachment_engine dev.4 with an opt-in text search bar, adds cache pinning and URL prefetch APIs backed by the existing resolution pipeline, and completes the related iOS QuickLook dismissal callback/native package update. Sequence diagram for attachment prefetch and offline pinningsequenceDiagram
participant App
participant Manager as AttachmentManager
participant Resolver as ResolutionPipeline
participant Cache as AttachmentCacheManager
participant Store as MetadataStore
App->>Manager: prefetch(url, id, name)
Manager->>Resolver: _resolveWithDiagnostics(attachment)
Resolver->>Cache: lookup(attachment)
alt cache miss
Resolver->>Resolver: download and cache write
else cache hit
Cache-->>Resolver: cached file
end
Resolver-->>Manager: ResolvedAttachment
Manager-->>App: ResolvedAttachment or null
App->>Manager: pinForOffline(attachment)
Manager->>Resolver: _resolveWithDiagnostics(attachment)
Resolver-->>Manager: ResolvedAttachment
Manager->>Cache: pin(attachment)
Cache->>Store: put(pinned entry)
Manager-->>App: ResolvedAttachment
Sequence diagram for iOS QuickLook dismissal callbacksequenceDiagram
participant Flutter as OfficeView
participant Channel as OfficePreviewChannel
participant QuickLook as QLPreviewController
participant Host as HostApp
Flutter->>Channel: openOfficePreview(path)
Channel->>QuickLook: present preview
Channel-->>Channel: await previewControllerDidDismiss
QuickLook-->>Channel: previewControllerDidDismiss(controller)
Channel-->>Flutter: openOfficePreview completes
Flutter->>Host: onDismissed()
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 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 |
digvijaysinh-cloudemy
approved these changes
Sep 1, 2026
…h-default-off # Conflicts: # packages/attachment_engine/attachment_engine/CHANGELOG.md # packages/attachment_engine/attachment_engine/pubspec.yaml
github-actions
Bot
added
infra: dependencies
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
digvijaysinh-cloudemy
enabled auto-merge (squash)
September 1, 2026 04:45
digvijaysinh-cloudemy
disabled auto-merge
September 1, 2026 04:45
digvijaysinh-cloudemy
added
autosubmit
…e search feature Fixes 8 test failures introduced by the showSearch default flipping to false: these tests exercise the in-file search UI and previously relied on the (now-changed) default rather than opting in explicitly.
dhc-tech
added
autosubmit
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
2 participants
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.
What
Flips
TextAttachmentRenderer.showSearchdefault fromtruetofalse. The in-file search bar in the plain-text viewer is now opt-in.Why
Consuming apps that don't want the search bar had to override the renderer at the call site just to disable it. Making it opt-in matches the plain
Text()/scrollable-view behavior most callers actually want.Notes
0.0.1-dev.4, CHANGELOG updated.^0.0.1-dev.3should be bumped to pick this up.Summary by Sourcery
Make in-file search opt-in for text attachments while preserving explicit search behavior.
Bug Fixes:
TextAttachmentRenderer.showSearchtofalse.Tests:
Chores:
attachment_engineto version0.0.1-dev.4and document the changed default in the changelog.