Implement memories page #778
Conversation
|
|
WalkthroughThis PR introduces a Memories feature to the frontend while refactoring CSS for improved responsive design. It adds API endpoints and fetching functions for memories, implements the Memories page component with grid layout and image viewer, and consolidates API-specific styling rules into a generalized responsive approach that prevents overflow across device sizes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
docs/stylesheets/extra.css (2)
66-71: Code blocks:pre-wrap+break-allharms copy/paste and code readability.For docs, it’s usually better to keep
pre { white-space: pre; overflow-x: auto; }and only use wrapping on inline code or very specific long-token areas. Current rules will reflow code samples and break tokens/URLs.Also applies to: 279-285
59-64: Tables:display: blockon<table>can break styling/semantics.If you’re on MkDocs Material, prefer applying horizontal scrolling to the table wrapper (often
.md-typeset__table) instead of the<table>element itself.frontend/src/api/api-functions/memories.ts (1)
5-10: Consider supporting query cancellation (AbortSignal) to avoid wasted work on fast nav.If
usePictoQueryprovides a signal, pass it through to axios so requests are abortable; otherwise this is fine as-is.frontend/src/pages/Memories/Memories.tsx (1)
50-63: Consider adding an explicit guard for empty memory images in handleMemoryClick.While
MediaViewalready handles empty image arrays gracefully (returns null at line 142), adding an explicit check inhandleMemoryClickwould improve clarity and UX. Since Memory cards already conditionally render based on image availability (line 112), an empty array should be theoretically unreachable, but an explicit guard provides defensive consistency.Also applies to: 179-187
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
docs/backend/backend_python/api.md(0 hunks)docs/stylesheets/extra.css(5 hunks)frontend/src/api/api-functions/index.ts(1 hunks)frontend/src/api/api-functions/memories.ts(1 hunks)frontend/src/api/apiEndpoints.ts(1 hunks)frontend/src/pages/Memories/Memories.tsx(1 hunks)frontend/src/routes/AppRoutes.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- docs/backend/backend_python/api.md
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/routes/AppRoutes.tsx (1)
frontend/src/constants/routes.ts (1)
ROUTES(1-12)
frontend/src/api/api-functions/memories.ts (3)
frontend/src/types/API.ts (1)
APIResponse(1-8)frontend/src/api/axiosConfig.ts (1)
apiClient(5-12)frontend/src/api/apiEndpoints.ts (1)
memoriesEndpoints(34-36)
🔇 Additional comments (3)
frontend/src/api/apiEndpoints.ts (1)
33-36: LGTM: clean endpoint addition with consistent style.frontend/src/api/api-functions/index.ts (1)
1-7: LGTM: exposes the new memories API helper via the existing barrel.frontend/src/routes/AppRoutes.tsx (1)
12-26: LGTM: MEMORIES route now correctly renders the new page component.
| /* Global overflow prevention */ | ||
| * { | ||
| box-sizing: border-box; | ||
| } | ||
|
|
||
| html { | ||
| overflow-x: hidden; | ||
| max-width: 100vw; | ||
| } | ||
|
|
||
| body { | ||
| overflow-x: hidden; | ||
| max-width: 100vw; | ||
| position: relative; | ||
| } |
There was a problem hiding this comment.
Global overflow-x: hidden can mask real overflow and clip UI (shadows/menus/tooltips).
Consider scoping overflow rules to docs content containers (or only specific problematic components) rather than html/body, otherwise you can end up hiding regressions and clipping positioned elements.
🤖 Prompt for AI Agents
In docs/stylesheets/extra.css around lines 1–15, remove the global overflow-x:
hidden on html and body and instead apply overflow-x rules to a scoped docs
wrapper (e.g., the main docs content container or specific components that
exhibit horizontal scroll); audit and adjust any affected positioned children
(tooltips, menus, shadows) to use overflow: visible or be rendered outside that
container as needed so they are not clipped.
| .swagger-ui, | ||
| .swagger-ui * { | ||
| box-sizing: border-box; | ||
| max-width: 100%; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the file size and read the specific lines in question
wc -l docs/stylesheets/extra.cssRepository: AOSSIE-Org/PictoPy
Length of output: 92
🏁 Script executed:
# Read the specific lines and surrounding context (lines 330-355)
sed -n '330,355p' docs/stylesheets/extra.cssRepository: AOSSIE-Org/PictoPy
Length of output: 426
🏁 Script executed:
# Search for .swagger-ui overrides or exceptions
rg '\.swagger-ui' docs/stylesheets/extra.css -A 3 | head -60Repository: AOSSIE-Org/PictoPy
Length of output: 1204
🏁 Script executed:
# Check for any flex or grid related rules within swagger-ui context
rg '\.swagger-ui.*(?:flex|grid|display)' docs/stylesheets/extra.css -iRepository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
# Search for TODOs or FIXMEs related to swagger or width issues
rg '(TODO|FIXME|BUG|HACK).*(?:swagger|width|max-width)' docs/stylesheets/extra.css -iRepository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
# Search for input, select, textarea styling within swagger-ui
rg '\.swagger-ui.*(input|select|textarea|button|form)' docs/stylesheets/extra.css -i -A 2 | head -50Repository: AOSSIE-Org/PictoPy
Length of output: 146
🏁 Script executed:
# Check the broader context - are there any comments or issues mentioning layout problems?
rg 'swagger' docs/stylesheets/extra.css -B 2 -i | head -40Repository: AOSSIE-Org/PictoPy
Length of output: 537
🏁 Script executed:
# Look at the full selector specificity - check if the universal selector is necessary
rg '\.swagger-ui' docs/stylesheets/extra.css | wc -lRepository: AOSSIE-Org/PictoPy
Length of output: 64
Swagger UI: .swagger-ui * { max-width: 100% } has unnecessary broad scope.
The universal child selector applies max-width: 100% to all descendants indiscriminately. While form inputs, selects, and container elements have explicit overrides, the blanket rule is wasteful and error-prone. Scope width constraints to specific elements needing them (paths, descriptions, code blocks) and rely on overflow-x: auto on containers (opblock-body, parameters, responses-wrapper) rather than cascading max-width through the entire tree.
Also applies to: 344-347
🤖 Prompt for AI Agents
In docs/stylesheets/extra.css around lines 338-342 and 344-347, the rule
`.swagger-ui * { max-width: 100% }` is too broad and applies width constraints
to every descendant; remove the universal descendant selector and instead apply
max-width (or width constraints) only to the specific elements that need it
(e.g., .swagger-ui .opblock, .swagger-ui .opblock-description, .swagger-ui pre,
.swagger-ui code, .swagger-ui .parameter__value) and add `overflow-x: auto` to
container elements that should scroll horizontally (e.g., .opblock-body,
.parameters, .responses-wrapper); update the stylesheet to delete the blanket
selector and add targeted selectors and overflow rules accordingly.
| const { data, isLoading, isSuccess, isError, error } = usePictoQuery({ | ||
| queryKey: ['memories'], | ||
| queryFn: () => fetchMemories(), | ||
| }); | ||
|
|
||
| useMutationFeedback( | ||
| { isPending: isLoading, isSuccess, isError, error }, | ||
| { | ||
| loadingMessage: 'Loading memories', | ||
| showSuccess: false, | ||
| errorTitle: 'Error', | ||
| errorMessage: 'Failed to load memories. Please try again later.', | ||
| }, | ||
| ); | ||
|
|
||
| const memories: Memory[] = (data?.data as Memory[]) || []; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the API endpoint for memories
find . -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" \) | xargs grep -l "memories" | head -20Repository: AOSSIE-Org/PictoPy
Length of output: 664
🏁 Script executed:
# Look for type definitions related to Memory and APIResponse
fd -e "ts" -e "tsx" | xargs grep -l "interface Memory\|type Memory" | head -10Repository: AOSSIE-Org/PictoPy
Length of output: 499
🏁 Script executed:
# Find the usePictoQuery hook implementation
fd -e "ts" -e "tsx" | xargs grep -l "usePictoQuery" | head -10Repository: AOSSIE-Org/PictoPy
Length of output: 883
🏁 Script executed:
cat -n frontend/src/api/api-functions/memories.tsRepository: AOSSIE-Org/PictoPy
Length of output: 465
🏁 Script executed:
cat -n frontend/src/hooks/useQueryExtension.tsRepository: AOSSIE-Org/PictoPy
Length of output: 3519
🏁 Script executed:
# Find type definitions for Memory
fd -e "ts" -e "tsx" | xargs grep -l "type Memory\|interface Memory" | head -10Repository: AOSSIE-Org/PictoPy
Length of output: 499
🏁 Script executed:
# Look for API response type definitions
fd -e "ts" -e "tsx" | xargs grep -l "APIResponse\|ApiResponse" | head -10Repository: AOSSIE-Org/PictoPy
Length of output: 794
🏁 Script executed:
cat -n frontend/src/types/API.tsRepository: AOSSIE-Org/PictoPy
Length of output: 248
🏁 Script executed:
cat -n frontend/src/pages/Memories/Memories.tsxRepository: AOSSIE-Org/PictoPy
Length of output: 8039
🏁 Script executed:
# Check if there's a backend route handler for memories to see what it actually returns
find . -type f \( -name "*.py" -o -name "*.js" -o -name "*.ts" \) | xargs grep -l "getMemories\|/memories" | grep -v node_modules | head -10Repository: AOSSIE-Org/PictoPy
Length of output: 183
🏁 Script executed:
# Check the API endpoint definition
cat -n frontend/src/api/apiEndpoints.tsRepository: AOSSIE-Org/PictoPy
Length of output: 1447
🏁 Script executed:
# Check if there's backend code showing what /memories/ actually returns
find . -type f -name "*.py" | xargs grep -l "memories" | grep -v __pycache__ | head -5Repository: AOSSIE-Org/PictoPy
Length of output: 92
Define a specific type for the memories API response instead of relying on an unsafe cast.
Line 48 uses (data?.data as Memory[]) || [], which silently coerces any response shape mismatch to an empty array. Since APIResponse.data is typed as { [key: string]: any }, if the backend returns data nested differently (e.g., { memories: [...] } or any other structure), the cast hides the mismatch and renders "No Memories" with no indication of the actual problem. Recommend:
- Create a specific response type for
/memories/(e.g.,MemoriesResponseextendingAPIResponsewithdata: Memory[]) - Pass this type to
usePictoQuery<MemoriesResponse>to get compile-time safety, or - Add runtime validation (e.g., with Zod) to catch shape mismatches
🤖 Prompt for AI Agents
In frontend/src/pages/Memories/Memories.tsx around lines 33 to 49, the code
unsafely casts data?.data to Memory[] which hides response-shape mismatches;
define a concrete response type (e.g., interface MemoriesResponse extends
APIResponse { data: Memory[] }), update the usePictoQuery call to use the
generic usePictoQuery<MemoriesResponse> so TypeScript enforces the shape,
replace the unsafe cast with typed access (e.g., const memories = data?.data ??
[] typed as Memory[]), and optionally add runtime validation (Zod or similar) in
the fetchMemories function to guard against backend shape changes and surface
errors instead of silently rendering an empty list.
| const formatDate = (dateString: string) => { | ||
| const date = new Date(dateString); | ||
| return date.toLocaleDateString('en-US', { | ||
| year: 'numeric', | ||
| month: 'long', | ||
| day: 'numeric', | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd frontend/src/pages/Memories && cat -n Memories.tsx | head -100Repository: AOSSIE-Org/PictoPy
Length of output: 3880
🏁 Script executed:
cd frontend/src/pages/Memories && grep -n "formatDate" Memories.tsxRepository: AOSSIE-Org/PictoPy
Length of output: 170
🏁 Script executed:
find frontend -type f -name "*.tsx" -o -name "*.ts" | xargs grep -l "formatDate" | head -20Repository: AOSSIE-Org/PictoPy
Length of output: 102
🏁 Script executed:
find frontend -type f -name "*api*" | head -20Repository: AOSSIE-Org/PictoPy
Length of output: 94
🏁 Script executed:
find frontend/src/api -type f -name "*.ts" -o -name "*.tsx" | xargs catRepository: AOSSIE-Org/PictoPy
Length of output: 8829
🏁 Script executed:
find . -type f -name "api-functions.*" | head -5Repository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
find . -type d -name "api" | head -5Repository: AOSSIE-Org/PictoPy
Length of output: 80
🏁 Script executed:
find . -type f \( -name "*.py" -o -name "*.java" -o -name "*.js" -o -name "*.go" \) | grep -E "(memories|memory)" | head -20Repository: AOSSIE-Org/PictoPy
Length of output: 99
🏁 Script executed:
find . -type d -name "backend" -o -name "server" -o -name "src" | grep -v node_modules | head -10Repository: AOSSIE-Org/PictoPy
Length of output: 145
🏁 Script executed:
git ls-files | grep -E "(memories|memory)" | head -20Repository: AOSSIE-Org/PictoPy
Length of output: 140
🏁 Script executed:
find ./backend -type f -name "*.py" | xargs grep -l "memories" | head -20Repository: AOSSIE-Org/PictoPy
Length of output: 92
🏁 Script executed:
find ./backend -type f -name "*.py" | xargs grep -l "memory" | head -20Repository: AOSSIE-Org/PictoPy
Length of output: 128
🏁 Script executed:
ls -la ./backend/app/ | head -30Repository: AOSSIE-Org/PictoPy
Length of output: 558
🏁 Script executed:
find ./backend -type f -name "*.py" | grep -E "(route|api|main|app\.py)" | head -20Repository: AOSSIE-Org/PictoPy
Length of output: 281
🏁 Script executed:
cat ./backend/app/utils/memory_monitor.py | head -50Repository: AOSSIE-Org/PictoPy
Length of output: 1284
🏁 Script executed:
find ./backend -type f -name "*.py" -exec grep -l "memories\|on_this_day\|on this day" {} \;Repository: AOSSIE-Org/PictoPy
Length of output: 92
🏁 Script executed:
cat ./backend/main.pyRepository: AOSSIE-Org/PictoPy
Length of output: 4902
🏁 Script executed:
cat ./backend/tests/test_albums.py | grep -i "memor"Repository: AOSSIE-Org/PictoPy
Length of output: 112
🏁 Script executed:
find ./backend/app/routes -type f -name "*.py" -exec cat {} \; | grep -i "memor"Repository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
ls -la ./backend/app/routes/Repository: AOSSIE-Org/PictoPy
Length of output: 551
🏁 Script executed:
cat ./backend/tests/test_albums.py | head -100Repository: AOSSIE-Org/PictoPy
Length of output: 2983
🏁 Script executed:
node << 'EOF'
// Test how JavaScript parses date-only strings
const dateString = "2024-12-15";
const date = new Date(dateString);
console.log("Input:", dateString);
console.log("Parsed date:", date);
console.log("ISO string:", date.toISOString());
console.log("Local date string:", date.toLocaleDateString('en-US', {
year: 'numeric',
month: 'long',
day: 'numeric',
}));
// Test with timezone offset
console.log("\n--- Timezone Issue Example ---");
console.log("If user is in UTC-8 (PST), the parsed date might be off by one day");
const userTimezone = Intl.DateTimeFormat().resolvedOptions().timeZone;
console.log("Current timezone:", userTimezone);
EOFRepository: AOSSIE-Org/PictoPy
Length of output: 312
The date parsing implementation has a real timezone issue that can cause off-by-one day errors in certain timezones.
When new Date("YYYY-MM-DD") parses a date-only string, JavaScript interprets it as UTC midnight. For users in timezones west of UTC (e.g., PST, EST), toLocaleDateString() will convert this UTC time to their local timezone, potentially displaying the previous day instead.
For example, a user in UTC-8 seeing the date "2024-12-15" would get displayed as December 14 due to the 8-hour offset.
To fix this, either:
- Have the backend return full ISO 8601 timestamps with timezone information, or
- Format dates in UTC using
toLocaleDateString('en-US', { timeZone: 'UTC', ... })if date-only values are unavoidable
| <Card | ||
| key={memory.id} | ||
| className="group cursor-pointer transition-all hover:shadow-lg" | ||
| onClick={() => handleMemoryClick(memory)} | ||
| > |
There was a problem hiding this comment.
Make memory cards keyboard-accessible (clickable Card likely isn’t).
Add role="button", tabIndex={0}, and an onKeyDown handler for Enter/Space (or render as a <button>/<a> via your UI lib’s asChild pattern) to avoid an accessibility blocker.
🤖 Prompt for AI Agents
In frontend/src/pages/Memories/Memories.tsx around lines 104-108, the Card used
for each memory is not keyboard-accessible; make it accessible by adding
role="button", tabIndex={0}, and an onKeyDown handler that triggers the same
action as onClick when Enter or Space is pressed (or alternatively render the
Card as an actual interactive element via your UI library’s asChild pattern,
e.g., as a button or anchor). Ensure the onKeyDown checks for key === 'Enter' or
key === ' ' / key === 'Spacebar' (and prevents default for Space) before calling
handleMemoryClick(memory), and keep the existing onClick behavior.
|
No backend APIs available. |
2 participants
Changes-
Implemented the Memories page UI and removed the “Coming Soon” state
Added logic to automatically generate memories based on:
Time (date ranges like “On this day last year”)
Location (same or nearby locations)
Grouped related photos/media into story-style memory cards
Displayed a small set of representative highlights instead of all media
Ensured memories refresh dynamically based on the current date
Integrated frontend and backend to support automatic memory generation without manual album creation
Result
Users can now easily revisit past moments through interactive, time- and location-based memories, making the Memories feature fully functional and ready for use.
Summary by CodeRabbit
New Features
Style
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.