Gallery hero zoom animation with interactive drag-to-dismiss (#14) - #41
Merged
Conversation
Present the gallery as a fullScreenCover with iOS 18's native zoom navigation transition instead of a sheet. Thumbnails register as matchedTransitionSource so the system drives the thumbnail->fullscreen expansion and Photos-style interactive drag-to-dismiss, replacing the scroll-bounce detection approach from the earlier WIP. Dismissing zooms back to whichever media is currently shown. Adds a close button and disables interactive dismiss while pinch-zoomed or scrubbing video. Claude-Session: https://claude.ai/code/session_01BzkuYBpheMbQn6EdRPT4wg
The swiftlint build phase's invisible_character autocorrect was
stripping the literal U+200B characters these URL-parser tests
deliberately contain. Explicit \u{200B} escapes keep the fixtures
intact and lint-clean.
Claude-Session: https://claude.ai/code/session_01BzkuYBpheMbQn6EdRPT4wg
📝 WalkthroughWalkthroughThe gallery now uses shared namespace zoom transitions, full-screen presentation, and an in-gallery close button. Reply context and transition-source environment values support thumbnail ownership. URL tests use explicit zero-width-space escapes. ChangesGallery transition and presentation
Test URL encoding
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PostView
participant ThreadView
participant GalleryView
PostView->>ThreadView: select media and present gallery
ThreadView->>GalleryView: present full-screen gallery with shared namespace
GalleryView->>GalleryView: disable dismissal while zoomed or seeking
GalleryView->>ThreadView: dismiss through close button
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@swiftchan/Views/Boards/Catalog/Thread/PostView.swift`:
- Around line 55-61: Update the gallery source activation around
galleryTransitionSource in PostView so the obscured ThreadView or RepliesView
list source is inactive while the pushed post-detail destination is visible.
Track and use the active gallery-source owner rather than relying only on
inRepliesContext and presentationState.presentingReplies, ensuring only the
current context registers mediaIndex.
In `@swiftchan/Views/Boards/Catalog/Thread/ThreadView.swift`:
- Around line 131-135: Add a ScrollViewReader around the reply grid in
RepliesView and, when presentationState.presentingReplies is true, scroll to the
reply corresponding to presentationState.galleryIndex before GalleryView
dismissal so its thumbnail is rendered as the zoom source. Reuse the existing
reply-to-media mapping and scrollToPost behavior where applicable, without
changing the non-replies flow.
In `@swiftchan/Views/Media/Gallery/GalleryView.swift`:
- Around line 120-133: Add .allowsHitTesting(!isZoomed) and
.accessibilityHidden(isZoomed) to the closeButton view so it is neither
interactive nor exposed to accessibility when zoomed, while preserving its
existing opacity and animation behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6f4e7d76-b841-4755-b3c8-5c7f1730bc71
📒 Files selected for processing (7)
swiftchan/Models/PresentationState.swiftswiftchan/Services/AccessibilityIdentifiers.swiftswiftchan/Views/Boards/Catalog/Thread/PostView.swiftswiftchan/Views/Boards/Catalog/Thread/RepliesView.swiftswiftchan/Views/Boards/Catalog/Thread/ThreadView.swiftswiftchan/Views/Media/Gallery/GalleryView.swiftswiftchanTests/swiftchanTests.swift
Comment on lines
+55
to
+61
| .galleryTransitionSource( | ||
| id: mediaIndex, | ||
| namespace: galleryNamespace, | ||
| // Only one context may own a source id: the thread | ||
| // list normally, RepliesView while it is pushed. | ||
| isActive: inRepliesContext == presentationState.presentingReplies | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file="swiftchan/Views/Boards/Catalog/Thread/PostView.swift"
printf '%s\n' '--- target file ---'
sed -n '1,180p' "$file"
printf '%s\n' '--- related declarations and usages ---'
rg -n -C 4 \
'inRepliesContext|presentingReplies|galleryTransitionSource|galleryNamespace|ThreadView|RepliesView' \
swiftchan/ViewsRepository: vanities/swiftchan
Length of output: 35102
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(PostView|ThreadView|RepliesView|Gallery|Catalog)([^/]*\.swift)$'
printf '%s\n' '--- exact environment declarations and presentation state ---'
rg -n -C 8 \
'EnvironmentKey|inRepliesContext|presentingReplies|presentationState|navigationDestination|sheet|fullScreenCover|NavigationStack|NavigationLink' \
swiftchan -g '*.swift'Repository: vanities/swiftchan
Length of output: 49487
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
terms = (
"inRepliesContext",
"presentingReplies",
"galleryTransitionSource",
"galleryNamespace",
)
for path in Path("swiftchan").rglob("*.swift"):
text = path.read_text(errors="replace")
if any(term in text for term in terms):
print(f"\n--- {path} ---")
for i, line in enumerate(text.splitlines(), 1):
if any(term in line for term in terms):
lo, hi = max(1, i - 8), min(len(text.splitlines()), i + 12)
lines = text.splitlines()
for n in range(lo, hi + 1):
print(f"{n}: {lines[n-1]}")
print()
PYRepository: vanities/swiftchan
Length of output: 18627
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- ThreadView hierarchy ---'
sed -n '45,145p' swiftchan/Views/Boards/Catalog/Thread/ThreadView.swift
sed -n '225,245p' swiftchan/Views/Boards/Catalog/Thread/ThreadView.swift
printf '%s\n' '--- RepliesView hierarchy ---'
sed -n '20,45p' swiftchan/Views/Boards/Catalog/Thread/RepliesView.swift
printf '%s\n' '--- deterministic source-activation model ---'
python3 - <<'PY'
cases = [
("thread list", False, False),
("ThreadView PostView destination", False, False),
("RepliesView list", True, True),
("RepliesView PostView destination", True, True),
]
for name, in_replies, presenting_replies in cases:
active = in_replies == presenting_replies
print(f"{name}: inRepliesContext={in_replies}, "
f"presentingReplies={presenting_replies}, isActive={active}")
groups = {}
for name, in_replies, presenting_replies in cases:
if in_replies == presenting_replies:
groups.setdefault((in_replies, presenting_replies), []).append(name)
for state, owners in groups.items():
if len(owners) > 1:
print(f"DUPLICATE-CONTEXT: {state} -> {', '.join(owners)}")
PYRepository: vanities/swiftchan
Length of output: 7284
Deactivate the obscured list source for pushed PostView destinations.
Both ThreadView and RepliesView register the same mediaIndex as their pushed PostView because the destination inherits the list’s context values. Track the active gallery-source owner and deactivate the obscured list source while a post-detail destination is visible.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@swiftchan/Views/Boards/Catalog/Thread/PostView.swift` around lines 55 - 61,
Update the gallery source activation around galleryTransitionSource in PostView
so the obscured ThreadView or RepliesView list source is inactive while the
pushed post-detail destination is visible. Track and use the active
gallery-source owner rather than relying only on inRepliesContext and
presentationState.presentingReplies, ensuring only the current context registers
mediaIndex.
Comment on lines
+131
to
+135
| // Zoom back to whichever media the user is on; | ||
| // scrollToPost keeps its thumbnail on screen. | ||
| .navigationTransition( | ||
| .zoom(sourceID: presentationState.galleryIndex, in: galleryNamespace) | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(ThreadView|GalleryView|RepliesView)\.swift$'
printf '%s\n' '--- ThreadView outline ---'
ast-grep outline swiftchan/Views/Boards/Catalog/Thread/ThreadView.swift
printf '%s\n' '--- relevant symbols and call sites ---'
rg -n -C 5 'galleryIndex|scrollToPost|presentingReplies|fullScreenCover|navigationTransition|RepliesView|GalleryView' swiftchan/Views/Boards/Catalog/Thread
printf '%s\n' '--- line range under review ---'
cat -n swiftchan/Views/Boards/Catalog/Thread/ThreadView.swift | sed -n '90,165p'Repository: vanities/swiftchan
Length of output: 18137
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- ThreadView body and helpers ---'
cat -n swiftchan/Views/Boards/Catalog/Thread/ThreadView.swift | sed -n '1,115p'
cat -n swiftchan/Views/Boards/Catalog/Thread/ThreadView.swift | sed -n '390,465p'
printf '%s\n' '--- RepliesView ---'
cat -n swiftchan/Views/Boards/Catalog/Thread/RepliesView.swift
printf '%s\n' '--- GalleryView structure and paging ---'
ast-grep outline swiftchan/Views/Media/Gallery/GalleryView.swift
rg -n -C 8 'galleryIndex|TabView|selection|presentingGallery|dismiss|onDisappear|onChange' swiftchan/Views/Media/Gallery/GalleryView.swift
printf '%s\n' '--- PostView media source and reply context ---'
cat -n swiftchan/Views/Boards/Catalog/Thread/PostView.swift | sed -n '1,90p'
cat -n swiftchan/Views/Boards/Catalog/Thread/PostView.swift | sed -n '145,190p'
printf '%s\n' '--- presentation state declarations and media mappings ---'
rg -n -C 5 'class PresentationState|struct PresentationState|galleryIndex|presentingIndex|postMediaMapping|media:' swiftchanRepository: vanities/swiftchan
Length of output: 47609
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- transition environment definitions ---'
cat -n swiftchan/Models/PresentationState.swift
rg -n -C 8 'galleryTransitionSource|inRepliesContext|galleryNamespace' swiftchan
printf '%s\n' '--- complete GalleryView state update path ---'
cat -n swiftchan/Views/Media/Gallery/GalleryView.swift | sed -n '130,205p'
printf '%s\n' '--- relevant navigation and cover modifiers ---'
rg -n -C 12 'NavigationStack|NavigationSplitView|navigationDestination|fullScreenCover|presentingReplies|presentingGallery' swiftchan/Views/Boards/Catalog/Thread swiftchan/Models
printf '%s\n' '--- repository changes near the reviewed code ---'
git diff --stat
git diff -- swiftchan/Views/Boards/Catalog/Thread/ThreadView.swift swiftchan/Views/Boards/Catalog/Thread/RepliesView.swift swiftchan/Views/Boards/Catalog/Thread/PostView.swiftRepository: vanities/swiftchan
Length of output: 39697
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
thread = Path("swiftchan/Views/Boards/Catalog/Thread/ThreadView.swift").read_text()
replies = Path("swiftchan/Views/Boards/Catalog/Thread/RepliesView.swift").read_text()
post = Path("swiftchan/Views/Boards/Catalog/Thread/PostView.swift").read_text()
gallery = Path("swiftchan/Views/Media/Gallery/GalleryView.swift").read_text()
checks = {
"gallery transition uses galleryIndex": ".zoom(sourceID: presentationState.galleryIndex" in thread,
"gallery paging updates galleryIndex": "state.galleryIndex = index" in gallery,
"thread scroll is disabled in replies": "if !presentationState.presentingReplies && !showReply" in thread,
"replies view has a ScrollView": "ScrollView(.vertical" in replies,
"replies view lacks a ScrollViewReader": "ScrollViewReader" not in replies,
"replies view lacks galleryIndex change handling": ".onChange(of: presentationState.galleryIndex)" not in replies,
"reply context owns the active source": "inRepliesContext == presentationState.presentingReplies" in post,
}
for name, result in checks.items():
print(f"{'PASS' if result else 'FAIL'}: {name}")
if not all(checks.values()):
raise SystemExit(1)
PYRepository: vanities/swiftchan
Length of output: 461
Keep the active reply thumbnail visible before dismissal.
When presentationState.presentingReplies is true, ThreadView does not call scrollToPost. RepliesView does not scroll when GalleryView changes presentationState.galleryIndex. If the user pages to media whose reply is outside the visible LazyVGrid range, the zoom transition has no rendered source.
Add a ScrollViewReader to RepliesView and scroll the reply that maps to the active media before dismissal.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@swiftchan/Views/Boards/Catalog/Thread/ThreadView.swift` around lines 131 -
135, Add a ScrollViewReader around the reply grid in RepliesView and, when
presentationState.presentingReplies is true, scroll to the reply corresponding
to presentationState.galleryIndex before GalleryView dismissal so its thumbnail
is rendered as the zoom source. Reuse the existing reply-to-media mapping and
scrollToPost behavior where applicable, without changing the non-replies flow.
Comment on lines
+120
to
+133
| private var closeButton: some View { | ||
| Button { | ||
| state.presentingGallery = false | ||
| } label: { | ||
| Image(systemName: "xmark.circle.fill") | ||
| .font(.system(size: 28)) | ||
| .symbolRenderingMode(.hierarchical) | ||
| .foregroundStyle(.white) | ||
| .shadow(radius: 4) | ||
| } | ||
| .padding(16) | ||
| .opacity(isZoomed ? 0 : 1) | ||
| .animation(.easeInOut(duration: 0.15), value: isZoomed) | ||
| .accessibilityIdentifier(AccessibilityIdentifiers.galleryCloseButton) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Disable the hidden close button.
When isZoomed is true, .opacity(0) hides the button but keeps its hit area and accessibility element active. Add .allowsHitTesting(!isZoomed) and .accessibilityHidden(isZoomed).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@swiftchan/Views/Media/Gallery/GalleryView.swift` around lines 120 - 133, Add
.allowsHitTesting(!isZoomed) and .accessibilityHidden(isZoomed) to the
closeButton view so it is neither interactive nor exposed to accessibility when
zoomed, while preserving its existing opacity and animation behavior.
6 tasks
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
1 participant
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
fullScreenCoverusing iOS 18's native zoom navigation transition (.matchedTransitionSource+.navigationTransition(.zoom)) instead of a sheet\u{200B}escapes so the swiftlintinvisible_characterautocorrect build phase can't strip themTesting
Closes #14. Supersedes #39.
https://claude.ai/code/session_01BzkuYBpheMbQn6EdRPT4wg
Summary by CodeRabbit