Feat: Scrollbar Timeline with {Month-Year} Markers for Home page and AI Tagging page. #552
Conversation
WalkthroughCentralizes SQLite connections and normalizes image metadata as structured mappings across backend DB, utils, routes, and schemas; frontend adds chronological gallery grouping, a timeline scrollbar with month markers and hooks, UI/CSS tweaks, and a ResizeObserver test polyfill. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Page as Home / AI Tagging
participant Chrono as ChronologicalGallery
participant Utils as dateUtils/timelineUtils
participant Scroll as ScrollContainer
participant TL as TimelineScrollbar
User->>Page: Open gallery page
Page->>Chrono: render(images, scrollContainerRef, onMonthOffsetsChange)
Chrono->>Utils: group images by year/month
Chrono->>Scroll: observe headers & measure offsets
Chrono-->>Page: onMonthOffsetsChange(markers)
Page->>TL: render(monthMarkers, scrollableRef)
User->>TL: hover / click / drag
TL->>Scroll: scrollTo(offset)
Scroll-->>Chrono: scroll/resize events
Chrono-->>Page: recompute markers
sequenceDiagram
autonumber
participant Client
participant Route as images.py (route)
participant DB as images.py (db)
participant Util as app.utils.images
Client->>Route: request insert/update images
Route->>DB: db_bulk_insert_images(records)
DB->>Util: image_util_parse_metadata / image_util_extract_metadata
Util-->>DB: metadata dict / JSON string
DB->>DB: INSERT ... ON CONFLICT ... (store JSON)
DB-->>Route: rows with parsed metadata
Route-->>Client: response with structured metadata
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/pages/Home/Home.tsx (1)
33-52: Loader never hides when search is toggled mid-fetch.
showLoaderfires beforeisSearchActiveflips, but once the guard short-circuits the effect we never dispatchhideLoader, leaving the overlay stuck until search is deactivated. Please move the hide/error handling outside the guard (or otherwise ensure we always clear the loader when the query resolves).
🧹 Nitpick comments (1)
frontend/src/components/Media/MediaInfoPanel.tsx (1)
83-87: Consider extracting specific location fields from metadata.The current logic handles both string and object metadata types, but using
JSON.stringifyfor object metadata may not provide the best user experience. IfImageMetadataincludes specific location fields (e.g.,file_location), consider extracting and displaying those directly for better readability.Example refactor to display
file_locationif available:-{typeof currentImage?.metadata === 'string' - ? currentImage.metadata - : currentImage?.metadata - ? JSON.stringify(currentImage.metadata) - : 'No location data'} +{typeof currentImage?.metadata === 'string' + ? currentImage.metadata + : currentImage?.metadata?.file_location + ? currentImage.metadata.file_location + : currentImage?.metadata + ? JSON.stringify(currentImage.metadata) + : 'No location data'}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
backend/app/database/images.py(12 hunks)backend/app/routes/images.py(2 hunks)docs/backend/backend_python/openapi.json(1 hunks)frontend/jest.setup.ts(1 hunks)frontend/src/App.css(1 hunks)frontend/src/components/Media/ChronologicalGallery.tsx(1 hunks)frontend/src/components/Media/MediaInfoPanel.tsx(1 hunks)frontend/src/components/Timeline/TimelineScrollbar.tsx(1 hunks)frontend/src/layout/layout.tsx(1 hunks)frontend/src/pages/AITagging/AITagging.tsx(3 hunks)frontend/src/pages/Home/Home.tsx(3 hunks)frontend/src/types/Media.ts(1 hunks)frontend/src/utils/dateUtils.ts(2 hunks)frontend/src/utils/timelineUtils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
frontend/src/pages/AITagging/AITagging.tsx (3)
frontend/src/features/imageSelectors.ts (2)
selectIsImageViewOpen(24-27)selectTaggedImages(39-41)frontend/src/components/Media/ChronologicalGallery.tsx (2)
MonthMarker(11-15)ChronologicalGallery(26-190)frontend/src/components/Timeline/TimelineScrollbar.tsx (1)
TimelineScrollbar(39-421)
frontend/src/pages/Home/Home.tsx (5)
frontend/src/components/Media/ChronologicalGallery.tsx (2)
MonthMarker(11-15)ChronologicalGallery(26-190)frontend/src/hooks/useQueryExtension.ts (1)
usePictoQuery(80-108)frontend/src/api/api-functions/images.ts (1)
fetchAllImages(5-10)frontend/src/app/store.ts (1)
RootState(22-22)frontend/src/components/Timeline/TimelineScrollbar.tsx (1)
TimelineScrollbar(39-421)
frontend/src/utils/dateUtils.ts (1)
frontend/src/types/Media.ts (1)
Image(11-20)
frontend/src/components/Timeline/TimelineScrollbar.tsx (2)
frontend/src/components/Media/ChronologicalGallery.tsx (1)
MonthMarker(11-15)frontend/src/utils/timelineUtils.ts (4)
TooltipState(4-8)useScroll(10-32)useWheel(34-53)getMarkerForScrollPosition(55-67)
frontend/src/components/Media/ChronologicalGallery.tsx (4)
frontend/src/types/Media.ts (1)
Image(11-20)frontend/src/features/imageSelectors.ts (1)
selectImages(5-7)frontend/src/utils/dateUtils.ts (2)
groupImagesByYearMonthFromMetadata(31-55)createImageIndexMap(58-66)frontend/src/components/Media/ImageCard.tsx (1)
ImageCard(20-111)
frontend/src/utils/timelineUtils.ts (1)
frontend/src/components/Media/ChronologicalGallery.tsx (1)
MonthMarker(11-15)
backend/app/database/images.py (1)
frontend/src/types/Media.ts (1)
Image(11-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Tauri Build Check (windows-latest)
- GitHub Check: Tauri Build Check (ubuntu-22.04)
- GitHub Check: Tauri Build Check (macos-latest, --target aarch64-apple-darwin)
- GitHub Check: Backend Tests
🔇 Additional comments (13)
frontend/src/layout/layout.tsx (1)
12-12: Spacing change is safe – verified that Home and AITagging include their own padding/margin (pr-6,mt-6,mb-8, etc.) and TimelineScrollbar uses absolute positioning without relying on container padding. No further adjustments needed.frontend/jest.setup.ts (1)
11-18: LGTM! ResizeObserver mock is appropriate for tests.The minimal mock with empty methods is sufficient for components that instantiate ResizeObserver without requiring actual resize observations during tests. This aligns with the existing polyfill pattern for TextEncoder/TextDecoder.
frontend/src/App.css (2)
200-207: LGTM! Standard cross-browser scrollbar hiding.The
.hide-scrollbarutility correctly implements cross-browser scrollbar hiding using WebKit-specific pseudo-element, IE/Edge-ms-overflow-style, and Firefoxscrollbar-widthproperties.
209-214: LGTM! Standard cross-browser selection disabling.The
.no-selectutility correctly implements cross-browser text selection disabling with vendor-prefixed and standard properties.docs/backend/backend_python/openapi.json (1)
1933-1936: LGTM! Schema aligns with backend metadata type change.The OpenAPI schema correctly reflects the backend change where
ImageData.metadatais now a structured object (Dict[str, Any]) instead of a string. This aligns with the broader PR changes to support structured image metadata.backend/app/routes/images.py (1)
2-2: LGTM! Metadata type change aligns with backend updates.The type change from
strtoDict[str, Any]forImageData.metadatacorrectly reflects the backend database layer changes where metadata is now stored and retrieved as structured JSON. This ensures consistent metadata representation across the API layer.Also applies to: 16-16
frontend/src/types/Media.ts (2)
1-9: LGTM! ImageMetadata interface provides clear metadata structure.The
ImageMetadatainterface defines a well-structured representation of image metadata with appropriate field types. Thedate_createdfield as string is suitable for ISO date strings used in chronological grouping throughout the application.
17-17: LGTM! Type change enables structured metadata access.Changing
metadatafromstringtoImageMetadataenables type-safe access to metadata fields throughout the frontend, particularly for the new chronological grouping functionality inChronologicalGalleryand date utilities.frontend/src/utils/dateUtils.ts (2)
30-55: LGTM! Robust date grouping with proper error handling.The
groupImagesByYearMonthFromMetadatafunction correctly handles:
- Missing metadata or date_created fields (early returns)
- Invalid date strings (NaN check)
- Nested group initialization
- Month padding for consistent keys
The logic is sound and appropriate for chronological gallery grouping.
57-66: LGTM! Efficient index lookup with Map.The
createImageIndexMapfunction provides O(1) lookup performance for mapping image IDs to their indices, which is optimal for the gallery rendering use case where quick index lookups are needed.frontend/src/pages/AITagging/AITagging.tsx (3)
24-25: LGTM! State additions support timeline scrollbar integration.The
scrollableRefandmonthMarkersstate are correctly initialized to support the timeline scrollbar feature:
scrollableRefprovides scroll control for TimelineScrollbarmonthMarkerstracks month positions from ChronologicalGallery'sonMonthOffsetsChangecallback
50-75: LGTM! Layout correctly integrates chronological gallery with timeline scrollbar.The restructured layout properly implements:
- Scrollable container with hidden scrollbar (
.hide-scrollbar)scrollableRefattachment for scroll controlChronologicalGalleryintegration withonMonthOffsetsChangecallback- Retained
FaceCollectionsandMediaViewcomponents in the flowThe structure supports the absolutely positioned
TimelineScrollbarrendered below.
76-82: LGTM! TimelineScrollbar correctly integrated with conditional rendering.The
TimelineScrollbarintegration properly:
- Conditionally renders only when month markers exist
- Receives correct props (
scrollableRef,monthMarkers)- Uses absolute positioning to overlay on the right edge
- Maintains consistent integration pattern with other pages
…cation support Moved image metadata extraction logic to a utility module and standardized metadata parsing across backend endpoints. Updated backend and frontend types and models to support location, latitude, and longitude in image metadata. Improved frontend display of image location and date.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
backend/app/database/face_clusters.py(2 hunks)backend/app/database/faces.py(2 hunks)backend/app/database/images.py(12 hunks)backend/app/routes/face_clusters.py(2 hunks)backend/app/routes/images.py(2 hunks)backend/app/schemas/face_clusters.py(2 hunks)backend/app/utils/images.py(3 hunks)docs/backend/backend_python/openapi.json(3 hunks)frontend/src/components/Media/MediaInfoPanel.tsx(2 hunks)frontend/src/pages/Home/Home.tsx(3 hunks)frontend/src/types/Media.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/pages/Home/Home.tsx
- backend/app/routes/images.py
- frontend/src/components/Media/MediaInfoPanel.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
backend/app/database/faces.py (1)
backend/app/utils/images.py (1)
image_util_parse_metadata(505-522)
backend/app/database/face_clusters.py (1)
backend/app/utils/images.py (1)
image_util_parse_metadata(505-522)
backend/app/database/images.py (1)
backend/app/utils/images.py (1)
image_util_parse_metadata(505-522)
backend/app/utils/images.py (1)
frontend/src/types/Media.ts (1)
Image(12-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Tauri Build Check (ubuntu-22.04)
- GitHub Check: Tauri Build Check (windows-latest)
- GitHub Check: Tauri Build Check (macos-latest, --target aarch64-apple-darwin)
- GitHub Check: Backend Tests
…oderabbit suggestions
rahulharpal1603
left a comment
rahulharpal1603
left a comment
There was a problem hiding this comment.
I wanted to merge this today itself, but there are some minor suggestions, please fix those too.
Moved GPS coordinate parsing logic into a dedicated extract_gps_coordinates() helper function for better readability and maintainability. Removed reverse geocoding using geopy as per review feedback to prevent external API overhead. Updated Python and TypeScript metadata models to only include latitude and longitude. Restored original dependencies (httptools, httpx, humanfriendly) and removed geopy from requirements.txt. Updated MediaInfoPanel.tsx to display latitude and longitude directly with Google Maps link integration via Tauri Opener.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
frontend/src/components/Media/MediaInfoPanel.tsx (1)
107-119: Consider adding explicittype="button"attribute.The clickable location button should explicitly specify
type="button"to prevent unintended form submission behavior if this component is ever nested within a form context.<button + type="button" onClick={handleLocationClick} className="w-full cursor-pointer truncate text-left font-medium text-white hover:underline" title={`Lat: ${currentImage.metadata.latitude}, Lon: ${currentImage.metadata.longitude}`} >backend/app/utils/images.py (2)
335-348: Consider extracting nested function for clarity.The
_convert_to_degreesfunction is nested inside the try block. While functional, extracting it to module level or defining it before the try block would improve readability and testability.Consider extracting the function:
def _convert_dms_to_degrees(value): """Convert GPS DMS (degrees, minutes, seconds) to decimal degrees.""" if hasattr(value[0], "numerator"): d = float(value[0].numerator) / float(value[0].denominator) else: d = float(value[0]) if hasattr(value[1], "numerator"): m = float(value[1].numerator) / float(value[1].denominator) else: m = float(value[1]) if hasattr(value[2], "numerator"): s = float(value[2].numerator) / float(value[2].denominator) else: s = float(value[2]) return d + (m / 60.0) + (s / 3600.0) def _extract_gps_coordinates(exif_data: Any) -> Tuple[float | None, float | None]: # ... existing code ... latitude = _convert_dms_to_degrees(lat_dms) # ... etc ...
384-488: LGTM! Robust metadata extraction with good error handling.The metadata extraction implementation is solid with:
- Proper fallbacks for missing files
- Defensive EXIF parsing with multiple exception handlers
- Consistent return structure
- Safe date parsing with fallback to mtime
Consider using the logging module instead of print statements for production-ready error reporting.
Example logging setup at module level:
import logging logger = logging.getLogger(__name__)Then replace print statements:
- print(f"[ERROR] Pillow could not open image {image_path}: {e}") + logger.error(f"Pillow could not open image {image_path}: {e}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/app/utils/images.py(3 hunks)frontend/src/components/Media/MediaInfoPanel.tsx(3 hunks)frontend/src/types/Media.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/types/Media.ts
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/utils/images.py (1)
frontend/src/types/Media.ts (1)
Image(13-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Backend Tests
- GitHub Check: Tauri Build Check (macos-latest, --target aarch64-apple-darwin)
- GitHub Check: Tauri Build Check (ubuntu-22.04)
- GitHub Check: Tauri Build Check (windows-latest)
🔇 Additional comments (7)
frontend/src/components/Media/MediaInfoPanel.tsx (2)
29-40: LGTM! Clean conditional date rendering.The date display logic properly handles missing metadata with a user-friendly fallback message, and the formatting is appropriate.
48-58: Verify plugin choice against past review comment.A past review comment specifically requested using
@tauri-apps/plugin-openerto open the Google Maps link, but the implementation uses@tauri-apps/plugin-shell. Whileplugin-shell'sopenfunction may work for URLs,plugin-openeris specifically designed for opening URLs/files with default applications.Please confirm whether using
plugin-shellinstead ofplugin-openeris intentional. If not, consider updating the import and usage to align with the previous review guidance:-import { open } from '@tauri-apps/plugin-shell'; +import { open } from '@tauri-apps/plugin-opener';Based on past review comments.
backend/app/utils/images.py (5)
3-6: LGTM! Imports support new metadata functionality.The new imports (datetime, json, typing, ExifTags) are appropriate for the metadata extraction features added in this file.
157-165: LGTM! Metadata extraction properly integrated.The metadata extraction is correctly integrated into the image processing flow, with metadata serialized as JSON for database storage.
328-328: LGTM! GPS tag hardcoded as requested.The GPS_INFO_TAG is correctly hardcoded to 34853, addressing the previous review comment.
Based on past review comments
490-507: LGTM! Defensive metadata parsing.The function safely handles various input formats (None, JSON string, dict) with appropriate fallbacks. The read-only
Mappingreturn type is appropriate.
318-318: Verify Python version compatibility for union type hints
Thefloat | Noneon line 318 requires Python 3.10+. Ensurepython_requires=">=3.10"is declared in pyproject.toml/setup.cfg or replace withOptional[float]for older Python versions.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
backend/app/utils/images.py (4)
321-335: Consider simplifying the DMS conversion logic.The current implementation correctly converts GPS coordinates from DMS to decimal degrees. However, the repeated
hasattrchecks and manual float conversions can be simplified.Apply this diff to simplify:
-def _convert_to_degrees(value): - """Converts a GPS coordinate value from DMS to decimal degrees.""" - if hasattr(value[0], "numerator"): - d = float(value[0].numerator) / float(value[0].denominator) - else: - d = float(value[0]) - if hasattr(value[1], "numerator"): - m = float(value[1].numerator) / float(value[1].denominator) - else: - m = float(value[1]) - if hasattr(value[2], "numerator"): - s = float(value[2].numerator) / float(value[2].denominator) - else: - s = float(value[2]) - return d + (m / 60.0) + (s / 3600.0) +def _convert_to_degrees(value): + """Converts a GPS coordinate value from DMS to decimal degrees.""" + def to_float(v): + return float(v.numerator) / float(v.denominator) if hasattr(v, "numerator") else float(v) + + d, m, s = (to_float(v) for v in value[:3]) + return d + (m / 60.0) + (s / 3600.0)
348-348: Consider moving GPS_INFO_TAG to module level.Based on past review comments,
GPS_INFO_TAGwas requested to be hardcoded. Consider defining it as a module-level constant rather than inside the function for better maintainability and reusability.Add this constant at the top of the file after the imports:
# GPS EXIF tag constant GPS_INFO_TAG = 34853Then update the function:
def _extract_gps_coordinates(exif_data: Any) -> Tuple[float | None, float | None]: """ Extracts GPS coordinates from EXIF data. Args: exif_data: The EXIF data from an image (PIL.Image.Exif object). Returns: A tuple containing latitude and longitude, or (None, None) if not found. """ latitude = None longitude = None - GPS_INFO_TAG = 34853 if exif_data:
381-383: Remove redundant None assignments.Lines 382-383 set
latitudeandlongitudetoNone, but they are already initialized toNoneat lines 346-347. This is redundant.Apply this diff:
else: - latitude = None - longitude = None + pass # gps_data is not a dict, keep None values
159-161: Consider combining thumbnail generation and metadata extraction.Currently, each image is opened twice: once for thumbnail generation (line 159) and once for metadata extraction (line 160). This could be optimized by extracting metadata during thumbnail generation, reducing I/O operations.
Here's how you could refactor:
- Modify
image_util_generate_thumbnailto return both success status and metadata:def image_util_generate_thumbnail( image_path: str, thumbnail_path: str, size: Tuple[int, int] = (600, 600) ) -> Tuple[bool, dict]: """Generate thumbnail and extract metadata in a single pass.""" metadata = {} try: with Image.open(image_path) as img: # Extract metadata first while image is open width, height = img.size mime_type = Image.MIME.get(img.format, "unknown") # Extract EXIF and GPS data # ... (metadata extraction logic) ... # Then generate thumbnail img.thumbnail(size) if img.mode in ("RGBA", "P"): img = img.convert("RGB") img.save(thumbnail_path, "JPEG") return True, metadata except Exception as e: logger.error(f"Error processing {image_path}: {e}") return False, {}
- Update the caller:
- if image_util_generate_thumbnail(image_path, thumbnail_path): - metadata = image_util_extract_metadata(image_path) + success, metadata = image_util_generate_thumbnail(image_path, thumbnail_path) + if success: logger.debug(f"Extracted metadata for {image_path}: {metadata}")This reduces file I/O and improves performance, especially when processing large image galleries.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/app/utils/images.py(11 hunks)frontend/src/components/Media/MediaInfoPanel.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/Media/MediaInfoPanel.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/utils/images.py (2)
frontend/src/types/Media.ts (1)
Image(13-22)backend/app/database/images.py (1)
db_delete_images_by_ids(345-377)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Tauri Build Check (windows-latest)
- GitHub Check: Tauri Build Check (ubuntu-22.04)
- GitHub Check: Backend Tests
🔇 Additional comments (5)
backend/app/utils/images.py (5)
3-22: LGTM! Proper imports and logger setup.The new imports support metadata extraction functionality, and the module-level logger setup follows Python best practices.
388-491: Excellent defensive error handling for metadata extraction.The implementation demonstrates strong defensive programming with multiple fallback layers:
- File existence check with minimal metadata fallback
- Robust EXIF extraction with support for both
getexif()and legacy_getexif()- Safe date parsing with fallback to file modification time
- GPS coordinate extraction with graceful degradation
- Per-layer exception handling that maintains partial metadata
This approach ensures the image processing pipeline continues even when metadata extraction partially fails, which is appropriate for real-world image processing scenarios.
494-511: LGTM! Safe metadata parsing with proper type handling.The function correctly handles various input types (None, JSON string, dict) with appropriate fallbacks. Using
Mapping[str, Any]as the return type is a good choice to indicate the result should be treated as read-only.
160-161: Metadata extraction and persistence correctly integrated.The metadata extraction is properly integrated into the image processing flow, with appropriate logging and JSON serialization for database storage.
Also applies to: 168-168
338-338: Verify Python version for union type hints
Union syntax (float | None) requires Python 3.10+. Confirm the project targets Python 3.10+; if not, useTuple[Optional[float], Optional[float]].
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/app/utils/images.py (1)
339-384: GPS extraction logic is robust.The function correctly handles GPS coordinate extraction with proper byte decoding, direction references, and comprehensive error handling. The broad exception catching is appropriate given the variability in EXIF data formats.
Optional: Remove unnecessary else block.
The empty
else: passblock at line 382 can be removed for cleaner code:except ( KeyError, IndexError, TypeError, ValueError, AttributeError, ): latitude = None longitude = None - else: - pass return latitude, longitude
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/app/utils/images.py(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/utils/images.py (2)
frontend/src/types/Media.ts (1)
Image(13-22)backend/app/database/images.py (1)
db_delete_images_by_ids(345-377)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Backend Tests
- GitHub Check: Tauri Build Check (macos-latest, --target aarch64-apple-darwin)
- GitHub Check: Tauri Build Check (ubuntu-22.04)
- GitHub Check: Tauri Build Check (windows-latest)
🔇 Additional comments (5)
backend/app/utils/images.py (5)
23-26: LGTM! Constants and logger properly initialized.The GPS_INFO_TAG is correctly hardcoded as requested in previous reviews, and the logger follows standard Python logging practices.
67-67: LGTM! Proper logging migration.Replacing print statements with appropriate logger calls (error, info, debug) improves observability and follows best practices.
Also applies to: 80-80, 97-97, 119-119, 209-209, 229-229, 253-253, 255-255, 259-259
164-172: LGTM! Metadata extraction integrated correctly.The metadata extraction, logging, and JSON serialization for database storage are implemented properly.
387-491: LGTM! Robust metadata extraction with multiple fallback layers.The function demonstrates excellent defensive programming with:
- Early return for non-existent files
- Safe EXIF extraction with fallback to basic file stats
- Date parsing from EXIF DateTimeOriginal with fallback to file mtime
- Conditional GPS coordinate inclusion
- Multiple exception handlers ensuring metadata extraction failures don't halt image processing
The debug log message now correctly references the function name (addressing previous review feedback).
493-510: LGTM! Robust metadata parsing with safe defaults.The function correctly handles all expected input types (None, JSON string, dict) with appropriate error handling and returns a safe empty dict default.
Updated scrollbar component styles and tooltip content.
Removed the empty state message for no images found in the gallery component.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
frontend/src/components/Media/MediaInfoPanel.tsx (2)
49-59: Consider user-facing error feedback.The implementation correctly addresses the past review comments by using Tauri's opener plugin and opening Google Maps with the suggested URL format. However, the error handling only logs to the console.
Consider showing a user-facing notification when the map fails to open:
try { await open(url); } catch (error) { console.error('Failed to open map URL:', error); + // Display a toast or notification to the user + // e.g., toast.error('Failed to open map. Please try again.'); }
108-121: Add aria-label for improved accessibility.The location display correctly addresses the past review comments by showing coordinates and making them clickable. The coordinate formatting and visual feedback are well implemented.
Consider adding an
aria-labelto improve screen reader accessibility:<button type="button" onClick={handleLocationClick} + aria-label={`Open location on Google Maps: Latitude ${currentImage.metadata.latitude}, Longitude ${currentImage.metadata.longitude}`} className="flex w-full cursor-pointer items-center truncate text-left font-medium text-white hover:underline" title={`Lat: ${currentImage.metadata.latitude}, Lon: ${currentImage.metadata.longitude}`} >frontend/src/components/Timeline/TimelineScrollbar.tsx (4)
24-24: Consider using standard abbreviation for September."Sept" is less common than "Sep" for September. While not incorrect, using "Sep" would align with more widely recognized conventions.
Apply this diff to use the standard abbreviation:
- September: 'Sept', + September: 'Sep',
244-252: Consider more specific error handling.The try-catch in the mousemove handler catches all errors generically. While this prevents crashes, it may mask unexpected issues. Consider being more specific about what errors you expect (e.g., DOM reference errors) or add context to the error log.
For example:
} catch (error) { - console.error('Error during drag:', error); + console.error('Error during drag (possible DOM reference issue):', error); handleMouseUp(); }
341-359: Consider using a stable key for marker elements.Using array index as the key can cause issues if markers are reordered or modified. Consider using a combination of month, year, and offset as a unique identifier.
For example:
-{markerPositions.map((marker, index) => { +{markerPositions.map((marker) => { return ( <div - key={index} + key={`${marker.year}-${marker.month}-${marker.offset}`} className="absolute left-1/2 h-0.5 w-full bg-gray-400 shadow-md"
315-408: Consider adding keyboard navigation and ARIA labels.The timeline scrollbar currently lacks keyboard accessibility and screen reader support. Consider adding:
- Arrow key navigation to move between month markers
- Enter/Space to jump to a marker
- ARIA labels describing the timeline and current position
role="slider"orrole="scrollbar"with appropriate ARIA attributesThis would improve usability for keyboard and screen reader users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/App.css(1 hunks)frontend/src/components/Media/ChronologicalGallery.tsx(1 hunks)frontend/src/components/Media/MediaInfoPanel.tsx(4 hunks)frontend/src/components/Timeline/TimelineScrollbar.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/App.css
- frontend/src/components/Media/ChronologicalGallery.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/Timeline/TimelineScrollbar.tsx (2)
frontend/src/components/Media/ChronologicalGallery.tsx (1)
MonthMarker(11-15)frontend/src/utils/timelineUtils.ts (4)
TooltipState(4-8)useScroll(10-32)useWheel(34-53)getMarkerForScrollPosition(55-67)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Tauri Build Check (windows-latest)
- GitHub Check: Tauri Build Check (macos-latest, --target aarch64-apple-darwin)
- GitHub Check: Tauri Build Check (ubuntu-22.04)
🔇 Additional comments (3)
frontend/src/components/Media/MediaInfoPanel.tsx (2)
10-10: LGTM!The import of
SquareArrowOutUpRightis correct and follows the tree-shakable pattern recommended for lucide-react.
29-41: LGTM!The date handling logic correctly uses
metadata.date_createdwith proper fallback behavior and formatted output. The defensive coding with optional chaining prevents runtime errors.frontend/src/components/Timeline/TimelineScrollbar.tsx (1)
81-104: LGTM! Robust marker positioning logic.The memoized calculation correctly handles edge cases:
- Filters unreachable markers (lines 90-92)
- Prevents division by zero with safeDenominator (line 94)
- Clamps positions to valid track bounds (line 101)
rahulharpal1603
left a comment
rahulharpal1603
left a comment
There was a problem hiding this comment.
Thank you so much, @rohan-pandeyy
This issue was open for so long, and you were very patient. I have made some minor changes; other than that, everything looks good.
|
@coderabbitai resolve |
✅ Actions performedComments resolved. |
@rahulharpal1603 thank you. I genuinely appreciate you taking the time to review it, this issue had quite the journey!!! Really glad to have contributed to such a vast feature.😁 |
2 participants
Also, Pictopy now retrieves image metadata! :)
Fixes: #495
This PR is divided into 3 major parts:
1. Image metadata retrieval:
Status: Done ✅
2. Chronological gallery view
Status: Done ✅
3. Timeline scrollbar integration
Status: Done ✅
Recording.2025-09-14.195728.mp4
Summary by CodeRabbit
New Features
Bug Fixes
Style
Documentation
Tests