Page transitions, shared-element morph, skeletons and scroll reveal - #10
Conversation
|
Warning Review limit reached
Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe change adds media tile hover states, loading skeletons, scroll reveals, and View Transition animations. It adds shared media hero ownership for recording and show navigation, plus Inertia handling for eligible full-page GET visits. ChangesMedia experience transitions
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant RecordingTile
participant Inertia
participant RecordingPlayer
User->>RecordingTile: pointer interaction
RecordingTile->>RecordingTile: claim media hero
RecordingTile->>Inertia: prefetch and navigate
Inertia->>RecordingPlayer: complete full-page GET visit
RecordingPlayer->>RecordingPlayer: apply media hero transition
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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 `@resources/js/Components/Recordings/RecordingTile.vue`:
- Line 9: Update the activation handlers in
resources/js/Components/Recordings/RecordingTile.vue at lines 9-9 and
resources/js/Components/Shows/ShowTile.vue at lines 14-14 to claim thumbnail on
Enter keyboard activation, while preserving the existing pointerdown behavior
and isPending guard.
In `@resources/js/Pages/Archive/Index.vue`:
- Around line 71-81: Update the collection-art image flow around loadedArt and
TilePlaceholder to handle load failures: track failed artwork by collection.year
with an `@error` handler, then render TilePlaceholder when the image errors so the
media-skeleton is not shown indefinitely. Preserve the existing successful-load
behavior.
In `@resources/js/Pages/RecordingPlayer.vue`:
- Around line 28-30: Update StreamPlayer.vue to render through a single native
root element so inherited attributes such as v-media-hero are applied correctly.
Ensure both direct root branches are contained within that element; the
VideoPlayer usage in resources/js/Pages/RecordingPlayer.vue:28-30 and
resources/js/Pages/ShowPlayer.vue:510-511 require no direct changes because they
are corrected by this root-wrapper fix.
🪄 Autofix (Beta)
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: 93e513b3-2b84-4bce-b955-872e78b4ff6c
📒 Files selected for processing (10)
resources/css/app.cssresources/js/Components/Recordings/RecordingTile.vueresources/js/Components/Shows/ShowTile.vueresources/js/Pages/Archive/Index.vueresources/js/Pages/RecordingPlayer.vueresources/js/Pages/Schedule.vueresources/js/Pages/ShowPlayer.vueresources/js/app.jsresources/js/composables/useMediaHero.jsresources/js/viewTransitions.js
| <VideoPlayer | ||
| :key="playerKey" | ||
| v-media-hero |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline resources/js/Components/Player/VideoPlayer.vue --items all
ast-grep outline resources/js/Components/Livestream/StreamPlayer.vue --items all
sed -n '/<template>/,/<\/template>/p' resources/js/Components/Player/VideoPlayer.vue
sed -n '/<template>/,/<\/template>/p' resources/js/Components/Livestream/StreamPlayer.vueRepository: Thiritin/streaming
Length of output: 2456
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Vue versions ---'
rg -n '"vue"|"`@vue/compiler`|vue/compiler' package.json package-lock.json yarn.lock pnpm-lock.yaml 2>/dev/null || true
printf '%s\n' '--- Component template root structure ---'
python3 - <<'PY'
from pathlib import Path
import re
files = [
Path("resources/js/Components/Player/VideoPlayer.vue"),
Path("resources/js/Components/Livestream/StreamPlayer.vue"),
]
tag_re = re.compile(r"<(/?)([A-Za-z][\w.-]*)(?:\s[^<>]*?)?(/?)>")
void = {"area","base","br","col","embed","hr","img","input","link","meta","param","source","track","wbr"}
for path in files:
text = path.read_text()
template = re.search(r"<template>(.*?)</template>", text, re.S).group(1)
roots = []
depth = 0
for m in tag_re.finditer(template):
closing, name, self_close = m.groups()
if name in {"template"}:
continue
if closing:
depth -= 1
elif depth == 0:
roots.append((name, "self-closing" if self_close or name.lower() in void else "element"))
if not closing and not self_close and name.lower() not in void:
depth += 1
print(f"{path}: top-level native nodes = {roots}")
PY
printf '%s\n' '--- Exact templates ---'
sed -n '/<template>/,/<\/template>/p' resources/js/Components/Player/VideoPlayer.vue
sed -n '/<template>/,/<\/template>/p' resources/js/Components/Livestream/StreamPlayer.vueRepository: Thiritin/streaming
Length of output: 5247
🌐 Web query:
Vue 3 fallthrough attributes component root v-if v-else single root directive on component
💡 Result:
In Vue 3, the automatic application of fallthrough attributes depends on the component having a single root node [1][2]. When a component renders a single root element, Vue automatically applies any attributes or event listeners not declared as props or emits to that root element [1][3]. However, if a component contains multiple root nodes, automatic fallthrough is disabled, and Vue will issue a runtime warning because it cannot determine which element should inherit the attributes [1][2]. In such cases, you must explicitly bind the attributes using v-bind="$attrs" to a specific element [1][2]. Regarding the interaction with v-if and v-else: - Structure as a single root: If your component has a single container wrapping the v-if/v-else logic (e.g., a single div surrounding the conditional branches), Vue treats it as a single-root component, and fallthrough attributes will be applied to that container [1][4]. - Structure as multiple roots: If the v-if and v-else elements are the direct children of the component (making them multiple root nodes), automatic attribute inheritance will not occur, and you will receive a warning [1][5]. - Note on Comments/Nodes: In certain scenarios, injecting comments or text nodes between conditional branches at the root level can cause Vue to interpret the fragment incorrectly, potentially affecting how fallthrough attributes are identified [5][6]. It is generally recommended to use a single wrapper element if you rely on automatic fallthrough behavior or, if using multiple roots, to explicitly handle attributes using inheritAttrs: false and v-bind="$attrs" [1][5]. If you wish to apply attributes to a specific element within a single-root component rather than the root itself, you must set inheritAttrs: false in your component options to disable the default automatic inheritance, then apply the attributes manually using v-bind="$attrs" on the desired target [1][4][3]. Failure to disable inheritAttrs while also using v-bind="$attrs" will result in attributes being applied to both the root element and the specific element you targeted [4].
Citations:
- 1: https://vuejs.org/guide/components/attrs
- 2: https://vue-doc.vercel.app/components-in-depth/fallthrough/
- 3: https://lukes.tips/posts/vue-3-fallthough-attributes-changes/
- 4: https://stackoverflow.com/questions/79680208/why-are-fallthrough-attributes-applied-to-both-the-root-element-and-the-one-you
- 5: Annotation nodes under the component root will affect “Fallthrough Attributes” vuejs/core#10087
- 6: fix(compiler-ssr): handle ssr attr fallthrough when preserve whitespace vuejs/core#12304
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if [ -d node_modules/@vue/compiler-sfc ]; then
node - <<'JS'
const { compileTemplate } = require('`@vue/compiler-sfc`')
for (const source of [
'<template><div v-if="ok"></div><div v-else></div></template>',
'<template><div v-if="ok"><span /></div><div v-else><p /></div></template>',
]) {
const result = compileTemplate({ source, filename: 'Probe.vue' })
console.log(result.code)
console.error(result.errors)
}
JS
else
echo 'node_modules/@vue/compiler-sfc is unavailable; compiler probe not run'
fi
printf '%s\n' '--- Vue compiler/runtime source references in tracked files ---'
rg -n 'fall.?through|single root|multiple root|root node|INSTANCE_ATTRS' resources package.json package-lock.json 2>/dev/null || trueRepository: Thiritin/streaming
Length of output: 418
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if [ -d node_modules/@vue/compiler-sfc ]; then
node - <<'JS'
const { compileTemplate } = require('`@vue/compiler-sfc`')
for (const source of [
'<template><div v-if="ok"></div><div v-else></div></template>',
'<template><div v-if="ok"><span /></div><div v-else><p /></div></template>',
]) {
const result = compileTemplate({ source, filename: 'Probe.vue' })
console.log(result.code)
console.error(result.errors)
}
JS
else
echo 'node_modules/@vue/compiler-sfc is unavailable; compiler probe not run'
fiRepository: Thiritin/streaming
Length of output: 226
Wrap StreamPlayer in one native root element. VideoPlayer.vue already has one root. StreamPlayer.vue has two direct root branches, so v-media-hero does not fall through to either branch.
📍 Affects 2 files
resources/js/Pages/RecordingPlayer.vue#L28-L30(this comment)resources/js/Pages/ShowPlayer.vue#L510-L511
🤖 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 `@resources/js/Pages/RecordingPlayer.vue` around lines 28 - 30, Update
StreamPlayer.vue to render through a single native root element so inherited
attributes such as v-media-hero are applied correctly. Ensure both direct root
branches are contained within that element; the VideoPlayer usage in
resources/js/Pages/RecordingPlayer.vue:28-30 and
resources/js/Pages/ShowPlayer.vue:510-511 require no direct changes because they
are corrected by this root-wrapper fix.
1 participant
Builds on the motion tokens already on
main(--dur-*,--ease-*, thetileTransitionGroup, the reduced-motion gate) and adds the pieces that were left over:
page transitions, a shared-element morph from tile to player, thumbnail skeletons,
scroll reveal, and a hover lift.
Page transitions
resources/js/viewTransitions.jswires the View Transitions API into the Inertiarouter. Inertia has no "about to swap" hook, so the transition is held open across
the request and closed on
finish, by which point the new page is in the DOM. TheDOM still mutates normally while it is held; only painting is suppressed.
Three guards make that safe:
MAX_FREEZE_MS = 600. While a transition is pending the browser suppressespaint, and that includes Inertia's own progress bar. Past the cap the transition
is released un-animated so a slow visit degrades to a plain swap instead of
looking hung.
preserveStateand partial visits are skipped.Components/Manage/useTableQuery.jsfires a
router.geton every filter change and every debounced keystroke. Asnapshot over the page each time would make typing feel broken. Same reasoning
for the archive search.
document.startViewTransitionor underprefers-reduced-motion.Root crossfade plus an 8px rise at
--dur-base.Tiles and archive collection cards now use Inertia
prefetch. That is what makesthe transition worth having: the response is usually already cached when the click
lands, so the morph starts immediately rather than after a round trip spent frozen.
Shared element (tile to player)
view-transition-namehas to be unique document-wide, andShowPlayerrendersboth a player and a sidebar of tiles, so ownership lives in one module
(
composables/useMediaHero.js) where claiming always strips the name off whoeverheld it last.
pointerdown, before Inertia starts the visit.v-media-herodirective onmountedandupdated—Inertia reuses the player component between two show pages, so
mountedonlyfires once, and a sidebar tile takes the name away when it is clicked.
Worth noting for review: releasing the name from a
router.on('finish')handlerdoes not work. It strips the name before the browser captures the new page, so the
morph silently does not happen. The directive's ownership model replaced that.
Thumbnail skeletons
.media-skeletonreuses the archive's existingpending-sweepgradient, so"image still loading" and "recording still processing" read as one family rather
than two separate inventions. It shows while a lazy thumbnail has not decoded yet,
which is a real state now that tile images are lazy — previously those slots were
flat
bg-primary-800boxes. Applied in both tiles and the archive collection art.Scroll reveal
CSS-only, via
animation-timeline: view(). No IntersectionObserver, nothingrunning on the scroll path. Behind
@supports(Firefox has no view timeline yet,where the rule simply never applies) and
prefers-reduced-motion: no-preference.Applied to schedule rows and archive collection cards, and deliberately not to
.stream-gridtiles: those already animate in through<TransitionGroup>, and twosystems animating one element fight each other.
Tile hover
.media-tilescales to 1.03 and takesposition: relative; z-index: 20so thehovered card sits over its neighbours.
:focus-withinis included so keyboardusers get the same affordance. Pending archive tiles are excluded, since they are
not clickable.
Testing
vite buildpasses and the generated CSS was checked for the new rules. Notruntime-verified — the browser profile was in use, so the transitions and the
morph have not been driven in a real page yet. Worth an eyeball on:
Summary by CodeRabbit