Skip to content

Fixups! #32

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.

Sign up for GitHub

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

Merged
clates merged 1 commit into from
Dec 28, 2025
Merged

Fixups! #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions components/add-sighting-drawer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,6 @@ export function AddSightingDrawer() {
) : (
<div className="space-y-6 text-center animate-in zoom-in-95 duration-500 py-2">
<BookStickerInstruction code={generatedCode} coverUrl={selectedBook?.coverUrl} />
Comment on lines 237 to 239

Copilot AI Dec 28, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of the "Register another book" button leaves the resetState function (defined at line 137) unused and orphaned. Users who want to register multiple books in one session now must close and reopen the drawer, but the generated code and selected book will persist since there's no effect that resets state when the drawer closes. Consider either: 1) Adding a useEffect to reset state when the drawer closes (when open changes to false), or 2) Keeping a reset button if the expected UX is to allow registering multiple books without closing the drawer.

Copilot uses AI. Check for mistakes.

<Button variant="outline" onClick={resetState} className="mt-2">
Register another book
</Button>
</div>
)}
</ParchmentFrame>
Expand Down
48 changes: 38 additions & 10 deletions components/book-sticker-instruction.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,17 @@ interface BookStickerInstructionProps {
code: string
coverUrl?: string
className?: string
animationSpeedMultiplier?: number

Copilot AI Dec 28, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The animationSpeedMultiplier prop is declared in the interface but never destructured or used in the component. Consider removing it if it's not needed, or implement the functionality to use this prop to scale animation timings.

Suggested change
animationSpeedMultiplier?: number

Copilot uses AI. Check for mistakes.
}

// Animation Configuration
const ANIMATION_BASE_DURATION_S = 2.5
// Derived timings
const ANIMATION_CONFIG = {
duration: ANIMATION_BASE_DURATION_S,
openDelay: ANIMATION_BASE_DURATION_S * 0.3, // ~0.5s if base is 2.5

Copilot AI Dec 28, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states "~0.5s if base is 2.5" but the actual calculation is 2.5 * 0.3 = 0.75 seconds, not 0.5 seconds. Update the comment to reflect the correct value.

Suggested change
openDelay: ANIMATION_BASE_DURATION_S * 0.3, // ~0.5s if base is 2.5
openDelay: ANIMATION_BASE_DURATION_S * 0.3, // ~0.75s if base is 2.5

Copilot uses AI. Check for mistakes.
urlWriteDelay: ANIMATION_BASE_DURATION_S * 0.28, // Starts shortly after page flip starts? No, originally 0.2 which is small.
codeWriteDelay: 1.0, // 1.2 - 0.2, relative to URL finishing
}

const TypewriterText = ({
Expand Down Expand Up @@ -76,20 +87,24 @@ const TypewriterText = ({
export function BookStickerInstruction({ code, coverUrl, className }: BookStickerInstructionProps) {
const [copied, setCopied] = useState(false)
const [isOpen, setIsOpen] = useState(false)
const [aspectRatio, setAspectRatio] = useState(1 / 1.3)

// Refined plan: Use a ref or state for style.
const [dynamicAspectRatio, setDynamicAspectRatio] = useState(1.3)
Comment on lines +90 to +93

Copilot AI Dec 28, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two unused state variables are declared: aspectRatio (line 90) and dynamicAspectRatio (line 93). The setDynamicAspectRatio is called in the image onLoad handler but the value is never read. Either use these variables to control the aspect ratio of the container or image, or remove them entirely.

Suggested change
const [aspectRatio, setAspectRatio] = useState(1 / 1.3)
// Refined plan: Use a ref or state for style.
const [dynamicAspectRatio, setDynamicAspectRatio] = useState(1.3)
// Refined plan: Use a ref or state for style.
const [, setDynamicAspectRatio] = useState(1.3)

Copilot uses AI. Check for mistakes.

useEffect(() => {
// Open the book shortly after mount
const timer = setTimeout(() => {
setIsOpen(true)
}, 500)
}, ANIMATION_CONFIG.openDelay * 1000)

return () => clearTimeout(timer)
}, [])

useEffect(() => {
if (isOpen) {
// Trigger confetti when book opens and ink dries (approx 2s delay total relative to open)
// Let's time it with the code writing completion
const confettiDelayMs = (ANIMATION_CONFIG.duration + 1.0) * 1000

const confettiTimer = setTimeout(() => {
const end = Date.now() + 1000
const colors = ["#a8e6cf", "#dcedc1", "#ffd3b6", "#ffaaa5", "#ff8b94"]
Expand All @@ -114,7 +129,7 @@ export function BookStickerInstruction({ code, coverUrl, className }: BookSticke
requestAnimationFrame(frame)
}
})()
}, 2500) // Delay to match writing animation
}, confettiDelayMs)

return () => clearTimeout(confettiTimer)
}
Expand All @@ -135,7 +150,7 @@ export function BookStickerInstruction({ code, coverUrl, className }: BookSticke
<div className={cn("relative w-full max-w-md mx-auto p-4 perspective-[1200px]", className)}>
<div className="flex gap-2 items-center justify-center relative">
{/* The 3D Book Container */}
<div className="relative w-full aspect-[1.3] mt-4 mb-2 max-w-[300px]">
<div className="relative w-auto h-[275px] mt-4 mb-2 max-w-[300px] aspect-[0.66]">

Copilot AI Dec 28, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The class combination w-auto with aspect-[0.66] creates a conflict. The aspect-[0.66] utility requires a width to calculate height (or vice versa), but w-auto means the width is determined by content. With a fixed height of h-[275px], the aspect ratio class won't work as intended. Consider using only h-[275px] and removing both w-auto and aspect-[0.66], or restructure to use aspect ratio with a defined width.

Suggested change
<div className="relative w-auto h-[275px] mt-4 mb-2 max-w-[300px] aspect-[0.66]">
<div className="relative h-[275px] mt-4 mb-2 max-w-[300px] aspect-[0.66]">

Copilot uses AI. Check for mistakes.
{/* The Book Itself */}
<motion.div
className="relative w-full h-full preserve-3d origin-left"
Expand All @@ -144,7 +159,7 @@ export function BookStickerInstruction({ code, coverUrl, className }: BookSticke
rotateY: isOpen ? -10 : 0,
x: isOpen ? "100%" : "0%",
}}
transition={{ duration: 1.5, ease: "easeInOut" }}
transition={{ duration: ANIMATION_CONFIG.duration, ease: "easeInOut" }}
>
{/* RIGHT PAGE (The "Book Block" that stays underneath) */}
<div className="absolute inset-0 w-full h-full bg-[#fdfbf6] rounded-r-md shadow-lg border-l border-stone-200">
Expand All @@ -163,7 +178,12 @@ export function BookStickerInstruction({ code, coverUrl, className }: BookSticke
animate={{
rotateY: isOpen ? -180 : 0,
}}
transition={{ duration: 1.5, type: "spring", stiffness: 40, damping: 12 }}
transition={{
duration: ANIMATION_CONFIG.duration,
type: "spring",
stiffness: 40,
damping: 12,
}}
onUpdate={(latest) => {
// Track rotation to toggle visibility state at 90 degrees (halfway)
if (typeof latest.rotateY === "number") {
Expand All @@ -184,7 +204,13 @@ export function BookStickerInstruction({ code, coverUrl, className }: BookSticke
<img
src={coverUrl}
alt="Book Cover"
className="w-full h-full opacity-90 mix-blend-overlay"
className="h-full opacity-90 mix-blend-overlay aspect-2/3"

Copilot AI Dec 28, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The class aspect-2/3 is not a standard Tailwind CSS class. Tailwind uses bracket notation for arbitrary aspect ratios. Change this to aspect-[2/3] to properly apply the 2:3 aspect ratio.

Suggested change
className="h-full opacity-90 mix-blend-overlay aspect-2/3"
className="h-full opacity-90 mix-blend-overlay aspect-[2/3]"

Copilot uses AI. Check for mistakes.
onLoad={(e) => {
const img = e.currentTarget
if (img.naturalWidth && img.naturalHeight) {
setDynamicAspectRatio(img.naturalWidth / img.naturalHeight)
}
}}
/>
) : (
<div className="w-full h-full flex items-center justify-center bg-amber-700 text-amber-100 p-6 text-center">
Expand Down Expand Up @@ -221,7 +247,7 @@ export function BookStickerInstruction({ code, coverUrl, className }: BookSticke
<div className="h-4 flex items-center justify-center">
<TypewriterText
text="TaleTrail.org"
delay={0.2} // Reduced delay since we wait for the page flip now
delay={ANIMATION_CONFIG.urlWriteDelay}
className="font-handwriting font-bold text-amber-800 tracking-wide text-xl"
/>
</div>
Expand All @@ -234,7 +260,9 @@ export function BookStickerInstruction({ code, coverUrl, className }: BookSticke
<div className="font-handwriting text-2xl font-bold text-stone-800 tracking-widest border-2 border-dashed border-stone-300 rounded px-3 py-1 bg-white/50 min-w-[160px] min-h-[48px] flex items-center justify-center">
<TypewriterText
text={code}
delay={1.2} // Relative to the URL finishing
delay={
ANIMATION_CONFIG.urlWriteDelay + ANIMATION_CONFIG.codeWriteDelay
}
/>
</div>

Expand Down