Новая анимация карусели #7
Conversation
📝 WalkthroughWalkthroughThe logo carousel component was refactored from a dual CSS keyframe animation approach to a single-track design driven by JavaScript. The animation now uses Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks✅ Passed checks (3 passed)
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: 1
🧹 Nitpick comments (2)
frontend/.vitepress/theme/components/LogoCarousel.vue (2)
40-41: Consider making the repetition logic more explicit.The 6 repetitions create 30 logos, which works with the reset logic that divides
scrollWidth / 2. However, this coupling is implicit. IfbaseLogoschanges significantly in count, the repetition factor might need adjustment.💡 Optional: Make repetition count explicit
-// Repeat enough times to fill and loop seamlessly -const allLogos = [...baseLogos, ...baseLogos, ...baseLogos, ...baseLogos, ...baseLogos, ...baseLogos]; +// Repeat enough times to fill and loop seamlessly +// We need an even number of repetitions for the reset logic to work correctly +const REPETITIONS = 6; +const allLogos = Array(REPETITIONS).fill(baseLogos).flat();This makes it clearer that the repetition count matters for the looping logic and makes it easier to adjust if needed.
43-47: Animation state setup looks good.The reactive references and animation tracking variables are properly initialized.
You could optionally make the
speedconfigurable as a prop or constant at the top of the file if you anticipate needing to adjust it, but the current hardcoded value is reasonable.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/.vitepress/theme/components/LogoCarousel.vue
🔇 Additional comments (4)
frontend/.vitepress/theme/components/LogoCarousel.vue (4)
6-17: LGTM! Clean template structure with proper optimizations.The template correctly binds the reactive offset, handles dark mode variants, and includes appropriate security attributes (
rel="noopener noreferrer") and performance optimizations (loading="lazy").
67-75: Excellent lifecycle management!The animation is properly initialized on mount and cleaned up on unmount, preventing memory leaks from orphaned animation frames.
112-112: Proper performance optimization.Using
will-change: transformon the animated track is appropriate and helps the browser optimize rendering for the continuous animation.
162-162: Good responsive adjustment.Increasing the gap on mobile improves readability and touch target spacing.
| const animate = (currentTime) => { | ||
| if (!lastTime) lastTime = currentTime; | ||
| const delta = currentTime - lastTime; | ||
| lastTime = currentTime; | ||
| offset.value -= (speed * delta) / 1000; | ||
| // Reset when half of the track has scrolled | ||
| if (trackRef.value) { | ||
| const trackWidth = trackRef.value.scrollWidth / 2; | ||
| if (Math.abs(offset.value) >= trackWidth) { | ||
| offset.value = 0; | ||
| } | ||
| } | ||
| animationId = requestAnimationFrame(animate); | ||
| }; |
There was a problem hiding this comment.
Add accessibility support for reduced motion preference.
The animation logic is well-implemented with proper delta time calculations and seamless looping. However, it doesn't respect the user's prefers-reduced-motion setting, which is an accessibility concern.
🔎 Suggested fix to respect prefers-reduced-motion
Add a check for the reduced motion preference:
+const prefersReducedMotion = ref(false);
+
const animate = (currentTime) => {
+ if (prefersReducedMotion.value) {
+ animationId = requestAnimationFrame(animate);
+ return;
+ }
+
if (!lastTime) lastTime = currentTime;
const delta = currentTime - lastTime;
lastTime = currentTime;
offset.value -= (speed * delta) / 1000;
// Reset when half of the track has scrolled
if (trackRef.value) {
const trackWidth = trackRef.value.scrollWidth / 2;
if (Math.abs(offset.value) >= trackWidth) {
offset.value = 0;
}
}
animationId = requestAnimationFrame(animate);
};
onMounted(() => {
+ const mediaQuery = window.matchMedia('(prefers-reduced-motion: reduce)');
+ prefersReducedMotion.value = mediaQuery.matches;
+ mediaQuery.addEventListener('change', (e) => {
+ prefersReducedMotion.value = e.matches;
+ });
+
animationId = requestAnimationFrame(animate);
});Additionally, consider adding pause-on-hover for better UX, allowing users to interact with logos more easily:
💡 Optional: Add pause-on-hover functionality
+const isPaused = ref(false);
+
const animate = (currentTime) => {
+ if (isPaused.value || prefersReducedMotion.value) {
animationId = requestAnimationFrame(animate);
return;
}
if (!lastTime) lastTime = currentTime;
// ... rest of animation logic
};Then in the template:
-<div class="logo-carousel">
+<div class="logo-carousel" @mouseenter="isPaused = true" @mouseleave="isPaused = false">Committable suggestion skipped: line range outside the PR's diff.
1 participant
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.