Skip to content

feat: per-journal Calendar page, anniversary filter, camera attach (v…#10

Merged
pboueke merged 2 commits into
mainfrom
develop
Apr 14, 2026
Merged

feat: per-journal Calendar page, anniversary filter, camera attach (v…#10
pboueke merged 2 commits into
mainfrom
develop

Conversation

@pboueke

@pboueke pboueke commented Apr 14, 2026

Copy link
Copy Markdown
Owner

v0.19.0

  • Calendar page at /journal/{id}/calendar with anniversary row + scroll of month cards highlighting days that have pages. Tap anniversary row or month card to filter the journal. Scroll offset is preserved per-journal across in-session navigation via CalendarScrollContext.
  • Anniversary filter in useFilter: pages whose UTC month+day match today from strictly prior years (Feb-29 only surfaces on leap Feb-29s). Activated from the calendar page via ?anniversary=1 URL param, cleared via existing FilterBar clear.
  • Calendar button with dot badge in JournalHeader when anniversaries exist.
  • Calendar -> filtered journal navigation uses router.dismissAll() so the journal's back button lands on home, not back on the calendar.
  • MonthPreview capped at 400px on web so the grid doesn't stretch.
  • Camera attach in page editor: AddAttachmentPopup gets Camera and Encrypted Camera buttons on native; expo-image-picker launchCameraAsync with permission prompt; photos stored identically to gallery images (type: 'image'), no data model changes. Added expo-image-picker plugin with photos+camera permission strings in app.json.
  • Route restructure: app/journal/[id].tsx -> app/journal/[id]/index.tsx, sibling app/journal/[id]/calendar.tsx.
  • i18n: new calendar.* namespace + journal.anniversary + page.takePhoto / takeEncryptedPhoto / cameraPermissionDenied across all 20 locales.
  • tests: 37 new tests (20 for src/lib/calendar, 6 useFilter anniversary, 6 CalendarScrollContext, 5 AnniversaryRow, 6 MonthPreview). 924 tests passing across 64 suites.

…0.19.0)

- Calendar page at /journal/{id}/calendar with anniversary row + scroll of
  month cards highlighting days that have pages. Tap anniversary row or
  month card to filter the journal. Scroll offset is preserved per-journal
  across in-session navigation via CalendarScrollContext.
- Anniversary filter in useFilter: pages whose UTC month+day match today
  from strictly prior years (Feb-29 only surfaces on leap Feb-29s).
  Activated from the calendar page via ?anniversary=1 URL param, cleared
  via existing FilterBar clear.
- Calendar button with dot badge in JournalHeader when anniversaries exist.
- Calendar -> filtered journal navigation uses router.dismissAll() so the
  journal's back button lands on home, not back on the calendar.
- MonthPreview capped at 400px on web so the grid doesn't stretch.
- Camera attach in page editor: AddAttachmentPopup gets Camera and
  Encrypted Camera buttons on native; expo-image-picker launchCameraAsync
  with permission prompt; photos stored identically to gallery images
  (type: 'image'), no data model changes. Added expo-image-picker plugin
  with photos+camera permission strings in app.json.
- Route restructure: app/journal/[id].tsx -> app/journal/[id]/index.tsx,
  sibling app/journal/[id]/calendar.tsx.
- i18n: new calendar.* namespace + journal.anniversary + page.takePhoto /
  takeEncryptedPhoto / cameraPermissionDenied across all 20 locales.
- tests: 37 new tests (20 for src/lib/calendar, 6 useFilter anniversary,
  6 CalendarScrollContext, 5 AnniversaryRow, 6 MonthPreview).
  924 tests passing across 64 suites.
@github-actions

Copy link
Copy Markdown

Carranca AI Review

PR #10 Review: Per-journal Calendar page, anniversary filter, camera attach


Summary

This PR adds a per-journal calendar screen (with anniversary detection and month-level navigation), a camera image capture option for page attachments, and the associated i18n strings across all 20 locales. The route structure is cleanly reorganized from app/journal/[id].tsx to app/journal/[id]/index.tsx + calendar.tsx. Overall the implementation is well-structured, well-tested (37 new tests, 924 total), and follows existing codebase conventions. Below are findings ordered by severity.


Findings

1. Duplicated anniversary matching logic — maintenance risk

Severity: medium
src/lib/calendar.ts:30-42 and src/hooks/useFilter.ts:58-64

The anniversary filter logic (same UTC month+day, strictly prior year) is implemented independently in both getAnniversaryPages() and matchesFilter(). Any future change to anniversary semantics (e.g., adding a grace period for Feb-29 on non-leap years) must be kept in sync across both files. Consider having useFilter delegate to getAnniversaryPages or extracting a shared isAnniversary(date, today) predicate from calendar.ts.

2. getMonthsWithPages sort 'none' falls through to descending

Severity: low
src/lib/calendar.ts:51-53

The sort comparator does return sort === 'ascending' ? ay - by : by - ay. When sort === 'none', this still sorts descending. While 'none' is never currently passed, the type signature explicitly allows it. Either remove 'none' from the type or add if (sort === 'none') return 0.

3. initialFilter ref captures stale URL params on re-mount

Severity: medium
app/journal/[id]/index.tsx:38-42

const initialFilter = useRef({
  anniversary: params.anniversary === '1',
  dateStart: params.dateStart,
  dateEnd: params.dateEnd,
}).current;

useRef(...).current is only evaluated once. This is the intended behavior (read-once seeding), but if the user navigates to a different filter URL for the same journal (e.g., tapping a different month from the calendar), the params change but initialFilter won't — it's already memoized. This works correctly only because router.dismissAll() + router.push() creates a new screen instance. If Expo Router ever reuses the component instance (e.g., due to fast refresh or stack behavior changes), the stale ref would be a bug. Consider using a useRef flag that gates a one-time useEffect reading params directly, or document this dependency clearly.

4. router.dismissAll() clears entire navigation stack

Severity: medium
app/journal/[id]/calendar.tsx:70-71, 78-79

Both onPressAnniversary and onPressMonth call router.dismissAll() before router.push(). This discards the entire modal/stack history, meaning the user cannot press "back" to return to the calendar after navigating to the filtered journal view. If this is intentional, it's fine — but it differs from typical drill-down patterns where back-navigation is expected. Worth confirming this matches the UX requirement.

5. Calendar scroll writes fire on every scroll event

Severity: low
app/journal/[id]/calendar.tsx:53-57

onScroll calls setOffset(id, contentOffset.y) on every scroll frame (throttled to 64ms via scrollEventThrottle). While setOffset only writes to a useRef Map (no re-renders), this is still frequent unnecessary work. Consider debouncing or using onMomentumScrollEnd / onScrollEndDrag to capture the final position only.

6. CalendarScrollContext accumulates entries without eviction

Severity: low
src/contexts/CalendarScrollContext.tsx:12

The offsets ref Map grows without bound as users visit different journals. In practice, this stays small, but there's no cleanup mechanism. Consider clearing entries when a journal is deleted, or capping the map size.

7. FilterBar formatDate uses local-time methods, inconsistent with UTC elsewhere

Severity: low
src/components/journal/FilterBar.tsx (pre-existing)

The existing formatDate helper uses d.getDate() / d.getMonth() (local time) while the new calendar code and useFilter anniversary logic use UTC-based comparisons (getUTCDate() / getUTCMonth()). This is pre-existing but worth noting since the new date-filter flow (calendar → journal) amplifies the inconsistency. A user in UTC+12 could see a different date displayed in the FilterBar vs. what was actually filtered.

8. Camera permission flow lacks requestLibraryPermissionsAsync parity

Severity: info
app/page/[id].tsx:202-219

The camera source path calls requestCameraPermissionsAsync() before launching the camera, which is correct. The library path (lines 218-222) does not call requestMediaLibraryPermissionsAsync() beforehand — it relies on launchImageLibraryAsync handling the permission prompt internally. This works but means permission denial UX differs between the two flows.

9. Hardcoded English string in error alert

Severity: low
app/page/[id].tsx:243

Alert.alert('Error', err instanceof Error ? err.message : String(err));

The title 'Error' is hardcoded instead of using the i18n system (t.page.error or similar). This is pre-existing code but the diff touches the function's dependency array, so it's worth flagging.

10. handleAddImage dependency array includes t but not source

Severity: info
app/page/[id].tsx:244

The useCallback dependency array was updated to include t (correct), but source is a parameter, not a closure variable, so it correctly doesn't need to be in the array. No issue here — just confirming correctness.

11. No accessibility label on BackButton used in CalendarHeader

Severity: low
src/components/common/BackButton.tsx (pre-existing)

The BackButton component lacks accessibilityLabel and accessibilityRole, which is inconsistent with other interactive elements. The calendar screen uses it, increasing its surface area.

12. MonthPreview hardcoded highlight text color to '#fff'

Severity: low
src/components/journal/MonthPreview.tsx:90

color: highlighted ? '#fff' : theme.colors.text,

Hardcoded white text for highlighted days may have contrast issues on light primary colors. The AnniversaryRow component correctly uses getContrastText(theme.colors.primary) for the same scenario. Consider using getContrastText here too for consistency.

13. AnniversaryRow zero-count row is still Pressable with no visual disabled state indication

Severity: info
src/components/journal/AnniversaryRow.tsx:39-48

When count === 0, the row renders as a Pressable with onPress: undefined and disabled: true. This means the opacity feedback on press is suppressed, but the row still appears tappable (just with muted colors). Consider adding opacity: 0.6 or similar in the disabled state for clearer visual feedback.

14. Test coverage is strong

Severity: info

37 new tests covering the calendar helpers (20), useFilter anniversary cases (6), CalendarScrollContext (6), AnniversaryRow (5), and MonthPreview (6). Edge cases tested include Feb-29, UTC boundary times, timezone offsets, malformed dates, and sort orders. Well done.

15. Route restructure is backward-compatible

Severity: info

Moving app/journal/[id].tsx to app/journal/[id]/index.tsx preserves URL resolution since Expo Router treats index files as the default route for their directory. No external links should break.


Verdict

APPROVE — with suggestions (not blockers):

  1. Extract a shared anniversary predicate to eliminate duplication between calendar.ts and useFilter.ts (medium priority).
  2. Fix the sort === 'none' latent bug in getMonthsWithPages (low priority).
  3. Use getContrastText() in MonthPreview instead of hardcoded '#fff' for highlighted day text (low priority).

The PR is well-structured, thoroughly tested, and follows existing patterns. No security, crypto, or critical correctness issues found.


carranca bound glm-5.1


Automated review by carranca sandbox

- Extract shared isAnniversary(page, today) predicate in src/lib/calendar.ts;
  useFilter's matchesFilter now delegates to it, eliminating the duplicated
  UTC month/day/year comparison.
- Fix getMonthsWithPages: sort === 'none' now preserves Map insertion order
  instead of silently falling through to descending. Added a dedicated test.
- MonthPreview highlighted day text uses getContrastText(theme.colors.primary)
  instead of hardcoded '#fff', matching the danger-button pattern.
@github-actions

Copy link
Copy Markdown

Carranca AI Review

Let me read the rest of the diff and examine the key source files.

PR #10 Code Review

Summary

This PR adds a per-journal calendar screen with anniversary filtering, scroll preservation across navigation, and camera attachment support. It restructures the journal route from [id].tsx to [id]/index.tsx (with a sibling calendar.tsx), extends useFilter with an anniversary boolean, and adds a CalendarScrollContext for session-scoped scroll offsets. The camera feature adds launchCameraAsync to the existing image attachment flow with proper permission handling.

Findings

1. [Medium] Unsafe Feather icon type cast — CalendarHeader.tsx:18

name={(journal.icon || 'book') as React.ComponentProps<typeof Feather>['name']}

If journal.icon contains a string not in the Feather glyph map, this crashes at runtime with no compile-time protection. Consider a validated lookup (e.g., check against Feather.glyphMap) or a try/catch render, matching whatever pattern the rest of the app uses for this field.

2. [Medium] useFilter creates new Date() inside useMemo — stale across midnight

useFilter.ts:86: const today = new Date() is captured when filteredPages memo recomputes. If a user leaves the journal screen open across midnight, the anniversary filter silently becomes stale (shows yesterday's anniversaries or misses today's). Same issue in calendar.tsx:48. Low practical impact for a mobile app, but worth a code comment or a midnight-triggered invalidation if precision matters.

3. [Medium] Missing screen-level integration tests

The plan (19.calendar.md) specifies integration tests for app/journal/[id]/calendar.tsx and app/journal/[id]/index.tsx, but neither test file appears in the diff. The core logic is well-tested at the unit level, but the URL-param seeding (?anniversary=1, ?dateStart=...), router.dismissAll() + router.push() navigation, and scroll restoration are untested integration paths.

4. [Low] router.dismissAll() + router.push() ordering assumption

calendar.tsx:80-81 and 88-89: router.dismissAll() then router.push() are called synchronously. Expo Router queues these navigation actions, so the push executes after the dismiss completes within the same tick. This works correctly in practice but relies on internal queueing behavior. A defensive router.dismissAll() followed by a setTimeout(() => router.push(...), 0) or using router.replace instead would be more explicit, though the current approach is acceptable.

5. [Low] isActive dependency in filteredPages useMemo is redundant

useFilter.ts:88: isActive is derived from filter and anniversary, both of which are already in the deps array. The extra isActive dependency doesn't cause bugs but makes the dependency contract unclear. Since the early return if (!isActive) return pages is the only use of isActive inside the memo, removing it from deps and inlining the check would be cleaner.

6. [Low] !journal fallback renders ActivityIndicator instead of null or immediate redirect

calendar.tsx:114-127: When !journal && !loading, the screen renders a spinner briefly before the useEffect on line 64 fires the redirect. The existing journal screen (index.tsx) has the same pattern, so this is consistent, but a null return would avoid the flash.

7. [Low] Camera permission not requested on web — but Platform.OS !== 'web' guard is in the popup, not the handler

AddAttachmentPopup.tsx:112 correctly hides camera options on web. However, handleAddImage in page/[id].tsx:201 accepts source: 'camera' and would attempt requestCameraPermissionsAsync if called. The guard is in the right place (the popup), but a defensive early-return in the handler for Platform.OS === 'web' would be safer if onTakePhoto is ever wired from another entry point.

8. [Info] Route restructure is backward-compatible

Moving app/journal/[id].tsxapp/journal/[id]/index.tsx is handled correctly by Expo Router. The Stack.Screen name="journal/[id]/index" registration in _layout.tsx preserves URL resolution.

9. [Info] CalendarScrollContext design is clean

Using useRef<Map> instead of state avoids re-renders on every scroll event. The useCallback + useMemo stabilization ensures stable references for consumers. Test coverage confirms no re-render on setOffset.

10. [Info] anniversary separation from canto-data Filter type is well-scoped

Keeping anniversary as hook-local state in useFilter rather than extending canto-data's Filter type avoids cross-package changes for a presentation-layer concern. Good architectural decision.

11. [Info] Test coverage for core logic is solid

37 new tests covering calendar helpers (20), useFilter anniversary (6), CalendarScrollContext (6), AnniversaryRow (5), and MonthPreview (6). All use jest.useFakeTimers() for time-sensitive assertions with proper cleanup.

12. [Info] Camera attachment follows existing encryption path

handleAddImage correctly routes camera-captured images through the same saveAttachment + updateDraft pipeline as library images, preserving the AES-256-GCM encryption flow for encrypted photos.

Verdict

COMMENT — The implementation is solid and well-tested at the unit level. The main gaps are the missing screen-level integration tests (#3) and the unsafe type assertion (#1). Neither is a blocker, but #1 should be validated against the existing pattern in the codebase (check how journal.icon is handled elsewhere) and #3 should be tracked as follow-up work. No security, encryption, or critical correctness issues found.

carranca bound glm-5.1


Automated review by carranca sandbox

@pboueke pboueke merged commit b4ac2bf into main Apr 14, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant