-
Notifications
You must be signed in to change notification settings - Fork 528
Add smooth exit animation to Image Details Panel #980
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e3e64a0
a9eb243
78d234d
44c1e17
e150827
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,57 @@ | ||||||||||||||||||||
| import { useEffect, useState } from 'react'; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| type ScrollToTopButtonProps = { | ||||||||||||||||||||
| scrollContainerRef?: React.RefObject<HTMLElement | null>; | ||||||||||||||||||||
| threshold?: number; | ||||||||||||||||||||
| }; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export const ScrollToTopButton = ({ | ||||||||||||||||||||
| scrollContainerRef, | ||||||||||||||||||||
| threshold = 300, | ||||||||||||||||||||
| }: ScrollToTopButtonProps) => { | ||||||||||||||||||||
| const [visible, setVisible] = useState(false); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||
| const scroller = scrollContainerRef?.current || window; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const handleScroll = () => { | ||||||||||||||||||||
| const scrollTop = | ||||||||||||||||||||
| scroller instanceof Window | ||||||||||||||||||||
| ? window.scrollY | ||||||||||||||||||||
| : scroller.scrollTop; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| setVisible(scrollTop > threshold); | ||||||||||||||||||||
| }; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| scroller.addEventListener('scroll', handleScroll); | ||||||||||||||||||||
| return () => scroller.removeEventListener('scroll', handleScroll); | ||||||||||||||||||||
| }, [scrollContainerRef, threshold]); | ||||||||||||||||||||
|
Comment on lines
+14
to
+28
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: cat frontend/tsconfig.json | jq '.compilerOptions | {strict, strictNullChecks, strictFunctionTypes}'Repository: AOSSIE-Org/PictoPy Length of output: 119 🏁 Script executed: cat -n frontend/src/components/ScrollToTopButton/ScrollToTopButton.tsxRepository: AOSSIE-Org/PictoPy Length of output: 1821 🏁 Script executed: cat frontend/tsconfig.jsonRepository: AOSSIE-Org/PictoPy Length of output: 804 🏁 Script executed: # Check the actual types being used - both Window and HTMLElement should inherit from EventTarget
# which has addEventListener and removeEventListener
rg -A 5 "addEventListener" frontend/src/components/ScrollToTopButton/ScrollToTopButton.tsxRepository: AOSSIE-Org/PictoPy Length of output: 307 Fix dependency array to track actual ref value changes. The dependency array should include The type safety concern about incompatible Update line 28 to: 🤖 Prompt for AI Agents |
||||||||||||||||||||
|
|
||||||||||||||||||||
| const scrollToTop = () => { | ||||||||||||||||||||
| const scroller = scrollContainerRef?.current; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (scroller) { | ||||||||||||||||||||
| scroller.scrollTo({ top: 0, behavior: 'smooth' }); | ||||||||||||||||||||
| } else { | ||||||||||||||||||||
| window.scrollTo({ top: 0, behavior: 'smooth' }); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| }; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (!visible) return null; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| return ( | ||||||||||||||||||||
| <button | ||||||||||||||||||||
| onClick={scrollToTop} | ||||||||||||||||||||
| aria-label="Scroll to top" | ||||||||||||||||||||
| className=" | ||||||||||||||||||||
| fixed top-35 right-15 z-50 | ||||||||||||||||||||
|
Comment on lines
+46
to
+47
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Find and examine Tailwind configuration
find . -type f \( -name "tailwind.config.*" -o -name "tailwind.config.js" -o -name "tailwind.config.ts" \) 2>/dev/nullRepository: AOSSIE-Org/PictoPy Length of output: 121 🏁 Script executed: # Look at the ScrollToTopButton component file
find . -path "*/frontend/src/components/ScrollToTopButton/ScrollToTopButton.tsx" -type fRepository: AOSSIE-Org/PictoPy Length of output: 127 🏁 Script executed: # Search for custom spacing definitions across config files
find . -type f \( -name "*.config.*" -o -name "postcss.config.*" \) | head -20Repository: AOSSIE-Org/PictoPy Length of output: 321 🏁 Script executed: # Read the ScrollToTopButton component
cat -n ./frontend/src/components/ScrollToTopButton/ScrollToTopButton.tsxRepository: AOSSIE-Org/PictoPy Length of output: 1821 🏁 Script executed: # Search for Tailwind config in frontend directory
find ./frontend -name "tailwind.config.*" -o -name "*tailwind*" -type f 2>/dev/null | grep -v node_modulesRepository: AOSSIE-Org/PictoPy Length of output: 44 🏁 Script executed: # Look for any CSS configuration or custom Tailwind setup in frontend
find ./frontend -type f \( -name "*.config.*" -o -name "vite.config.*" \) | xargs ls -laRepository: AOSSIE-Org/PictoPy Length of output: 285 🏁 Script executed: # Check if there's a root-level tailwind config
find . -maxdepth 2 -name "tailwind.config.*" -type fRepository: AOSSIE-Org/PictoPy Length of output: 121 🏁 Script executed: # Read the frontend vite.config to see if Tailwind is configured there
cat ./frontend/vite.config.tsRepository: AOSSIE-Org/PictoPy Length of output: 869 🏁 Script executed: # Check frontend package.json for Tailwind setup
cat ./frontend/package.json | grep -A 5 -B 5 tailwind || echo "Not found in package.json"Repository: AOSSIE-Org/PictoPy Length of output: 1169 🏁 Script executed: # Read one of the found tailwind configs to understand the structure
cat ./landing-page/tailwind.config.jsRepository: AOSSIE-Org/PictoPy Length of output: 1477 🏁 Script executed: # Search the entire frontend directory for any tailwind config file
find ./frontend -type f -name "tailwind.config.*"Repository: AOSSIE-Org/PictoPy Length of output: 44 🏁 Script executed: # Search for top-35 or right-15 definitions in the codebase
rg "top-35|right-15" --type-list | head -5
rg "top-35|right-15"Repository: AOSSIE-Org/PictoPy Length of output: 567 🏁 Script executed: # Check if there's a CSS file defining these classes
find ./frontend -type f \( -name "*.css" -o -name "*.scss" \) | head -20Repository: AOSSIE-Org/PictoPy Length of output: 84 🏁 Script executed: # Check the CSS file to see if custom classes are defined there
cat ./frontend/src/App.cssRepository: AOSSIE-Org/PictoPy Length of output: 7941 🏁 Script executed: # Also check for any tailwind directives or custom CSS in the main index/entry files
find ./frontend/src -maxdepth 1 -type f \( -name "*.css" -o -name "main.*" -o -name "index.*" \)Repository: AOSSIE-Org/PictoPy Length of output: 108 Replace non-standard Tailwind spacing classes with valid alternatives. The classes Use standard Tailwind spacing increments (0, 1, 2, 3, 4, 6, 8, 10, 12, 14, 16, 20, 24, 28, 32, 36, 40, 44, 48, 52, 56, 60, 64, 72, 80, 96) instead. 🎨 Example fix className="
- fixed top-35 right-15 z-50
+ fixed bottom-20 right-6 z-50
rounded-full bg-primary p-3
text-white shadow-lg
transition-all duration-300
hover:scale-110 hover:shadow-xl
"(Using 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
| rounded-full bg-primary p-3 | ||||||||||||||||||||
| text-white shadow-lg | ||||||||||||||||||||
| transition-all duration-300 | ||||||||||||||||||||
| hover:scale-110 hover:shadow-xl | ||||||||||||||||||||
| " | ||||||||||||||||||||
| > | ||||||||||||||||||||
| ↑ | ||||||||||||||||||||
| </button> | ||||||||||||||||||||
| ); | ||||||||||||||||||||
| }; | ||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Handle
showbecomingfalseto prevent stuck panel.The current effect only sets
isVisibletotruewhenshowbecomestrue, but doesn't handle the case whenshowbecomesfalse. This creates a critical desynchronization bug:Scenario: If the parent component sets
show={false}directly (without triggeringhandleClose), the panel will become stuck visible because:showbecomesfalse(from parent)isVisibleremainstrue(no effect updates it)nullwhen BOTH are false)isVisibleto control visibility classes)Impact: The panel cannot be closed if the parent updates
showdirectly, breaking the close functionality in certain usage patterns.🔎 Proposed fix
useEffect(() => { if (show) { setIsVisible(true); + } else { + // Animate out when parent sets show to false + setIsVisible(false); } }, [show]);Alternatively, if you want to preserve the exit animation when the parent closes directly:
useEffect(() => { if (show) { setIsVisible(true); + } else { + // Trigger exit animation, but let parent control final unmount + const timer = setTimeout(() => setIsVisible(false), 0); + return () => clearTimeout(timer); } }, [show]);📝 Committable suggestion
🤖 Prompt for AI Agents