added favorites section #117
Conversation
|
@Rish6392 is attempting to deploy a commit to the ACM FUN Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds a favorites feature allowing users to save and manage quotes and jokes from activities. Items are stored in localStorage with a 24-hour auto-expiry mechanism.
- Implements favorites storage utilities with localStorage persistence and automatic expiry after 24 hours
- Adds wrapper components for RandomQuote, RandomJoke, and RandomAnimeQuote activities that include a favorite button
- Creates a new Favorites page with expand/collapse functionality for long content and time-remaining display
Reviewed Changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/favoritesStorage.js | Core storage logic for managing favorites with localStorage, including add/remove/list operations and 24-hour expiry |
| src/components/common/FavoriteButton.js | Reusable button component to toggle favorite status of items |
| src/components/common/FavoriteButton.css | Styling for favorite button with saved state indication |
| src/pages/Favorites.js | Main favorites page displaying saved items with expand/collapse and auto-clear countdown |
| src/pages/activities_wrappers/*.js | Wrapper components that add favorite button to existing activity pages |
| src/data/content.js | Updates activity configuration to use wrapper components with favorite functionality |
| src/components/common/Navbar.js | Adds Favorites nav item and improves active state logic |
| src/styles/components/common/Navbar.css | Adds CSS for 4th navbar item and standardizes icon sizing |
| src/styles/pages/activities/*.css | Adds shared action container styles and text truncation utility |
| src/assets/icons/*.svg | New star and heart outline icons for favorites UI |
| src/App.js | Adds route for /favorites page |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const exists = (raw.items || []).some((it) => JSON.stringify(it.content) === JSON.stringify(item.content)); | ||
| if (exists) return false; | ||
| const entry = { | ||
| id: Date.now() + Math.floor(Math.random() * 1000), |
There was a problem hiding this comment.
The ID generation method can produce collisions if multiple items are added within the same millisecond. Consider using a more robust approach like incrementing IDs or using crypto.randomUUID() if available.
| const text = content.text || content.quote || ''; | ||
| const author = content.author || content.character || ''; | ||
| const extra = content.anime ? ` (${content.anime})` : ''; | ||
| const isLong = !!longMap[it.id]; |
There was a problem hiding this comment.
Inconsistent indentation: this line has only 2 spaces while surrounding code has proper indentation. Should be indented to align with the surrounding code block.
| const isLong = !!longMap[it.id]; | |
| const isLong = !!longMap[it.id]; |
|
|
||
| if (it.type && it.type.includes('joke')) { | ||
| const txt = content.text || (content.setup ? `${content.setup}\n\n${content.delivery}` : ''); | ||
| const isLong = !!longMap[it.id]; |
There was a problem hiding this comment.
Inconsistent indentation: this line has only 2 spaces while surrounding code has proper indentation. Should be indented to align with the surrounding code block.
| const isLong = !!longMap[it.id]; | |
| const isLong = !!longMap[it.id]; |
| @@ -0,0 +1,34 @@ | |||
| import React, {useEffect, useState} from 'react'; | |||
| import './FavoriteButton.css'; | |||
There was a problem hiding this comment.
Imported CSS file doesn't match the CSS module import pattern. Either use the .module.css file that exists (FavoriteButton.module.css) or ensure this plain CSS file exists. Currently both files are created but only the plain CSS is imported.
| const raw = _readRaw() || { items: [], createdAt: null }; | ||
| if (!raw.createdAt) raw.createdAt = Date.now(); | ||
| // avoid duplicates by content | ||
| const exists = (raw.items || []).some((it) => JSON.stringify(it.content) === JSON.stringify(item.content)); |
There was a problem hiding this comment.
Using JSON.stringify for comparison is inefficient and can produce false negatives due to property ordering. Consider using a deep equality function or a content-based hash for duplicate detection.
|
|
||
| useEffect(() => { | ||
| const all = listFavorites(); | ||
| const exists = all.some((it) => JSON.stringify(it.content) === JSON.stringify(item.content)); |
There was a problem hiding this comment.
Using JSON.stringify for comparison is inefficient and can produce false negatives due to property ordering. This same comparison is used multiple times. Consider extracting it to a utility function and using a more reliable comparison method.
| if (saved) { | ||
| // remove | ||
| const all = listFavorites(); | ||
| const found = all.find((it) => JSON.stringify(it.content) === JSON.stringify(item.content)); |
There was a problem hiding this comment.
Using JSON.stringify for comparison is inefficient and can produce false negatives due to property ordering. This same comparison is used multiple times. Consider extracting it to a utility function and using a more reliable comparison method.
| } | ||
| } | ||
|
|
||
| export function _isExpired(raw) { |
There was a problem hiding this comment.
The _isExpired function is exported but prefixed with underscore suggesting it's private. Either make it truly internal (remove export) or remove the underscore prefix to indicate it's a public API.
| export function _isExpired(raw) { | |
| function _isExpired(raw) { |
|
LGTM |
2 participants
Added a new favorite section as stated.