Fixing Highlights Display on Home page and create highlights page#2
Fixing Highlights Display on Home page and create highlights page#2caporalCode wants to merge 4 commits intomainfrom
Conversation
WalkthroughThe changes introduce a new highlights page with paginated video highlights, including new types, server and client components, and a highlights grid with modal video playback. API client pagination parameters are flattened for consistency. News and highlights components are refactored for improved structure, image handling, and styling. Additional CSS rules and formatting updates are applied. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HighlightsPage (Server)
participant APIClient
participant HighlightsClient (Client)
participant HighlightsGrid
User->>HighlightsPage: Request /highlights?page=N
HighlightsPage->>APIClient: Fetch highlights (page=N, size=9)
APIClient-->>HighlightsPage: Return paginated highlights data
HighlightsPage->>HighlightsClient: Render with initial data, page, size
User->>HighlightsClient: Navigate pages
HighlightsClient->>APIClient: Fetch highlights for new page
APIClient-->>HighlightsClient: Return new highlights data
HighlightsClient->>HighlightsGrid: Render highlights
User->>HighlightsGrid: Click play on video
HighlightsGrid->>HighlightsGrid: Open modal with video player
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 6
♻️ Duplicate comments (1)
components/Highlights/HighlightsClient.tsx (1)
20-28: Use shared utility for highlight filtering.This is the third duplicate of the same filtering logic. Implement the shared utility suggested in the
page.tsxreview.
🧹 Nitpick comments (7)
app/globals.css (1)
170-221: Well-structured content styling for sanitized HTML.The new
.dom-purify-contentclass provides comprehensive styling for rich HTML content with proper spacing, typography, and visual hierarchy. The hardcoded color values could be replaced with CSS custom properties for better theme consistency.Consider replacing hardcoded colors with CSS custom properties:
.dom-purify-content h2 { margin-top: 2rem; margin-bottom: 1rem; font-size: 1.875rem; font-weight: 700; - border-bottom: 1px solid #e5e7eb; + border-bottom: 1px solid var(--border); padding-bottom: 0.25rem; } .dom-purify-content blockquote { - border-left: 4px solid #3b82f6; + border-left: 4px solid var(--primary); padding-left: 1rem; font-style: italic; - color: #6b7280; + color: var(--muted-foreground); margin: 1.5rem 0; }app/highlights/types.ts (1)
3-3: Consider making PAGE_SIZE configurable.The hardcoded
PAGE_SIZE = 9works for the current implementation, but consider making it configurable through environment variables or a configuration file for better flexibility.-export const PAGE_SIZE = 9 +export const PAGE_SIZE = parseInt(process.env.NEXT_PUBLIC_HIGHLIGHTS_PAGE_SIZE || '9', 10)components/Home/PointsForts.tsx (1)
15-16: Remove redundant type casting.The
apiClient.highlights.findAll()already returnsPageHighlightDto, so the explicit cast is unnecessary.const response = await apiClient.highlights.findAll() - const data = response as PageHighlightDto + const data = responsecomponents/Highlights/HighlightsGrid.tsx (3)
64-70: Consider adding iframe sandbox attribute for security.While the current implementation works, adding a sandbox attribute with specific permissions would enhance security.
<iframe src={`${selectedVideo}?autoplay=1`} title="YouTube video player" allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture" allowFullScreen + sandbox="allow-scripts allow-same-origin allow-presentation" className="w-full h-full" ></iframe>
146-151: Remove duplicate type badge display.The highlight type is displayed twice - once on the thumbnail and again in the meta info section. Consider keeping only one instance for cleaner UI.
Remove one of the type badge displays to avoid redundancy. The thumbnail badge (lines 146-151) provides better visual hierarchy.
Also applies to: 178-182
244-249: Remove duplicate type badge for consistency.Similar to the featured card, the type badge is displayed twice. Consider keeping only the thumbnail badge for visual consistency across both card types.
Also applies to: 274-277
components/Highlights/HighlightsClient.tsx (1)
60-60: Fix French typography.Missing apostrophe in the French text.
- Il ny a actuellement aucun point fort avec des vidéos à afficher. + Il n'y a actuellement aucun point fort avec des vidéos à afficher.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
app/api/generated/types.gen.tsis excluded by!**/generated/**package-lock.jsonis excluded by!**/package-lock.jsonyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (13)
app/api/api-client.ts(5 hunks)app/api/types.ts(1 hunks)app/globals.css(3 hunks)app/highlights/page.tsx(1 hunks)app/highlights/types.ts(1 hunks)app/news/[id]/page.tsx(2 hunks)app/news/page.tsx(3 hunks)components/Highlights/HighlightsClient.tsx(1 hunks)components/Highlights/HighlightsGrid.tsx(1 hunks)components/Home/LiveScores.tsx(1 hunks)components/Home/PointsForts.tsx(2 hunks)components/Home/PointsFortsClient.tsx(1 hunks)components/Sections/News.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
app/highlights/types.ts (1)
app/api/generated/types.gen.ts (1)
HighlightDto(79-89)
app/api/api-client.ts (2)
app/api/generated/sdk.gen.ts (1)
listHighlights(665-676)app/api/generated/types.gen.ts (1)
PageHighlightDto(412-424)
components/Highlights/HighlightsGrid.tsx (2)
app/api/generated/types.gen.ts (1)
HighlightDto(79-89)lib/utils.ts (2)
isYouTubeUrl(34-36)getYouTubeEmbedUrl(38-49)
components/Home/PointsForts.tsx (2)
app/api/api-client.ts (1)
apiClient(25-231)app/api/generated/types.gen.ts (1)
PageHighlightDto(412-424)
🪛 Biome (1.9.4)
app/news/[id]/page.tsx
[error] 117-118: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (25)
app/globals.css (2)
7-38: CSS variable formatting improvements look good.The decimal precision normalization (removing trailing zeros) and consistent formatting improve code readability without affecting functionality.
45-54: Shadow variable multi-line formatting enhances readability.The reformatting of shadow variables to multi-line declarations with consistent spacing makes the CSS more maintainable.
app/news/page.tsx (3)
27-37: Breadcrumb localization and styling improvements.The French localization ("Accueil" and "Actualités") and added top margin improve the user experience for French-speaking users.
69-69: Improved image container with proper constraints.The addition of
w-fullandmax-h-80provides better responsive behavior and prevents images from becoming too large.
110-110: Minor formatting improvement.The added blank line improves code readability.
components/Sections/News.tsx (3)
11-32: Excellent UX improvement with fully clickable cards.Wrapping the entire card in a Link component improves user experience by making the whole card clickable, which is more intuitive than just having the title clickable.
15-22: Improved image handling with Next.js best practices.Using
fillprop with relative positioning provides better responsive behavior compared to fixed dimensions.
30-30: Better text truncation with CSS line-clamp.Using
line-clamp-2provides more consistent and visually appealing text truncation compared to manual string truncation.app/news/[id]/page.tsx (4)
16-36: Variable renaming improves code clarity.Renaming
latestNewstocurrentNewsbetter reflects the variable's purpose in the context of an individual news article page.
45-81: Excellent breadcrumb accessibility improvements.The structured data markup with schema.org and proper ARIA attributes significantly improve accessibility and SEO.
99-106: Semantic HTML improvement with figure element.Using
<figure>for images is semantically correct and improves accessibility. The responsive image handling withfillprop is also well implemented.
182-189: Consistent image handling with semantic markup.The use of
<figure>elements andfillprop maintains consistency with other image implementations in the codebase.app/api/api-client.ts (3)
15-15: Import addition looks good.The
PageHighlightDtoimport is properly added to support the new return type for highlights pagination.
65-65: Consistent pagination parameter flattening.The spread operator approach for flattening pagination parameters is applied consistently across fixtures and leagues endpoints, which improves API consistency.
Also applies to: 202-202
133-133: All existing consumers now handle the full pagination objectI’ve verified every call to
apiClient.highlights.findAll()is followed by casting toPageHighlightDtoand accessingdata.content(e.g. inPointsForts.tsx,HighlightsClient.tsx,app/highlights/page.tsx). There are no remaining consumers treating the return value as a raw array. This change won’t break existing code.components/Home/LiveScores.tsx (1)
51-52: Good defensive programming practice.The guard clause prevents rendering when fixtures data is empty or unavailable, which improves the user experience by avoiding empty state displays.
app/api/types.ts (1)
34-35: Minor formatting improvement.The additional blank lines improve readability by providing clear separation between type declarations.
components/Home/PointsFortsClient.tsx (1)
48-48: Improved semantic routing.The link destination change from
/galleryto/highlightsprovides better semantic meaning and aligns with the new highlights page implementation.app/highlights/types.ts (2)
1-1: Proper type import.The import of
HighlightDtofrom the generated API types ensures type safety and consistency with the API contract.
5-10: Well-structured interface design.The
HighlightInitialPageDatainterface properly encapsulates the paginated highlights data structure with pagination metadata and error handling, which aligns with the API client changes.components/Home/PointsForts.tsx (2)
5-5: LGTM!The import aligns with the API client's updated return type.
18-25: Good defensive programming!The conditional check ensures safe access to
data.contentand prevents runtime errors.components/Highlights/HighlightsGrid.tsx (2)
19-29: Good URL validation!The YouTube URL check prevents attempting to embed non-YouTube videos, which is a good security practice.
287-291: Clean duration formatting implementation!The function correctly formats video duration with proper zero-padding for seconds.
app/highlights/page.tsx (1)
52-59: Correct async params handling!Properly handles the Promise-based searchParams in Next.js 15+.
| <figure className="relative h-20 max-h-20 w-24"> | ||
| <Image | ||
| fill | ||
| src={article.imageUrl ?? "/ln-icon.svg"} | ||
| alt={article.title ?? ""} | ||
| className="h-full w-full rounded object-cover" | ||
| /> | ||
| </figure> | ||
|
|
||
| <div> | ||
| <h3 className='font-medium line-clamp-2'> | ||
| <h3 className="font-medium line-clamp-2"> | ||
| <Link | ||
| href={`/news/${article.id}`} | ||
| className='hover:text-blue-600' | ||
| className="hover:text-blue-600" | ||
| > | ||
| {article.title} | ||
| </Link> | ||
| </h3> | ||
| <p className='mt-1 text-sm text-gray-500'> | ||
| <p className="mt-1 text-sm text-gray-500"> | ||
| {formatDate(article.createdAt ?? new Date())} | ||
| </p> | ||
| </div> | ||
| </div> | ||
| </Link> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improved sidebar article accessibility.
Making the entire article container clickable improves user experience, though there's a redundant nested Link element that should be removed.
Remove the redundant nested Link element:
<Link
href={`/news/${article.id}`}
key={article.id}
className={`flex gap-4 ${index < recentArticles.length - 1 ? "border-b border-gray-100 pb-4" : ""}`}
>
<figure className="relative h-20 max-h-20 w-24">
<Image
fill
src={article.imageUrl ?? "/ln-icon.svg"}
alt={article.title ?? ""}
className="h-full w-full rounded object-cover"
/>
</figure>
<div>
<h3 className="font-medium line-clamp-2">
- <Link
- href={`/news/${article.id}`}
- className="hover:text-blue-600"
- >
- {article.title}
- </Link>
+ <span className="hover:text-blue-600">
+ {article.title}
+ </span>
</h3>
<p className="mt-1 text-sm text-gray-500">
{formatDate(article.createdAt ?? new Date())}
</p>
</div>
</Link>🤖 Prompt for AI Agents
In app/news/[id]/page.tsx between lines 138 and 160, the article container is
wrapped in a Link component, but there is a redundant nested Link around the
article title. Remove the inner Link component wrapping the article title and
keep only the outer Link to make the entire container clickable and avoid nested
links for better accessibility.
| <div className="prose max-w-none dom-purify-content"> | ||
| <div | ||
| dangerouslySetInnerHTML={{ | ||
| __html: DOMPurify.sanitize(latestNews.content ?? ''), | ||
| __html: DOMPurify.sanitize(currentNews.content ?? ""), | ||
| }} | ||
| ></div> | ||
| </div> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify DOMPurify configuration for security.
While DOMPurify is being used to sanitize HTML content, ensure it's configured with appropriate security settings for your use case.
Let me verify the DOMPurify configuration and check if additional security measures are needed:
🏁 Script executed:
#!/bin/bash
# Check DOMPurify import and usage patterns
rg -A 5 -B 5 "DOMPurify" --type tsx --type ts
# Check for any custom DOMPurify configuration
ast-grep --pattern 'DOMPurify.sanitize($_, $_)'Length of output: 123
I didn’t find any DOMPurify imports or custom configurations with the previous search. Let’s try a more targeted scan across .ts and .tsx files:
🏁 Script executed:
#!/bin/bash
# Locate how DOMPurify is imported
rg -n "import.*DOMPurify" -g "*.ts" -g "*.tsx"
# Find all sanitize calls (with or without options)
rg -n "DOMPurify\.sanitize" -g "*.ts" -g "*.tsx"
# Check for server-side setup via createDOMPurify
rg -n "createDOMPurify" -g "*.ts" -g "*.tsx"Length of output: 472
Configure DOMPurify with explicit security options
It looks like you’re importing and using DOMPurify.sanitize(...) only with default settings—no custom config or profiles are applied. To ensure you’re protected against XSS in both browser and server environments, explicitly set or pass a strict configuration:
• Files needing updates:
- app/news/page.tsx (line 89)
- app/news/[id]/page.tsx (line 118)
• Example of enforcing a safe profile and stripping any unexpected attributes:
// utils/sanitize.ts
import DOMPurify from 'isomorphic-dompurify';
const purifyConfig = {
USE_PROFILES: { html: true }, // allow only standard HTML tags
ALLOWED_ATTR: ['href', 'src', 'alt'], // tighten attributes list
FORBID_ATTR: ['style', 'onerror', 'onclick'],
};
export function sanitize(html: string) {
return DOMPurify.sanitize(html, purifyConfig);
}Then update your pages to use the wrapper:
- __html: DOMPurify.sanitize(currentNews.content ?? "")
+ __html: sanitize(currentNews.content ?? "")This ensures consistency across rendering contexts and locks down the default–open configuration.
🧰 Tools
🪛 Biome (1.9.4)
[error] 117-118: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🤖 Prompt for AI Agents
In app/news/[id]/page.tsx around lines 115 to 121, the DOMPurify.sanitize call
uses default settings without explicit security options, which may leave XSS
risks. Create a utility function that imports DOMPurify from
'isomorphic-dompurify' and applies a strict configuration object specifying
allowed HTML profiles and attributes, forbidding risky attributes like style and
event handlers. Replace the direct DOMPurify.sanitize call with this wrapper
function to ensure consistent and secure sanitization across browser and server
environments.
| async findAll(pageable: Pageable = DEFAULT_PAGEABLE) { | ||
| try { | ||
| const { data, error } = await listHighlights({ query: { pageable } }) | ||
| const { data, error } = await listHighlights({ query: pageable }) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify API endpoint compatibility for direct pageable queries.
The change from { pageable } to direct pageable for highlights and advertisements assumes the API endpoints expect flattened pagination parameters. Ensure this aligns with the actual API contract.
Also applies to: 160-160
🏁 Script executed:
#!/bin/bash
# Description: Check API endpoint usage patterns to verify pagination parameter structure
# Expected: Find consistent usage of either nested or flattened pagination parameters
# Search for listHighlights and getLatestAdvertisements usage patterns
echo "=== Checking listHighlights usage ==="
rg -A 5 -B 5 "listHighlights"
echo "=== Checking getLatestAdvertisements usage ==="
rg -A 5 -B 5 "getLatestAdvertisements"
echo "=== Checking other API call patterns for pagination ==="
rg -A 3 "query.*pageable|pageable.*query"Length of output: 129395
Fix API calls to nest the pageable object under the query key
The OpenAPI spec defines a single query parameter named pageable (an object), so passing its fields directly will not bind to that parameter. Update both calls in app/api/api-client.ts to wrap pageable in an object:
• Lines around 127 (highlights):
- const { data, error } = await listHighlights({ query: pageable })
+ const { data, error } = await listHighlights({ query: { pageable } })• Lines around 160 (advertisements):
- const { data, error } = await getLatestAdvertisements({ query: pageable })
+ const { data, error } = await getLatestAdvertisements({ query: { pageable } })This ensures the client emits ?pageable[page]=…&pageable[size]=… (deep-object style) as the API expects.
📝 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.
| const { data, error } = await listHighlights({ query: pageable }) | |
| // app/api/api-client.ts | |
| // … around line 127 | |
| - const { data, error } = await listHighlights({ query: pageable }) | |
| + const { data, error } = await listHighlights({ query: { pageable } }) | |
| // … around line 160 | |
| - const { data, error } = await getLatestAdvertisements({ query: pageable }) | |
| + const { data, error } = await getLatestAdvertisements({ query: { pageable } }) |
🤖 Prompt for AI Agents
In app/api/api-client.ts at line 127 and around line 160, the API calls
incorrectly pass the pageable object fields directly instead of nesting them
under the query key. To fix this, wrap the pageable object inside another object
with the key 'pageable' when passing it to the listHighlights and
listAdvertisements functions. This change will ensure the query parameters are
serialized as pageable[page]=...&pageable[size]=..., matching the OpenAPI spec
requirements.
| function filterAndSortHighlights(content: HighlightDto[] = []): HighlightDto[] { | ||
| return content | ||
| .filter((highlight) => highlight.videoUrl) | ||
| .sort((a, b) => { | ||
| const dateA = a.createdAt ? new Date(a.createdAt) : new Date(0); | ||
| const dateB = b.createdAt ? new Date(b.createdAt) : new Date(0); | ||
| return dateB.getTime() - dateA.getTime(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract duplicated filtering logic to a shared utility.
This function duplicates the same logic found in PointsForts.tsx and HighlightsClient.tsx. Consider extracting it to a shared utility file.
Create a shared utility function in a new file like app/highlights/utils.ts:
import type { HighlightDto } from "@/app/api/generated";
export function filterAndSortHighlights(content: HighlightDto[] = []): HighlightDto[] {
return content
.filter((highlight) => highlight.videoUrl)
.sort((a, b) => {
const dateA = a.createdAt ? new Date(a.createdAt) : new Date(0);
const dateB = b.createdAt ? new Date(b.createdAt) : new Date(0);
return dateB.getTime() - dateA.getTime();
});
}Then import and use it in all three files.
🤖 Prompt for AI Agents
In app/highlights/page.tsx around lines 15 to 23, the filterAndSortHighlights
function duplicates logic also present in PointsForts.tsx and
HighlightsClient.tsx. Extract this function into a new shared utility file, for
example app/highlights/utils.ts, and export it from there. Then replace the
local definitions in all three files with imports from this new utility to avoid
duplication and centralize the logic.
| // function Highlights() { | ||
| // const [currentPage, setCurrentPage] = useState(0); | ||
| // const [loading, setLoading] = useState(true); | ||
| // const [highlights, setHighlights] = useState<HighlightDto[]>([]); | ||
| // const [totalPages, setTotalPages] = useState(0); | ||
| // const [totalElements, setTotalElements] = useState(0); | ||
| // const [error, setError] = useState<string | null>(null); | ||
|
|
||
| // const pageSize = 9; | ||
|
|
||
| // useEffect(() => { | ||
| // fetchHighlights(currentPage); | ||
| // }, [currentPage]); | ||
|
|
||
| // const fetchHighlights = async (page: number) => { | ||
| // try { | ||
| // setLoading(true); | ||
| // setError(null); | ||
|
|
||
| // const response = await apiClient.highlights.findAll({ | ||
| // page: page, | ||
| // size: pageSize, | ||
| // }); | ||
| // const data = response as PageHighlightDto; | ||
|
|
||
| // // Filter highlights with video URLs and sort by creation date | ||
| // const filteredHighlights: HighlightDto[] = data?.content | ||
| // ? data.content | ||
| // .filter((highlight: HighlightDto) => highlight.videoUrl) | ||
| // .sort((a: HighlightDto, b: HighlightDto) => { | ||
| // const dateA = a.createdAt ? new Date(a.createdAt) : new Date(0); | ||
| // const dateB = b.createdAt ? new Date(b.createdAt) : new Date(0); | ||
| // return dateB.getTime() - dateA.getTime(); | ||
| // }) | ||
| // : []; | ||
|
|
||
| // setHighlights(filteredHighlights); | ||
| // setTotalPages(data?.totalPages || 0); | ||
| // setTotalElements(data?.totalElements || 0); | ||
| // } catch (err) { | ||
| // setError("Une erreur est survenue lors du chargement des points forts."); | ||
| // console.error("Error fetching highlights:", err); | ||
| // } finally { | ||
| // setLoading(false); | ||
| // } | ||
| // }; | ||
|
|
||
| // const handlePageChange = (newPage: number) => { | ||
| // if (newPage >= 0 && newPage < totalPages) { | ||
| // setCurrentPage(newPage); | ||
| // window.scrollTo({ top: 0, behavior: "smooth" }); | ||
| // } | ||
| // }; | ||
|
|
||
| // if (loading) { | ||
| // return <HighlightsSkeleton />; | ||
| // } | ||
|
|
||
| // if (error) { | ||
| // return ( | ||
| // <div className="text-center py-12"> | ||
| // <div className="bg-red-50 border border-red-200 rounded-lg p-6 max-w-md mx-auto"> | ||
| // <h3 className="text-red-800 font-semibold mb-2">Erreur</h3> | ||
| // <p className="text-red-600">{error}</p> | ||
| // <button | ||
| // onClick={() => fetchHighlights(currentPage)} | ||
| // className="mt-4 px-4 py-2 bg-red-600 text-white rounded hover:bg-red-700 transition-colors" | ||
| // > | ||
| // Réessayer | ||
| // </button> | ||
| // </div> | ||
| // </div> | ||
| // ); | ||
| // } | ||
|
|
||
| // if (highlights.length === 0) { | ||
| // return ( | ||
| // <div className="text-center py-16"> | ||
| // <div className="max-w-md mx-auto"> | ||
| // <div className="w-20 h-20 bg-gray-100 rounded-full flex items-center justify-center mx-auto mb-6"> | ||
| // <svg | ||
| // className="w-10 h-10 text-gray-400" | ||
| // fill="none" | ||
| // stroke="currentColor" | ||
| // viewBox="0 0 24 24" | ||
| // > | ||
| // <path | ||
| // strokeLinecap="round" | ||
| // strokeLinejoin="round" | ||
| // strokeWidth={2} | ||
| // d="M15 10l4.553-2.276A1 1 0 0121 8.618v6.764a1 1 0 01-1.447.894L15 14M5 18h8a2 2 0 002-2V8a2 2 0 00-2-2H5a2 2 0 00-2 2v8a2 2 0 002 2z" | ||
| // /> | ||
| // </svg> | ||
| // </div> | ||
| // <h3 className="text-xl font-semibold text-gray-900 mb-2"> | ||
| // Aucun point fort disponible | ||
| // </h3> | ||
| // <p className="text-gray-600"> | ||
| // Il ny a actuellement aucun point fort avec des vidéos à afficher. | ||
| // </p> | ||
| // </div> | ||
| // </div> | ||
| // ); | ||
| // } | ||
|
|
||
| // return ( | ||
| // <div> | ||
| // <div className="mb-6"> | ||
| // <h1 className="text-2xl font-bold text-gray-900 mb-2">Points forts</h1> | ||
| // <p className="text-gray-600"> | ||
| // {totalElements} {totalElements === 1 ? "point fort" : "points forts"} | ||
| // {totalElements > 0 && ` • Page ${currentPage + 1} sur ${totalPages}`} | ||
| // </p> | ||
| // </div> | ||
|
|
||
| // <HighlightsGrid highlights={highlights} /> | ||
|
|
||
| // {totalPages > 1 && ( | ||
| // <div className="mt-12 flex items-center justify-center space-x-2"> | ||
| // <button | ||
| // onClick={() => handlePageChange(currentPage - 1)} | ||
| // disabled={currentPage === 0} | ||
| // className={`flex items-center px-3 py-2 rounded-md text-sm font-medium transition-colors ${ | ||
| // currentPage === 0 | ||
| // ? "bg-gray-100 text-gray-400 cursor-not-allowed" | ||
| // : "bg-white text-gray-700 hover:bg-gray-50 border border-gray-300" | ||
| // }`} | ||
| // > | ||
| // <ChevronLeft className="h-4 w-4 mr-1" /> | ||
| // Précédent | ||
| // </button> | ||
|
|
||
| // <div className="flex items-center space-x-1"> | ||
| // {Array.from({ length: totalPages }, (_, i) => ( | ||
| // <button | ||
| // key={i} | ||
| // onClick={() => handlePageChange(i)} | ||
| // className={`px-3 py-2 rounded-md text-sm font-medium transition-colors ${ | ||
| // i === currentPage | ||
| // ? "bg-blue-600 text-white" | ||
| // : "bg-white text-gray-700 hover:bg-gray-50 border border-gray-300" | ||
| // }`} | ||
| // > | ||
| // {i + 1} | ||
| // </button> | ||
| // ))} | ||
| // </div> | ||
|
|
||
| // <button | ||
| // onClick={() => handlePageChange(currentPage + 1)} | ||
| // disabled={currentPage === totalPages - 1} | ||
| // className={`flex items-center px-3 py-2 rounded-md text-sm font-medium transition-colors ${ | ||
| // currentPage === totalPages - 1 | ||
| // ? "bg-gray-100 text-gray-400 cursor-not-allowed" | ||
| // : "bg-white text-gray-700 hover:bg-gray-50 border border-gray-300" | ||
| // }`} | ||
| // > | ||
| // Suivant | ||
| // <ChevronRightIcon className="h-4 w-4 ml-1" /> | ||
| // </button> | ||
| // </div> | ||
| // )} | ||
| // </div> | ||
| // ); | ||
| // } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove commented legacy implementation.
This large block of commented code should be removed. Version control preserves the history if this code needs to be referenced later.
Delete lines 91-255 to keep the codebase clean.
🤖 Prompt for AI Agents
In app/highlights/page.tsx from lines 91 to 255, remove the entire large block
of commented-out legacy code to clean up the codebase. This code is no longer
needed and version control retains its history if needed later, so deleting it
will improve readability and maintainability.
| {Array.from({ length: totalPages }, (_, i) => ( | ||
| <button | ||
| key={i} | ||
| onClick={() => onPageChange(i)} | ||
| className={`px-3 py-2 rounded-md text-sm font-medium transition-colors ${ | ||
| i === currentPage | ||
| ? 'bg-blue-600 text-white' | ||
| : 'bg-white text-gray-700 hover:bg-gray-50 border border-gray-300' | ||
| }`} | ||
| > | ||
| {i + 1} | ||
| </button> | ||
| ))} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement pagination window for better performance.
Creating buttons for all pages can cause performance issues with large datasets. Consider implementing a pagination window.
<div className="flex items-center space-x-1">
- {Array.from({ length: totalPages }, (_, i) => (
+ {(() => {
+ const maxVisible = 5;
+ const halfVisible = Math.floor(maxVisible / 2);
+ let start = Math.max(0, currentPage - halfVisible);
+ let end = Math.min(totalPages, start + maxVisible);
+
+ if (end - start < maxVisible) {
+ start = Math.max(0, end - maxVisible);
+ }
+
+ const pages = [];
+
+ if (start > 0) {
+ pages.push(
+ <button key={0} onClick={() => onPageChange(0)} className="...">1</button>
+ );
+ if (start > 1) pages.push(<span key="start-ellipsis">...</span>);
+ }
+
+ for (let i = start; i < end; i++) {
+ pages.push(
<button
key={i}
onClick={() => onPageChange(i)}
className={`px-3 py-2 rounded-md text-sm font-medium transition-colors ${
i === currentPage
? 'bg-blue-600 text-white'
: 'bg-white text-gray-700 hover:bg-gray-50 border border-gray-300'
}`}
>
{i + 1}
</button>
- ))}
+ );
+ }
+
+ if (end < totalPages) {
+ if (end < totalPages - 1) pages.push(<span key="end-ellipsis">...</span>);
+ pages.push(
+ <button key={totalPages - 1} onClick={() => onPageChange(totalPages - 1)} className="...">
+ {totalPages}
+ </button>
+ );
+ }
+
+ return pages;
+ })()}
</div>📝 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.
| {Array.from({ length: totalPages }, (_, i) => ( | |
| <button | |
| key={i} | |
| onClick={() => onPageChange(i)} | |
| className={`px-3 py-2 rounded-md text-sm font-medium transition-colors ${ | |
| i === currentPage | |
| ? 'bg-blue-600 text-white' | |
| : 'bg-white text-gray-700 hover:bg-gray-50 border border-gray-300' | |
| }`} | |
| > | |
| {i + 1} | |
| </button> | |
| ))} | |
| <div className="flex items-center space-x-1"> | |
| {(() => { | |
| const maxVisible = 5; | |
| const halfVisible = Math.floor(maxVisible / 2); | |
| let start = Math.max(0, currentPage - halfVisible); | |
| let end = Math.min(totalPages, start + maxVisible); | |
| if (end - start < maxVisible) { | |
| start = Math.max(0, end - maxVisible); | |
| } | |
| const pages: React.ReactNode[] = []; | |
| // First page + leading ellipsis | |
| if (start > 0) { | |
| pages.push( | |
| <button | |
| key={0} | |
| onClick={() => onPageChange(0)} | |
| className="px-3 py-2 rounded-md text-sm font-medium transition-colors bg-white text-gray-700 hover:bg-gray-50 border border-gray-300" | |
| > | |
| 1 | |
| </button> | |
| ); | |
| if (start > 1) { | |
| pages.push(<span key="start-ellipsis" className="px-2">…</span>); | |
| } | |
| } | |
| // Main window | |
| for (let i = start; i < end; i++) { | |
| pages.push( | |
| <button | |
| key={i} | |
| onClick={() => onPageChange(i)} | |
| className={`px-3 py-2 rounded-md text-sm font-medium transition-colors ${ | |
| i === currentPage | |
| ? 'bg-blue-600 text-white' | |
| : 'bg-white text-gray-700 hover:bg-gray-50 border border-gray-300' | |
| }`} | |
| > | |
| {i + 1} | |
| </button> | |
| ); | |
| } | |
| // Trailing ellipsis + last page | |
| if (end < totalPages) { | |
| if (end < totalPages - 1) { | |
| pages.push(<span key="end-ellipsis" className="px-2">…</span>); | |
| } | |
| pages.push( | |
| <button | |
| key={totalPages - 1} | |
| onClick={() => onPageChange(totalPages - 1)} | |
| className="px-3 py-2 rounded-md text-sm font-medium transition-colors bg-white text-gray-700 hover:bg-gray-50 border border-gray-300" | |
| > | |
| {totalPages} | |
| </button> | |
| ); | |
| } | |
| return pages; | |
| })()} | |
| </div> |
🤖 Prompt for AI Agents
In components/Highlights/HighlightsClient.tsx around lines 94 to 106, the
current code renders a button for every page, which can degrade performance with
many pages. Modify the pagination to display a limited window of page buttons
around the current page (e.g., show only 5 pages at a time), including first and
last pages with ellipses if needed. Update the rendering logic to generate
buttons only for this window to improve performance and usability.
There was a problem hiding this comment.
@caporalCode They're reviews for you. Also note the main branch was updated.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
Chores