feat: introduce new 404 page design #418
Conversation
|
@techwithanirudh is attempting to deploy a commit to the Zero Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe Changes
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
apps/mail/app/layout.tsx (1)
21-21: Consider using absolute path for font sourceWhile the relative path works, it's generally better to use Next.js absolute paths with the
@/prefix for better maintainability.-const pixy = localFont({ variable: '--font-pixy', src: '../public/fonts/PIXY.woff2' }) +const pixy = localFont({ variable: '--font-pixy', src: '@/public/fonts/PIXY.woff2' })apps/mail/app/(error)/not-found.tsx (1)
30-35: Consider converting comment to Tailwind classesThe sizing explanation in the comment could be implemented directly with Tailwind classes for better maintainability.
apps/mail/app/(error)/_components/marquee.tsx (5)
39-45: Consider handling high-DPI or Retina rendering.Currently, the canvas dimensions are set to match the element's CSS size, which can lead to blurry text on high-DPI or Retina screens. A common approach is to multiply the canvas width/height by
window.devicePixelRatio(then scale the context accordingly) to ensure crisper rendering on high-density displays.
72-72: Verify custom font loading or provide fallback.The specified
"Pixy"font may not be installed on all systems. Ensure there's a proper @font-face definition for"Pixy"or confirm its presence to avoid layout shifts or a jarring fallback.
79-83: Check for off-by-one hover boundary cases.Hover detection relies on integer division of
(x - startX) / squareSize. Consider whether mouse positioning near square boundaries might cause unexpected flicker or off-by-one outcomes, and adjust accordingly if needed.
123-127: Optimize reduced motion behavior.When
useReducedMotionis true, the code still callsrequestAnimationFramein a continuous loop, even though the grid is static. Conditionally stopping the animation loop (or only recalling it on resize) can further reduce motion or CPU usage for users with reduced motion preferences.
129-129: Allow zero-speed mode if desired.The clamp to
0.1prevents the animation speed from ever being zero. If you want to support a fully static grid, allowspeed=0by removing or adjusting theMath.max(speed, 0.1)logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/mail/public/fonts/PIXY.woff2is excluded by!**/*.woff2
📒 Files selected for processing (5)
apps/mail/app/(error)/_components/marquee.tsx(1 hunks)apps/mail/app/(error)/not-found.tsx(1 hunks)apps/mail/app/globals.css(1 hunks)apps/mail/app/layout.tsx(3 hunks)apps/mail/tailwind.config.ts(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/mail/app/globals.css
[error] 26-26: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 29-29: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
🔇 Additional comments (10)
apps/mail/app/globals.css (1)
24-30: LGTM: Excellent implementation of a cross-browser drag prevention utility classThe
.drag-noneutility class is well-implemented with appropriate vendor-prefixed properties to ensure consistent behavior across different browsers. This will be useful for preventing drag interactions on elements like images in the 404 page.Don't worry about the static analysis warnings about unknown properties - these are false positives. The vendor-prefixed properties are necessary for maximum browser compatibility.
🧰 Tools
🪛 Biome (1.9.4)
[error] 26-26: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.(lint/correctness/noUnknownProperty)
[error] 29-29: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.(lint/correctness/noUnknownProperty)
apps/mail/app/layout.tsx (2)
9-9: LGTM: Good use of Next.js font importCorrectly importing the local font module.
32-32: LGTM: Proper application of the new font variableThe pixel font is correctly added to the body className alongside existing fonts.
apps/mail/tailwind.config.ts (2)
9-9: LGTM: Correct dark mode configurationThe
darkMode: ["class"]setting ensures that dark mode is controlled via CSS classes, which is the recommended approach for Next.js applications that use theme providers.
85-85: LGTM: Well-configured font family for the pixel fontProper configuration of the
pixelfont family that includes thePixyfont with appropriate fallbacks.apps/mail/app/(error)/not-found.tsx (5)
3-9: LGTM: Clean imports for the updated 404 pageAppropriate imports for the new design elements, including the custom icons for both light and dark themes and the new Marquee component.
14-20: Excellent navigation handling with fallbackThe
back()function intelligently determines whether to use browser history or redirect to the home page, providing a better user experience than a simple redirect.
23-26: LGTM: Good use of the Marquee component as a background effectThe Marquee component is properly positioned with appropriate opacity to create a subtle animated background effect.
36-55: LGTM: Excellent implementation of the 404 displayThe 404 display with the creative use of the Zero logo as the "0" in "404" is visually appealing and well-implemented. The responsive sizing with viewport units ensures it scales appropriately across different screen sizes.
Good use of the
drag-noneutility class to prevent image dragging, and the hover animation adds a nice interactive touch.
58-65: LGTM: Clean and concise error message with intuitive navigationThe error message is clear and the navigation button is appropriately styled and positioned.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/mail/app/(error)/_components/marquee.tsx (1)
130-130: Consider allowing zero speed.
Currently, speed is clamped to a minimum of 0.1, which might prevent users from disabling animation if they want a stationary grid.- const effectiveSpeed = Math.max(speed, 0.1); + const effectiveSpeed = speed <= 0 ? 0 : speed;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/mail/app/(error)/_components/marquee.tsx(1 hunks)apps/mail/app/(error)/_components/marquee.tsx(6 hunks)
🔇 Additional comments (16)
apps/mail/app/(error)/_components/marquee.tsx (16)
2-2: No issues detected.
7-7: Renaming the interface to "GridPosition" is clear and more intuitive.
29-33: Good approach using refs to store animation state, grid offset, and hovered square.
37-37: No issues detected.
39-39: No issues detected.
61-80: Well-structured square-drawing function.
The modular approach to applying transformations, hover effects, and text rendering keeps the code clean and readable.
82-98: Vignette effect implementation is elegant.
Creating a radial gradient for the final overlay adds visual depth.
99-122: Refactored grid drawing logic is organized and reusable.
Utilizing the newdrawSquarefunction clarifies the main loop, and the consistent calculations for offsets help maintain smooth animation.
151-151: No issues detected.
162-165: Potential negative index issue.
If the mouse hovers near the top or left edges, the computed hovered square might be negative. While this likely won’t break drawing logic (because strict equality checks prevent matching invalid squares), consider bounding values for clarity or verifying they don’t cause unexpected behavior.
168-171: Resize handler is well-implemented.
Automatically redrawing the grid on window resize improves responsiveness.
174-176: Event listeners successfully register.
Attaching resize and mousemove event handlers is correct and reflects the intended behavior.
180-184: Conditional rendering for reduced motion is appropriate.
Opting out of animation for users preferring reduced motion is an excellent accessibility practice.
187-195: Cleanup approach is mostly correct.
References for animation and primary event handlers are properly removed, preventing memory leaks, aside from the inline arrow function issue noted above.
197-197: No issues detected.
202-205: Tailwind class usage and ARIA attributes greatly enhance usability.
The.block h-full w-fullclass ensures full coverage, androle="img"witharia-labelimproves accessibility by describing the decorative purpose of the canvas.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
apps/mail/app/(error)/_components/marquee.tsx (2)
92-97:⚠️ Potential issueFix misplaced gradient code
There appears to be duplicate or misplaced code in the
drawVignettefunction. Lines 93-95 seem to be out of place and might be causing rendering issues.gradient.addColorStop(0, colors.vignetteStart); - canvas.height / 2, - Math.sqrt(canvas.width ** 2 + canvas.height ** 2) / 2, - ); + gradient.addColorStop(1, colors.vignetteEnd); + ctx.fillStyle = gradient; ctx.fillRect(0, 0, canvas.width, canvas.height);
129-147:⚠️ Potential issueRemove duplicate speed calculation and direction handling
There appears to be duplicate code for the
effectiveSpeedcalculation and direction switch statement.const effectiveSpeed = Math.max(speed, 0.1); switch (direction) { case "right": gridOffset.current.x = (gridOffset.current.x - effectiveSpeed + squareSize) % squareSize; - const effectiveSpeed = Math.max(speed, 0.1); - - switch (direction) { - case "right": - gridOffset.current.x = (gridOffset.current.x - effectiveSpeed + squareSize) % squareSize; break; case "left": gridOffset.current.x = (gridOffset.current.x + effectiveSpeed + squareSize) % squareSize; break; case "up": gridOffset.current.y = (gridOffset.current.y + effectiveSpeed + squareSize) % squareSize; - gridOffset.current.x = (gridOffset.current.x - effectiveSpeed + squareSize) % squareSize; - gridOffset.current.y = (gridOffset.current.y - effectiveSpeed + squareSize) % squareSize; break; + case "down": + gridOffset.current.y = (gridOffset.current.y - effectiveSpeed + squareSize) % squareSize; + break; + case "diagonal": + gridOffset.current.x = (gridOffset.current.x - effectiveSpeed + squareSize) % squareSize; + gridOffset.current.y = (gridOffset.current.y - effectiveSpeed + squareSize) % squareSize; + break; }
♻️ Duplicate comments (1)
apps/mail/app/(error)/_components/marquee.tsx (1)
176-178:⚠️ Potential issueInline arrow function cannot be removed in cleanup
Because the arrow function passed to "mouseleave" is anonymous,
removeEventListenerin cleanup won't remove the same reference, leading to a stale listener or memory leak.Apply this diff to fix the issue:
+ const handleMouseLeave = () => { + hoveredSquare.current = null; + }; - canvas.addEventListener("mouseleave", () => { - hoveredSquare.current = null; - }); + canvas.addEventListener("mouseleave", handleMouseLeave); return () => { window.removeEventListener("resize", handleResize); canvas.removeEventListener("mousemove", handleMouseMove); - canvas.removeEventListener("mouseleave", () => { - hoveredSquare.current = null; - }); + canvas.removeEventListener("mouseleave", handleMouseLeave);Also applies to: 189-191
🧹 Nitpick comments (1)
apps/mail/app/(error)/_components/marquee.tsx (1)
154-157: Consider optimizing mouseMove handlerThe current implementation might cause unnecessary repaints because
drawGrid()is called immediately, followed by requesting an animation frame that will calldrawGrid()again.const handleMouseMove = (event: MouseEvent) => { - drawGrid(); animationRef.current = requestAnimationFrame(updateAnimation); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/mail/app/(error)/_components/marquee.tsx(1 hunks)apps/mail/app/(error)/_components/marquee.tsx(6 hunks)apps/mail/app/(error)/_components/marquee.tsx(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/mail/app/(error)/_components/marquee.tsx
🔇 Additional comments (9)
apps/mail/app/(error)/_components/marquee.tsx (9)
7-7: Improved interface namingGood job renaming
GridOffsettoGridPosition- this better reflects the purpose of the interface as it represents a point's position in the grid rather than an offset.
28-32: Better state management with proper refsNice improvement to the state management structure:
- Adding
animationReffor tracking animation frames- Using
gridDimensionsto track width and height together- Consistent naming with
GridPositioninterface
44-47: Well-structured grid dimensions calculationGood approach storing both width and height dimensions together in a single ref object. This makes the code more maintainable and easier to understand.
61-80: Great modularization with drawSquare functionExcellent code organization by extracting the square drawing logic into a separate function. This improves readability and maintainability by:
- Breaking down complex drawing operations
- Encapsulating hover state handling
- Following the single responsibility principle
168-172: Good implementation of handleResizeNice addition of a dedicated
handleResizefunction that correctly handles canvas resizing and redrawing when the window size changes.
180-184: Good conditional animation handlingNice implementation of the reduced motion preference - you're correctly using
shouldReduceMotionto determine whether to animate the grid or draw it statically.
193-195: Proper animation cleanupGood job adding proper cleanup for the animation frame by checking and canceling the request when the component unmounts, which prevents memory leaks.
197-197: Complete dependency arrayWell done including all necessary dependencies in the useEffect dependency array, which ensures the effect properly re-runs when props change.
202-202: Clean class namingGood use of Tailwind utility classes for styling the canvas with a consistent naming pattern.
|
We're working with the design team to introduce a brand identity and elements and this might not match, thank you but closing for now |
Type of Change
Areas Affected
Testing Done
Describe the tests you've done:
Checklist
Additional Notes
Add any other context about the pull request here.
Screenshots/Recordings
Dark:
Light:
By submitting this pull request, I confirm that my contribution is made under the terms of the project's license.
Summary by CodeRabbit
New Features
Style
Bug Fixes