Added a new widget for the footer and modified the globalcss, watchlater, archives, catalogs, and history tsx files #158
Conversation
…ter, archives, catalogs, and history tsx files
WalkthroughThe changes introduce a new responsive footer component named ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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: 4
🔭 Outside diff range comments (2)
src/views/explore/catalogs.tsx (1)
1-6: 🧹 Nitpick (assertive)Consider fixing the import order
The ESLint tool flagged an issue with import sorting. Consider organizing imports for better readability and to satisfy linting rules.
import fetchApi from "~/shared/lib/api/fetch"; +import FavoriteCatalog from "./favorite-catalogs"; import DetailsCard from "~/widgets/details-card"; import GridContainer from "~/widgets/grid-container"; +import MobileFooter from "~/widgets/mobile-footer"; -import FavoriteCatalog from "./favorite-catalogs"; -import MobileFooter from "~/widgets/mobile-footer";🧰 Tools
🪛 ESLint
[error] 1-6: Run autofix to sort these imports!
(simple-import-sort/imports)
src/views/explore/watch-later.tsx (1)
3-8: 🧹 Nitpick (assertive)Consider reorganizing imports
For better readability and maintainability, consider grouping related imports together.
"use client"; import { useLiveQuery } from "dexie-react-hooks"; -import MobileFooter from "~/widgets/mobile-footer"; import { indexedDB } from "~/shared/lib/api/fetch"; import GridContainer from "~/widgets/grid-container"; +import MobileFooter from "~/widgets/mobile-footer"; import YouTubeCard from "~/widgets/youtube/youtube-card";🧰 Tools
🪛 ESLint
[error] 3-7: Run autofix to sort these imports!
(simple-import-sort/imports)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
src/app/styles/globals.css(1 hunks)src/views/explore/archives.tsx(2 hunks)src/views/explore/catalogs.tsx(2 hunks)src/views/explore/watch-history.tsx(2 hunks)src/views/explore/watch-later.tsx(2 hunks)src/widgets/mobile-footer.tsx(1 hunks)
🧰 Additional context used
🪛 ESLint
src/views/explore/catalogs.tsx
[error] 1-6: Run autofix to sort these imports!
(simple-import-sort/imports)
src/widgets/mobile-footer.tsx
[error] 1-2: Run autofix to sort these imports!
(simple-import-sort/imports)
🔇 Additional comments (5)
src/views/explore/archives.tsx (1)
4-4: Good implementation of mobile footer!The MobileFooter component has been correctly imported and placed at the end of the JSX structure. This provides consistent navigation across the application's mobile view.
Also applies to: 31-31
src/views/explore/catalogs.tsx (1)
6-6: Good implementation of mobile footer!The MobileFooter component has been correctly imported and placed at the end of the JSX structure, maintaining consistency with other pages.
Also applies to: 35-35
🧰 Tools
🪛 ESLint
[error] 1-6: Run autofix to sort these imports!
(simple-import-sort/imports)
src/views/explore/watch-later.tsx (1)
4-4: Good implementation of mobile footer!The MobileFooter component has been correctly imported and placed at the end of the JSX structure, providing consistent navigation across views.
Also applies to: 35-35
src/views/explore/watch-history.tsx (1)
8-8: Good implementation of mobile footer!The MobileFooter component has been correctly imported and placed at the end of the JSX structure. The import is well-organized with other widget imports, and the placement is consistent with other views.
Also applies to: 37-37
src/app/styles/globals.css (1)
99-103: Good implementation of mobile-specific styles.The media query correctly targets mobile devices with a max-width of 768px and sets the mobile footer to display as flex. This is a clean approach to conditionally showing the mobile footer only on smaller screens.
| import Link from "next/link"; | ||
| import { Archive, BookOpen, Clock8, History } from "lucide-react"; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Fix import order.
The static analysis is flagging an issue with import order. It's generally good practice to sort imports for better code organization.
-import Link from "next/link";
-import { Archive, BookOpen, Clock8, History } from "lucide-react";
+import Link from "next/link";
+import { Archive, BookOpen, Clock8, History } from "lucide-react";You can run ESLint's autofix command to sort these imports automatically.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 ESLint
[error] 1-2: Run autofix to sort these imports!
(simple-import-sort/imports)
There was a problem hiding this comment.
@Sl33pySage Please run pnpm lint prior to pushing the changes to the remote, also the commit message doesn't follow https://www.conventionalcommits.org/en/v1.0.0/.
| export default function MobileFooter() { | ||
| return ( | ||
| <footer className="fixed bottom-0 w-full bg-white shadow-md flex justify-around items-center p-2 md:hidden"> | ||
| <Link href="/explore/catalogs"> | ||
| <div className="flex flex-col items-center"> | ||
| <BookOpen className="h-6 w-6" /> | ||
| <span>Catalogs</span> | ||
| </div> | ||
| </Link> | ||
| <Link href="/explore/archives"> | ||
| <div className="flex flex-col items-center"> | ||
| <Archive className="h-6 w-6" /> | ||
| <span>Archives</span> | ||
| </div> | ||
| </Link> | ||
| <Link href="/explore/watch-later"> | ||
| <div className="flex flex-col items-center"> | ||
| <Clock8 className="h-6 w-6" /> | ||
| <span>Watch Later</span> | ||
| </div> | ||
| </Link> | ||
| <Link href="/explore/history"> | ||
| <div className="flex flex-col items-center"> | ||
| <History className="h-6 w-6" /> | ||
| <span>History</span> | ||
| </div> | ||
| </Link> | ||
| </footer> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add the mobile-footer class for consistency with CSS.
You've defined a .mobile-footer class in the CSS file but haven't applied it to the component. This would ensure the component's display behavior is controlled by the CSS media query you defined.
-export default function MobileFooter() {
+export default function MobileFooter() {
return (
- <footer className="fixed bottom-0 w-full bg-white shadow-md flex justify-around items-center p-2 md:hidden">
+ <footer className="fixed bottom-0 w-full bg-white shadow-md justify-around items-center p-2 md:hidden mobile-footer">This change leverages the CSS media query you created and ensures consistency in how the component's visibility is managed.
Committable suggestion skipped: line range outside the PR's diff.
| <footer className="fixed bottom-0 w-full bg-white shadow-md flex justify-around items-center p-2 md:hidden"> | ||
| <Link href="/explore/catalogs"> | ||
| <div className="flex flex-col items-center"> | ||
| <BookOpen className="h-6 w-6" /> | ||
| <span>Catalogs</span> | ||
| </div> | ||
| </Link> | ||
| <Link href="/explore/archives"> | ||
| <div className="flex flex-col items-center"> | ||
| <Archive className="h-6 w-6" /> | ||
| <span>Archives</span> | ||
| </div> | ||
| </Link> | ||
| <Link href="/explore/watch-later"> | ||
| <div className="flex flex-col items-center"> | ||
| <Clock8 className="h-6 w-6" /> | ||
| <span>Watch Later</span> | ||
| </div> | ||
| </Link> | ||
| <Link href="/explore/history"> | ||
| <div className="flex flex-col items-center"> | ||
| <History className="h-6 w-6" /> | ||
| <span>History</span> | ||
| </div> | ||
| </Link> | ||
| </footer> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider improving accessibility and dark mode support.
The footer implementation looks good functionally, but there are a few opportunities for improvement:
- The component lacks accessibility attributes like
aria-labelfor the navigation links - The styling uses fixed colors (
bg-white) which won't adapt to dark mode - Missing active/current page indication for better user experience
Here's an improved implementation:
- <footer className="fixed bottom-0 w-full bg-white shadow-md flex justify-around items-center p-2 md:hidden">
+ <footer className="fixed bottom-0 w-full bg-background shadow-md flex justify-around items-center p-2 md:hidden mobile-footer">
<Link href="/explore/catalogs">
- <div className="flex flex-col items-center">
+ <div className="flex flex-col items-center" aria-label="Navigate to Catalogs">
<BookOpen className="h-6 w-6" />
<span>Catalogs</span>
</div>
</Link>
<Link href="/explore/archives">
- <div className="flex flex-col items-center">
+ <div className="flex flex-col items-center" aria-label="Navigate to Archives">
<Archive className="h-6 w-6" />
<span>Archives</span>
</div>
</Link>
<Link href="/explore/watch-later">
- <div className="flex flex-col items-center">
+ <div className="flex flex-col items-center" aria-label="Navigate to Watch Later">
<Clock8 className="h-6 w-6" />
<span>Watch Later</span>
</div>
</Link>
<Link href="/explore/history">
- <div className="flex flex-col items-center">
+ <div className="flex flex-col items-center" aria-label="Navigate to History">
<History className="h-6 w-6" />
<span>History</span>
</div>
</Link>
</footer>Additionally, consider adding active state indicators to show which page is currently active.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <footer className="fixed bottom-0 w-full bg-white shadow-md flex justify-around items-center p-2 md:hidden"> | |
| <Link href="/explore/catalogs"> | |
| <div className="flex flex-col items-center"> | |
| <BookOpen className="h-6 w-6" /> | |
| <span>Catalogs</span> | |
| </div> | |
| </Link> | |
| <Link href="/explore/archives"> | |
| <div className="flex flex-col items-center"> | |
| <Archive className="h-6 w-6" /> | |
| <span>Archives</span> | |
| </div> | |
| </Link> | |
| <Link href="/explore/watch-later"> | |
| <div className="flex flex-col items-center"> | |
| <Clock8 className="h-6 w-6" /> | |
| <span>Watch Later</span> | |
| </div> | |
| </Link> | |
| <Link href="/explore/history"> | |
| <div className="flex flex-col items-center"> | |
| <History className="h-6 w-6" /> | |
| <span>History</span> | |
| </div> | |
| </Link> | |
| </footer> | |
| <footer className="fixed bottom-0 w-full bg-background shadow-md flex justify-around items-center p-2 md:hidden mobile-footer"> | |
| <Link href="/explore/catalogs"> | |
| <div className="flex flex-col items-center" aria-label="Navigate to Catalogs"> | |
| <BookOpen className="h-6 w-6" /> | |
| <span>Catalogs</span> | |
| </div> | |
| </Link> | |
| <Link href="/explore/archives"> | |
| <div className="flex flex-col items-center" aria-label="Navigate to Archives"> | |
| <Archive className="h-6 w-6" /> | |
| <span>Archives</span> | |
| </div> | |
| </Link> | |
| <Link href="/explore/watch-later"> | |
| <div className="flex flex-col items-center" aria-label="Navigate to Watch Later"> | |
| <Clock8 className="h-6 w-6" /> | |
| <span>Watch Later</span> | |
| </div> | |
| </Link> | |
| <Link href="/explore/history"> | |
| <div className="flex flex-col items-center" aria-label="Navigate to History"> | |
| <History className="h-6 w-6" /> | |
| <span>History</span> | |
| </div> | |
| </Link> | |
| </footer> |
There was a problem hiding this comment.
@Sl33pySage This is how it currently looks,
.
I want this to look like YouTube Mobile footer. I believe the ticket #156 has a screenshot attached. Please refer to that.
The footer should be on these public routes (I should have updated the ticket title😅)
-
/explore, -
/explore/catalogs -
/explore/archives -
/explore/c/:id -
/explore/a/:id -
/explore/watch-later -
/explore/history
Also please address the points left by @coderabbitai.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the issue exists and get its details
gh issue view 156 --repo realChakrawarti/yt-catalog
Length of output: 489
🏁 Script executed:
#!/bin/bash
# Look for all relevant route files to check footer implementation
echo "Checking for route files..."
# Looking for /explore route implementation
echo "Searching for /explore route implementation:"
fd "explore.tsx" --type file
# Looking for /explore/c/:id route implementation
echo -e "\nSearching for /explore/c/:id route implementation:"
fd "c/\\[id\\].tsx" --type file || fd "c.tsx" --type file
# Looking for /explore/a/:id route implementation
echo -e "\nSearching for /explore/a/:id route implementation:"
fd "a/\\[id\\].tsx" --type file || fd "a.tsx" --type file
# Looking at the current mobile-footer implementation
echo -e "\nChecking mobile-footer.tsx content:"
cat src/widgets/mobile-footer.tsx
# Check if there's any active page indication implementation in the project
echo -e "\nLooking for active page indication examples in the project:"
rg "active|current.*page" --type ts --type tsx
Length of output: 1889
🏁 Script executed:
#!/bin/bash
# Explore the project structure to find route implementations
echo "Exploring project structure for route files..."
# Check app router structure (Next.js)
echo "Checking app router structure:"
find src/app -type d | sort
# Check pages router structure (Next.js)
echo -e "\nChecking pages router structure:"
find src/pages -type d 2>/dev/null | sort
# Look for explore-related components and routes
echo -e "\nLooking for explore-related files:"
find src -name "*explore*" -type f | sort
# Look specifically for the route components mentioned in the comment
echo -e "\nLooking for specific route components:"
find src -path "*explore*" -name "*.tsx" | grep -v "node_modules" | sort
# Check if there are dynamic route files for catalog and archive detail pages
echo -e "\nLooking for dynamic route files for catalog and archive details:"
find src -name "*[id]*" -type f | sort
# Look for existing UI components that might have active state indicators
echo -e "\nLooking for components with potential active state indicators:"
grep -r "isActive\|active" --include="*.tsx" src/ | head -15
Length of output: 8518
@realChakrawarti I've analyzed the current implementation and the requested changes for the mobile footer. Here's what needs to be done to match the YouTube mobile footer style and implement it across all the specified routes:
1. Improving the Mobile Footer Component
The current implementation needs updates to:
- Match YouTube's mobile footer style
- Add active state indicators
- Support dark mode
- Improve accessibility
- Implement on all specified routes
Here's an enhanced implementation:
import Link from "next/link";
import { usePathname } from "next/navigation";
import { Archive, BookOpen, Clock8, History } from "lucide-react";
import { cn } from "~/shared/lib/tailwind-merge";
export default function MobileFooter() {
const pathname = usePathname();
const isActive = (path: string) => {
if (path === "/explore" && pathname === "/explore") return true;
if (path !== "/explore" && pathname.startsWith(path)) return true;
return false;
};
return (
<footer className="fixed bottom-0 w-full bg-background border-t border-border shadow-sm flex justify-around items-center py-2 z-10 md:hidden mobile-footer">
<Link href="/explore" aria-label="Home">
<div className={cn("flex flex-col items-center", isActive("/explore") && "text-primary")}>
<BookOpen className="h-6 w-6" />
<span className="text-xs mt-1">Home</span>
</div>
</Link>
<Link href="/explore/catalogs" aria-label="Catalogs">
<div className={cn("flex flex-col items-center", isActive("/explore/catalogs") && "text-primary")}>
<BookOpen className="h-6 w-6" />
<span className="text-xs mt-1">Catalogs</span>
</div>
</Link>
<Link href="/explore/archives" aria-label="Archives">
<div className={cn("flex flex-col items-center", isActive("/explore/archives") && "text-primary")}>
<Archive className="h-6 w-6" />
<span className="text-xs mt-1">Archives</span>
</div>
</Link>
<Link href="/explore/watch-later" aria-label="Watch Later">
<div className={cn("flex flex-col items-center", isActive("/explore/watch-later") && "text-primary")}>
<Clock8 className="h-6 w-6" />
<span className="text-xs mt-1">Watch Later</span>
</div>
</Link>
<Link href="/explore/history" aria-label="History">
<div className={cn("flex flex-col items-center", isActive("/explore/history") && "text-primary")}>
<History className="h-6 w-6" />
<span className="text-xs mt-1">History</span>
</div>
</Link>
</footer>
);
}2. Implementing on All Required Routes
Looking at your project structure, you should include this footer in the following locations:
-
For the base routes (
/explore,/explore/catalogs, etc.):- Add it to
src/app/(views)/explore/layout.tsxto ensure it appears on all explore pages
- Add it to
-
For the dynamic routes (
/explore/c/:idand/explore/a/:id):- Add it to
src/app/(views)/(public-catalog)/c/[catalogId]/page.tsx - Add it to
src/app/(views)/(public-archive)/a/[archiveId]/page.tsx
- Add it to
Example implementation for a layout file:
import MobileFooter from "~/widgets/mobile-footer";
export default function ExploreLayout({ children }: { children: React.ReactNode }) {
return (
<div className="relative min-h-screen pb-16 md:pb-0">
{children}
<MobileFooter />
</div>
);
}Key Improvements:
-
Dark Mode Support:
- Changed
bg-whitetobg-backgroundto respect theme colors - Added
border-borderfor better contrast in dark mode
- Changed
-
Accessibility:
- Added
aria-labelattributes to navigation links - Improved text contrast with the active state
- Added
-
Active State Indication:
- Added
isActivefunction that highlights the current route - Used
text-primaryfor active links
- Added
-
YouTube-Style Footer:
- Smaller text below icons
- Border top instead of shadow (more like YouTube)
- Added z-index to ensure it stays on top
- More subtle shadow
-
Responsive Layout:
- Added padding to improve touch targets
- Kept the
md:hiddenclass to hide on larger screens
These changes should address all the requirements and make the footer match the YouTube mobile style while also improving accessibility and dark mode support.
| <Link href="/explore/catalogs"> | ||
| <div className="flex flex-col items-center"> | ||
| <BookOpen className="h-6 w-6" /> | ||
| <span>Catalogs</span> | ||
| </div> | ||
| </Link> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add hover and active states for better user experience.
The navigation links would benefit from visual feedback on hover and active states. Consider adding transition effects or color changes.
<Link href="/explore/catalogs">
- <div className="flex flex-col items-center">
+ <div className="flex flex-col items-center transition-colors hover:text-primary active:scale-95">
<BookOpen className="h-6 w-6" />
<span>Catalogs</span>
</div>Apply similar changes to the other navigation items for consistency.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Link href="/explore/catalogs"> | |
| <div className="flex flex-col items-center"> | |
| <BookOpen className="h-6 w-6" /> | |
| <span>Catalogs</span> | |
| </div> | |
| </Link> | |
| <Link href="/explore/catalogs"> | |
| <div className="flex flex-col items-center transition-colors hover:text-primary active:scale-95"> | |
| <BookOpen className="h-6 w-6" /> | |
| <span>Catalogs</span> | |
| </div> | |
| </Link> |
There was a problem hiding this comment.
@Sl33pySage Thank you for raising the PR. We are off to a great start 👏🏻. I have added few comments, please take a look at them.
If you have any question or query with regards to this ticket, please feel free drop a comment, would be more than happy to discuss on it.
Again, thank you for raising the PR. Really appreciate it. 🍻
There was a problem hiding this comment.
Could you please remove package-lock.json? In this project, we are using pnpm and the corresponding lockfile is pnpm-lock.yaml.
| import Link from "next/link"; | ||
| import { Archive, BookOpen, Clock8, History } from "lucide-react"; |
There was a problem hiding this comment.
@Sl33pySage Please run pnpm lint prior to pushing the changes to the remote, also the commit message doesn't follow https://www.conventionalcommits.org/en/v1.0.0/.
| <footer className="fixed bottom-0 w-full bg-white shadow-md flex justify-around items-center p-2 md:hidden"> | ||
| <Link href="/explore/catalogs"> | ||
| <div className="flex flex-col items-center"> | ||
| <BookOpen className="h-6 w-6" /> | ||
| <span>Catalogs</span> | ||
| </div> | ||
| </Link> | ||
| <Link href="/explore/archives"> | ||
| <div className="flex flex-col items-center"> | ||
| <Archive className="h-6 w-6" /> | ||
| <span>Archives</span> | ||
| </div> | ||
| </Link> | ||
| <Link href="/explore/watch-later"> | ||
| <div className="flex flex-col items-center"> | ||
| <Clock8 className="h-6 w-6" /> | ||
| <span>Watch Later</span> | ||
| </div> | ||
| </Link> | ||
| <Link href="/explore/history"> | ||
| <div className="flex flex-col items-center"> | ||
| <History className="h-6 w-6" /> | ||
| <span>History</span> | ||
| </div> | ||
| </Link> | ||
| </footer> |
There was a problem hiding this comment.
@Sl33pySage This is how it currently looks,
.
I want this to look like YouTube Mobile footer. I believe the ticket #156 has a screenshot attached. Please refer to that.
The footer should be on these public routes (I should have updated the ticket title😅)
-
/explore, -
/explore/catalogs -
/explore/archives -
/explore/c/:id -
/explore/a/:id -
/explore/watch-later -
/explore/history
Also please address the points left by @coderabbitai.
| @@ -95,3 +95,9 @@ | |||
| } | |||
|
|
|||
| @import url("./custom.css"); | |||
|
|
|||
There was a problem hiding this comment.
@Sl33pySage I feel this is redundant as same could be achieved using the tailwind utility classes. Please remove this.
I hope this is good! I'd love to have some feedback. Thank you for your time!
Summary by CodeRabbit