CU-868ex18rd Working on calendar and bug fixes #62
Conversation
WalkthroughAdds secure WebView HTML rendering (sanitization, dark-mode) across calendar and notes, introduces local-time date utilities and timezone-safe comparisons, enhances compose-message validation/unsaved-changes UX, adjusts CI/Dependabot and some API payloads/methods, broad test expansions and translation updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Sheet as CalendarItemDetailsSheet
participant Store as PersonnelStore
participant Web as WebView
participant API as Calendar API
participant Toast as useToast
User->>Sheet: Open (item present)
Sheet->>Store: check personnel (length / isLoading)
alt store empty & not loading
Sheet->>Store: fetchPersonnel()
Store-->>Sheet: personnel data
end
Sheet->>Web: render sanitized HTML (generateWebViewHtml)
User->>Sheet: Toggle attendance
Sheet->>API: setCalendarAttending({ CalendarEventId, Type, Note })
API-->>Sheet: response
alt success
Sheet->>API: fetchCalendarItem(item.Id)
Sheet->>Toast: show success
else failure
Sheet->>Toast: show error
end
sequenceDiagram
autonumber
actor User
participant Compose as ComposeMessageSheet
participant API as Messaging API
participant Analytics as useAnalytics
User->>Compose: Tap Send
Compose->>Compose: validateForm()
alt invalid
Compose-->>User: show inline errors
else valid
Compose->>API: sendNewMessage(payload)
API-->>Compose: success/failure
Compose->>Analytics: track send_success/send_failure
end
User->>Compose: Close
alt hasFormChanges
Compose-->>User: show discard confirmation
alt discard confirmed
Compose->>Analytics: track close {discardedChanges:true}
Compose-->>User: close
else cancel
Compose-->>User: stay open
end
else no changes
Compose-->>User: close
end
sequenceDiagram
autonumber
participant View as Calendar View
participant Utils as Date Utils
participant Store as Calendar Store
View->>Utils: getTodayLocalString / formatLocalDateString
View->>Store: loadCalendarItemsForDateRange(startLocal, endLocal)
Store-->>View: items
View->>Utils: isSameDate(item.Start, selectedDate)
Utils-->>View: true/false
View-->>User: render CompactCalendarItem list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/api/messaging/messages.ts (1)
78-82: Ensure DELETE payload is sent in the request bodyThe
createApiEndpoint.deletehelper insrc/api/common/client.tsxcurrently maps its single parameter to Axios’sparamskey, notdata. As a result, callingdeleteMessageApi.delete<DeleteMessageResult>({ MessageId: messageId });will issue a DELETE with
?MessageId=…rather than a JSON body, which will break any endpoint expecting a request body.To fix this, update the helper and its call sites:
• In src/api/common/client.tsx (around line 126), change the
deleteimplementation to accept a full Axios config:- delete: <T,>(params?: Record<string, unknown>) => - api.delete<T>(endpoint, { params }), + delete: <T,>(config?: AxiosRequestConfig) => + api.delete<T>(endpoint, config),• In src/api/messaging/messages.ts (lines 78–82), send the payload as
datarather than inline params:- const response = await deleteMessageApi.delete<DeleteMessageResult>({ - MessageId: messageId, - }); + const response = await deleteMessageApi.delete<DeleteMessageResult>({ + data: { MessageId: messageId }, + });This ensures the
MessageIdis included in the DELETE request body as intended.src/components/messages/__tests__/compose-message-sheet.test.tsx (1)
57-65: Replace HTMLin lucide mocks with React Native ViewUsing DOM elements in RN tests can cause “View config not found for name div” errors. Mock icons with React Native View instead.
-jest.mock('lucide-react-native', () => ({ - CalendarDays: ({ size, className }: any) => <div data-testid="calendar-days-icon" />, - Check: ({ size, className }: any) => <div data-testid="check-icon" />, - ChevronDown: ({ size, color }: any) => <div data-testid="chevron-down-icon" />, - Plus: ({ size, className }: any) => <div data-testid="plus-icon" />, - Send: ({ size, color }: any) => <div data-testid="send-icon" />, - Users: ({ size, className }: any) => <div data-testid="users-icon" />, - X: ({ size, className }: any) => <div data-testid="x-icon" />, -})); +jest.mock('lucide-react-native', () => ({ + CalendarDays: () => { + const { View } = require('react-native'); + return <View testID="calendar-days-icon" />; + }, + Check: () => { + const { View } = require('react-native'); + return <View testID="check-icon" />; + }, + ChevronDown: () => { + const { View } = require('react-native'); + return <View testID="chevron-down-icon" />; + }, + Plus: () => { + const { View } = require('react-native'); + return <View testID="plus-icon" />; + }, + Send: () => { + const { View } = require('react-native'); + return <View testID="send-icon" />; + }, + Users: () => { + const { View } = require('react-native'); + return <View testID="users-icon" />; + }, + X: () => { + const { View } = require('react-native'); + return <View testID="x-icon" />; + }, +}));src/components/calendar/enhanced-calendar-view.tsx (1)
56-83: Marking type mismatch: multi-day ranges not rendered with 'multi-dot'You’re computing startingDay/endingDay/color (period markings) but the Calendar uses markingType="multi-dot". As-is, only the start day gets a dot; intermediate days in a multi-day event are unmarked. Either switch to markingType="period" or add dots for every day in the range and remove period-only fields.
Minimal fix while keeping multi-dot:
-// If it's a multi-day event, mark the range +// If it's a multi-day event, mark every day in the range for multi-dot if (startDate !== endDate) { - const start = new Date(startDate); - const end = new Date(endDate); - const current = new Date(start); + // Build local Date objects to avoid UTC parsing + const [sy, sm, sd] = startDate.split('-').map(Number); + const [ey, em, ed] = endDate.split('-').map(Number); + const startLocal = new Date(sy, sm - 1, sd); + const endLocal = new Date(ey, em - 1, ed); + const current = new Date(startLocal); - while (current <= end) { + while (current <= endLocal) { const dateStr = formatLocalDateString(current); if (!marked[dateStr]) { marked[dateStr] = { marked: true, dots: [], }; } - - // Add period marking for multi-day events - if (dateStr === startDate) { - marked[dateStr].startingDay = true; - marked[dateStr].color = item.TypeColor || '#3B82F6'; - } else if (dateStr === endDate) { - marked[dateStr].endingDay = true; - marked[dateStr].color = item.TypeColor || '#3B82F6'; - } else { - marked[dateStr].color = item.TypeColor || '#3B82F6'; - } + // Add a dot for each day of the multi-day event + marked[dateStr].dots.push({ + key: item.CalendarItemId, + color: item.TypeColor || '#3B82F6', + }); current.setDate(current.getDate() + 1); } }Alternatively, if you prefer range visualization, change
markingType="period"and keep the startingDay/endingDay logic (but still fix the UTC parsing as above).Also applies to: 183-184
src/components/calendar/__tests__/calendar-card.test.tsx (1)
296-304: Fix press handler test to target the Pressable, not inner textPressing the title Text may not invoke Pressable’s onPress with the current mocks. Trigger the event on the Pressable testID to avoid flakiness.
Apply this diff:
- const { getByText } = render( + const { getByTestId } = render( <CalendarCard item={item} onPress={mockOnPress} /> ); - fireEvent.press(getByText('Test Event')); + fireEvent.press(getByTestId('pressable')); expect(mockOnPress).toHaveBeenCalledTimes(1);src/components/messages/compose-message-sheet.tsx (2)
515-538: Units tab missing from the tab selector (functional gap)You render Units content later, but the tab list only includes personnel, groups, roles. Users can’t switch to Units.
- {['personnel', 'groups', 'roles'].map((tab) => ( + {['personnel', 'groups', 'roles', 'units'].map((tab) => ( <Pressable key={tab} className={`flex-1 rounded-lg px-1 py-2 ${currentRecipientTab === tab ? 'bg-primary-500' : 'bg-gray-200 dark:bg-gray-600'}`} onPress={() => { const oldTab = currentRecipientTab; setCurrentRecipientTab(tab as any); // tracking ... }} > - <Text className={`text-center text-sm font-medium ${currentRecipientTab === tab ? 'text-white' : 'text-gray-700 dark:text-gray-300'}`}>{t(`calls.${tab}`)}</Text> + <Text className={`text-center text-sm font-medium ${currentRecipientTab === tab ? 'text-white' : 'text-gray-700 dark:text-gray-300'}`}>{t(`calls.${tab}`)}</Text> </Pressable> ))}
240-253: Handle singular/plural recipient types and provide a safe defaultSome data sets use singular (
Group,Unit) while others use plural. Expand mapping to avoid misclassification.-const getRecipientType = (typeString: string): number => { - switch (typeString) { - case 'Personnel': - return 1; - case 'Groups': - return 2; - case 'Roles': - return 3; - case 'Unit': - return 4; - default: - return 1; - } -}; +const getRecipientType = (typeString: string): number => { + const t = (typeString || '').toLowerCase(); + if (t === 'personnel' || t === 'user' || t === 'users') return 1; + if (t === 'group' || t === 'groups') return 2; + if (t === 'role' || t === 'roles') return 3; + if (t === 'unit' || t === 'units') return 4; + return 1; // default to personnel +};
🧹 Nitpick comments (70)
.github/dependabot.yml (1)
10-10: Pauses npm version-update PRs (limit=0). Confirm intent; consider batching or re-targeting instead.Setting
open-pull-requests-limit: 0effectively prevents Dependabot from opening version-update PRs for the npm updater. If your goal is to reduce PR noise (not fully pause), consider batching minor/patch updates, lowering the cap, and/or targeting your integration branch.Apply this minimal change if you just want to reduce churn while keeping updates flowing:
- open-pull-requests-limit: 0 + open-pull-requests-limit: 2Optional improvements (illustrative YAML) to batch updates weekly and target your working branch:
updates: - package-ecosystem: "npm" directory: "/" target-branch: "develop" # if PRs should go to develop, not master groups: all-minor-patch: applies-to: version-updates update-types: - "minor" - "patch" schedule: interval: "weekly" day: "monday" time: "09:00" timezone: "America/Los_Angeles"Note: Version-update settings don’t typically govern security update PRs configured via repo settings. If your intent is a full pause, keep
0and add a comment explaining when to revert. Otherwise, prefer batching + a small cap.src/api/calendar/calendar.ts (1)
43-45: Replace magic numbers with named mapping; confirm server codes 1/4 are finalUsing raw
1/4reduces readability and is brittle if the API changes. Introduce a small mapper or enum and use it here. Also, confirm that the contract is indeed1for attending and4for not attending.Proposed change within this block:
- CalendarEventId: params.calendarItemId, - Type: params.attending === true ? 1 : 4, + CalendarEventId: params.calendarItemId, + Type: attendingToApiType(params.attending), Note: params.note || '',Add this helper near the top of the file (outside this block):
// Prefer a tiny mapping function over magic numbers const attendingToApiType = (attending: boolean): 1 | 4 => (attending ? 1 : 4); // Or, if you prefer explicit names: // enum CalendarAttendanceType { Attending = 1, NotAttending = 4 } // const attendingToApiType = (a: boolean): CalendarAttendanceType => a ? CalendarAttendanceType.Attending : CalendarAttendanceType.NotAttending;src/components/sidebar/side-menu.tsx (2)
194-203: Simplify nested ternary: connected branch duplicates dark/light classesWhen
isConnectedis true, both dark/light paths return the same classes. Collapse to a single branch for readability.Apply:
- className={`flex-1 rounded-lg border px-3 py-2 ${ - isConnected - ? isDark - ? 'border-green-600 bg-green-600 hover:bg-green-700 active:bg-green-800' - : 'border-green-600 bg-green-600 hover:bg-green-700 active:bg-green-800' - : isDark - ? 'border-green-600 bg-transparent hover:bg-green-900/20 active:bg-green-900/30' - : 'border-green-600 bg-transparent hover:bg-green-50 active:bg-green-100' - }`} + className={`flex-1 rounded-lg border px-3 py-2 ${ + isConnected + ? 'border-green-600 bg-green-600 hover:bg-green-700 active:bg-green-800' + : isDark + ? 'border-green-600 bg-transparent hover:bg-green-900/20 active:bg-green-900/30' + : 'border-green-600 bg-transparent hover:bg-green-50 active:bg-green-100' + }`}
220-227: Ditto: remove redundant dark/light split for the “playing” branchThe connected/playing branch yields identical classes regardless of theme.
Apply:
- className={`flex-1 rounded-lg border px-3 py-2 ${ - currentStream && isPlaying - ? isDark - ? 'border-blue-600 bg-blue-600 hover:bg-blue-700 active:bg-blue-800' - : 'border-blue-600 bg-blue-600 hover:bg-blue-700 active:bg-blue-800' - : isDark - ? 'border-blue-600 bg-transparent hover:bg-blue-900/20 active:bg-blue-900/30' - : 'border-blue-600 bg-transparent hover:bg-blue-50 active:bg-blue-100' - }`} + className={`flex-1 rounded-lg border px-3 py-2 ${ + currentStream && isPlaying + ? 'border-blue-600 bg-blue-600 hover:bg-blue-700 active:bg-blue-800' + : isDark + ? 'border-blue-600 bg-transparent hover:bg-blue-900/20 active:bg-blue-900/30' + : 'border-blue-600 bg-transparent hover:bg-blue-50 active:bg-blue-100' + }`}.github/workflows/react-native-cicd.yml (1)
161-172: Harden cross-OS jq install for CI robustnessMinor: set noninteractive apt flags and avoid long brew updates; also handle existing jq by no-op installs.
Apply:
- # Ensure jq is available on both Linux and macOS + # Ensure jq is available on both Linux and macOS if ! command -v jq &> /dev/null; then echo "Installing jq..." - if [ "${RUNNER_OS}" = "Linux" ]; then - sudo apt-get update && sudo apt-get install -y jq + if [ "${RUNNER_OS}" = "Linux" ]; then + export DEBIAN_FRONTEND=noninteractive + sudo apt-get -yqq update + sudo apt-get -yqq install jq elif [ "${RUNNER_OS}" = "macOS" ]; then - brew update || true - brew install jq + export HOMEBREW_NO_AUTO_UPDATE=1 + brew install jq || brew upgrade jq || true else echo "Unsupported runner OS: ${RUNNER_OS}" >&2 exit 1 fi fisrc/lib/utils.ts (1)
258-286: Harden isSameDate against invalid inputs and normalize via local date keyIf either parsed date is invalid, current code silently compares NaN-derived values and returns false; also, the comparison can be simplified by reusing formatLocalDateString. Suggest:
Apply this diff:
-export function isSameDate(date1: string | Date, date2: string | Date): boolean { - // Helper function to create a date from string, handling date-only strings as local dates - const createDate = (date: string | Date): Date => { - if (date instanceof Date) { - return date; - } - - // If it's a date-only string (YYYY-MM-DD), treat it as local date - if (/^\d{4}-\d{2}-\d{2}$/.test(date)) { - const [year, month, day] = date.split('-').map(Number); - return new Date(year, month - 1, day); // Month is 0-indexed - } - - // Otherwise, parse as usual (handles ISO strings with time) - return new Date(date); - }; - - const d1 = createDate(date1); - const d2 = createDate(date2); - - // Use local date methods for comparison to match user's timezone context - // This ensures calendar items appear on the correct day as intended by the backend - return d1.getFullYear() === d2.getFullYear() && d1.getMonth() === d2.getMonth() && d1.getDate() === d2.getDate(); -} +export function isSameDate(date1: string | Date, date2: string | Date): boolean { + const toDate = (val: string | Date): Date => { + if (val instanceof Date) return val; + if (/^\d{4}-\d{2}-\d{2}$/.test(val)) { + const [y, m, d] = val.split('-').map(Number); + return new Date(y, m - 1, d); + } + return new Date(val); + }; + + const d1 = toDate(date1); + const d2 = toDate(date2); + + if (Number.isNaN(d1.getTime()) || Number.isNaN(d2.getTime())) return false; + + return formatLocalDateString(d1) === formatLocalDateString(d2); +}Also consider adding tests for date-only strings and invalid inputs (see test suggestion below).
src/components/calendar/calendar-view.tsx (2)
62-64: Using item.Start for day matching — consistent with .NET API, but precompute for O(1) lookupsThe approach is correct. For large months, consider precomputing a Set of local day keys for selectedMonthItems to avoid O(n) scans per cell.
Example (outside render):
const eventDaySet = useMemo(() => { const s = new Set<string>(); for (const it of selectedMonthItems) s.add(new Date(it.Start).toDateString()); return s; }, [selectedMonthItems]); const hasEventsOnDate = (date: Date) => eventDaySet.has(date.toDateString());
121-126: Use ternary instead of && for conditional rendering (project guideline)The codebase guidelines prefer ?: over && for conditionals. Replace the dot indicator render with a ternary.
Apply this diff:
- {hasEvents && <View className={['absolute bottom-1 h-1 w-1 rounded-full', isSelected ? 'bg-white' : 'bg-amber-500'].join(' ')} />} + {hasEvents ? ( + <View className={['absolute bottom-1 h-1 w-1 rounded-full', isSelected ? 'bg-white' : 'bg-amber-500'].join(' ')} /> + ) : null}src/lib/__tests__/utils-date.test.ts (1)
1-92: Add tests for date-only strings and invalid inputsTo lock in the intended behavior of isSameDate/isToday with 'YYYY-MM-DD' strings and guard against invalid dates, extend coverage as below.
Apply this diff at the end of the file:
@@ }); }); + +describe('Date Utilities - additional cases', () => { + beforeEach(() => { + jest.useFakeTimers(); + jest.setSystemTime(new Date('2024-01-15T10:00:00Z')); + }); + afterEach(() => jest.useRealTimers()); + + it('isSameDate should handle date-only strings as local dates', () => { + expect(isSameDate('2024-01-15', '2024-01-15T23:59:59Z')).toBe(true); + expect(isSameDate('2024-01-15', '2024-01-16T00:00:00Z')).toBe(false); + }); + + it('isSameDate should return false for invalid inputs', () => { + // @ts-expect-error testing runtime behavior with invalid strings + expect(isSameDate('not-a-date', '2024-01-15')).toBe(false); + }); + + it('isToday should handle date-only strings', () => { + expect(isToday('2024-01-15')).toBe(true); + expect(isToday('2024-01-16')).toBe(false); + }); +});src/components/calendar/compact-calendar-item.tsx (2)
50-53: Accessibility: mark Pressable as a buttonSmall win for screen readers.
Apply this diff:
- <Pressable onPress={onPress} testID={testID} className="mb-2"> + <Pressable onPress={onPress} testID={testID} accessibilityRole="button" className="mb-2">
20-21: Consider memoizing component to reduce list re-rendersIf rendered in lists, wrapping in React.memo can avoid unnecessary updates when props are referentially equal.
Example:
export const CompactCalendarItem = React.memo<CompactCalendarItemProps>(/* existing FC */);src/translations/es.json (1)
609-616: Remove unused top-level translation keysThe codebase only ever calls
t('messages.validation.body_required') t('messages.validation.recipients_required') t('messages.validation.subject_required')and never references the legacy
messages.body_required,messages.recipients_requiredormessages.subject_required. You can safely clean up the duplicate entries in the Spanish locale.• File:
src/translations/es.json
• Remove these keys (around lines 610–616):}, - "unsaved_changes": "Cambios No Guardados", - "unsaved_changes_message": "Tienes cambios no guardados. ¿Estás seguro de que quieres descartarlos?", - "validation": { - "body_required": "El cuerpo del mensaje es obligatorio", - "recipients_required": "Se requiere al menos un destinatario", - "subject_required": "El asunto es obligatorio" - } + "unsaved_changes": "Cambios No Guardados", + "unsaved_changes_message": "Tienes cambios no guardados. ¿Estás seguro de que quieres descartarlos?", + "validation": { + "body_required": "El cuerpo del mensaje es obligatorio", + "recipients_required": "Se requiere al menos un destinatario", + "subject_required": "El asunto es obligatorio" + }• Delete the orphaned top-level keys:
"body_required": "...", "recipients_required": "...", "subject_required": "..."This eliminates duplication and keeps only the active namespace.
src/utils/webview-html.ts (3)
1-9: Export the options interface for reuseExporting WebViewHtmlOptions enables consumers to type their local utilities and props.
-interface WebViewHtmlOptions { +export interface WebViewHtmlOptions { content: string; isDarkMode: boolean; fontSize?: number; lineHeight?: number; padding?: number; backgroundColor?: string; textColor?: string; }
85-86: Clamp code font size to avoid negative or tiny valuesfontSize - 2 can underflow if callers pass small values. Clamp to a sensible minimum.
- font-size: ${fontSize - 2}px; + font-size: ${Math.max(fontSize - 2, 10)}px;
119-132: Type props as WebViewProps and include 'data:' in origin whitelist
- Type the object to catch future prop typos.
- originWhitelist typically needs 'about:' and 'data:' for inline HTML sources across platforms.
- Consider making androidLayerType configurable; forcing 'software' globally can regress perf on some devices.
-import const -export const defaultWebViewProps = { +import type { WebViewProps } from 'react-native-webview'; + +export const defaultWebViewProps: Pick<WebViewProps, + | 'originWhitelist' + | 'javaScriptEnabled' + | 'domStorageEnabled' + | 'startInLoadingState' + | 'mixedContentMode' + | 'androidLayerType' + | 'showsVerticalScrollIndicator' + | 'showsHorizontalScrollIndicator' +> = { // Security: Only allow local content, no external origins - originWhitelist: ['about:'], + originWhitelist: ['about:', 'data:'], // Security: Disable JavaScript and DOM storage by default javaScriptEnabled: false, domStorageEnabled: false, // Performance and UX startInLoadingState: false, mixedContentMode: 'compatibility' as const, androidLayerType: 'software' as const, // Scroll behavior showsVerticalScrollIndicator: true, showsHorizontalScrollIndicator: false, };src/lib/__tests__/utils.test.ts (2)
180-207: Test title mentions null/undefined but doesn’t assert themThe "handles null and undefined gracefully" case only asserts a happy-path call. Either rename or add assertions for null/undefined inputs where the API accepts union types.
- describe('Edge Cases and Error Handling', () => { + describe('Edge Cases and Error Handling', () => { @@ - it('handles null and undefined gracefully', () => { - const validDate = new Date(2024, 0, 15); - - // These might be passed due to type issues in JavaScript - expect(() => formatLocalDateString(validDate)).not.toThrow(); - }); + it('handles null and undefined gracefully (no throws, false comparisons)', () => { + const validDate = new Date(2024, 0, 15); + // @ts-expect-error deliberate invalid inputs to verify runtime robustness + expect(() => isSameDate(null, validDate)).not.toThrow(); + // @ts-expect-error deliberate invalid inputs to verify runtime robustness + expect(() => isSameDate(validDate, undefined)).not.toThrow(); + // @ts-expect-error runtime returns false for invalids + expect(isSameDate(null, validDate)).toBe(false); + // @ts-expect-error runtime returns false for invalids + expect(isSameDate(validDate, undefined)).toBe(false); + });
108-116: Add an invalid Date object caseCovers NaN Dates constructed via new Date('invalid'), which can leak into code paths from external inputs.
it('handles ISO strings with time correctly', () => { @@ expect(isSameDate(date1, date3)).toBe(false); }); + + it('handles invalid Date objects (NaN) without throwing', () => { + // @ts-expect-error constructing invalid date intentionally + const badDate = new Date('invalid'); + const goodDate = new Date(2024, 0, 15); + expect(() => isSameDate(badDate, goodDate)).not.toThrow(); + expect(isSameDate(badDate as unknown as Date, goodDate)).toBe(false); + });src/components/contacts/contact-notes-list.tsx (4)
33-36: Remove unused variable and lean on shared utilsbackgroundColor is computed but unused. Also, instead of re-deriving colors for WebView styling here, prefer the centralized generator in src/utils/webview-html.ts for consistency.
- const { colorScheme } = useColorScheme(); - const textColor = colorScheme === 'dark' ? '#FFFFFF' : '#000000'; - const backgroundColor = colorScheme === 'dark' ? '#374151' : '#F9FAFB'; + const { colorScheme } = useColorScheme();
236-261: Track analytics after data loads to avoid noisy eventsAs written, the effect can fire multiple times (notes.length / hasNotes change). Track once post-load per contactId, or when isNotesLoading transitions to false.
- // Track when contact notes list is rendered - React.useEffect(() => { - if (contactId) { - trackEvent('contact_notes_list_rendered', { - contactId: contactId, - notesCount: notes.length, - hasNotes: hasNotes, - isLoading: isNotesLoading, - }); - } - }, [trackEvent, contactId, notes.length, hasNotes, isNotesLoading]); + // Track when notes are loaded for a contact + React.useEffect(() => { + if (contactId && !isNotesLoading) { + trackEvent('contact_notes_list_rendered', { + contactId, + notesCount: notes.length, + hasNotes, + }); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [contactId, isNotesLoading]);
27-35: Memoize ContactNoteCard to reduce re-rendersNotes list can be long; memoize by props to avoid unnecessary rerenders on list updates.
-const ContactNoteCard: React.FC<ContactNoteCardProps> = ({ note }) => { +const ContactNoteCardComponent: React.FC<ContactNoteCardProps> = ({ note }) => { ... -}; +}; + +const ContactNoteCard = React.memo(ContactNoteCardComponent);
303-313: Remove unused styles to reduce noisestyles.container is defined but not used.
const styles = StyleSheet.create({ - container: { - width: '100%', - backgroundColor: 'transparent', - }, webView: { height: 200, // Fixed height with scroll capability backgroundColor: 'transparent', width: '100%', }, });docs/calendar-card-webview-refactoring.md (1)
76-90: Example should reflect shared defaultsMirror the shared defaults, including origin whitelist for data/about.
-<WebView - {...defaultWebViewProps} +<WebView + {...defaultWebViewProps} source={{ html: generateWebViewHtml({ content: htmlContent, isDarkMode: colorScheme === 'dark', }), }} />docs/calendar-item-details-personnel-loading-enhancement.md (1)
41-51: Scope the loading UI to avoid masking known namesCurrent guidance returns t('loading') whenever isInitializing || isPersonnelLoading, which can briefly replace a previously resolved creator name during background refetches. Prefer returning the known name if it’s already in the store, and only show loading when a lookup is actually required.
Example adjustment to the snippet:
-// Loading detection in getCreatorName -if (isInitializing || isPersonnelLoading) { - return t('loading'); -} +// Loading detection in getCreatorName +const creator = personnel.find(p => p.UserId === item.CreatorUserId); +if (creator) { + return `${creator.FirstName} ${creator.LastName}`.trim(); +} +if (isInitializing || isPersonnelLoading) { + return t('loading'); +} +return t('unknown_user');src/components/messages/__tests__/compose-message-sheet.test.tsx (2)
551-575: Prefer stable testIDs over translation-dependent selectorsgetByText('messages.send') and placeholders tied to translation keys are brittle. Expose testIDs in ComposeMessageSheet (e.g., send-button, subject-input) and use them here for stability across locales.
Example change in the test:
-const sendButton = getByText('messages.send'); +const sendButton = getByTestId('send-button');And add in ComposeMessageSheet:
-<Button ... onPress={handleSend}> +<Button testID="send-button" ... onPress={handleSend}>
33-51: Pluralization mock is not representative; improve count handlingThe i18n mock replaces '{{count}}' within the key string, which most keys won’t contain. Returning a deterministic string that includes count helps assertions for pluralized messages.
- if (options && typeof options === 'object' && 'count' in options) { - return `${key.replace('{{count}}', options.count)}`; - } + if (options && typeof options === 'object' && 'count' in options) { + return `${key}:${options.count}`; + }src/components/calendar/__tests__/calendar-item-details-sheet-minimal.test.tsx (2)
35-43: Be cautious with broad react-native mocksOverriding react-native with a minimal surface can hide regressions if the component starts using unmocked APIs (e.g., Platform, Dimensions). Consider leveraging the default RN Jest preset and only mocking specific primitives you need.
145-163: Analytics assertions look solidThe payload fields align with likely production tracking. Consider adding one test covering error handling (e.g., trackEvent throwing) similar to the compose message tests to assert graceful degradation.
src/app/(app)/calendar.tsx (4)
109-116: Memoize and null-guard selected-date filteringgetItemsForSelectedDate() is recomputed multiple times per render and assumes selectedMonthItems is defined. Memoize and guard to avoid repeated work and potential NPEs.
-const getItemsForSelectedDate = () => { - if (!selectedDate) return []; - - return selectedMonthItems.filter((item) => { - // Use Start field for consistent date comparison with .NET backend timezone-aware dates - return isSameDate(item.Start, selectedDate); - }); -}; +const itemsForSelectedDate = React.useMemo(() => { + if (!selectedDate) return []; + const src = selectedMonthItems ?? []; + return src.filter((item) => isSameDate(item.Start, selectedDate)); +}, [selectedDate, selectedMonthItems]);And update usages below to reference itemsForSelectedDate.
139-140: Stabilize renderItem callbacks with useCallbackTo align with the project guideline “Avoid anonymous functions in renderItem or event handlers,” wrap renderers in useCallback. This reduces churn in FlatList and can improve perf.
-const renderCompactCalendarItem = ({ item }: { item: CalendarItemResultData }) => <CompactCalendarItem item={item} onPress={() => handleItemPress(item)} />; +const renderCompactCalendarItem = React.useCallback( + ({ item }: { item: CalendarItemResultData }) => ( + <CompactCalendarItem item={item} onPress={() => handleItemPress(item)} /> + ), + [handleItemPress] +);Similarly consider wrapping handleItemPress itself in useCallback.
205-217: Avoid repeated filtering in render; simplify conditionalUse the memoized itemsForSelectedDate to compute once and simplify the ternary.
- {isLoading ? ( - <Loading text={t('calendar.loading.date')} /> - ) : getItemsForSelectedDate().length === 0 ? ( - <Text className="py-8 text-center text-gray-500 dark:text-gray-400">{t('calendar.selectedDate.empty')}</Text> - ) : ( - <FlatList data={getItemsForSelectedDate()} renderItem={renderCompactCalendarItem} keyExtractor={(item) => item.CalendarItemId} showsVerticalScrollIndicator={false} /> - )} + {isLoading ? ( + <Loading text={t('calendar.loading.date')} /> + ) : itemsForSelectedDate.length === 0 ? ( + <Text className="py-8 text-center text-gray-500 dark:text-gray-400">{t('calendar.selectedDate.empty')}</Text> + ) : ( + <FlatList + data={itemsForSelectedDate} + renderItem={renderCompactCalendarItem} + keyExtractor={(item) => item.CalendarItemId} + showsVerticalScrollIndicator={false} + /> + )}
160-170: Optional: tune FlatList perfAdd props like removeClippedSubviews, windowSize, and maxToRenderPerBatch for long lists. Not mandatory, but aligns with app guidelines.
<FlatList data={todayCalendarItems} renderItem={renderCalendarItem} keyExtractor={(item) => item.CalendarItemId} className="flex-1" contentContainerStyle={{ padding: 16 }} showsVerticalScrollIndicator={false} + removeClippedSubviews + windowSize={5} + maxToRenderPerBatch={10} refreshControl={<RefreshControl refreshing={isTodaysLoading} onRefresh={handleRefresh} colors={['#3B82F6']} tintColor="#3B82F6" />} />Apply similarly to the upcoming list if needed.
Also applies to: 192-201
src/utils/__tests__/webview-html.test.ts (3)
29-45: Add guard/test for code block font-size floorgenerateWebViewHtml derives code font-size as fontSize - 2. If fontSize is small (<= 2), this can underflow to 0px or negative. Add a floor in the generator (e.g., Math.max(fontSize - 2, 10)) and a test for small font sizes.
Apply this test and generator adjustment:
- expect(html).toContain('font-size: 18px'); + expect(html).toContain('font-size: 18px'); expect(html).toContain('line-height: 1.8'); expect(html).toContain('padding: 12px'); expect(html).toContain('color: #FF0000'); expect(html).toContain('background-color: #00FF00');Outside this file (generator), clamp the code font-size:
- pre, code { + pre, code { background-color: ${codeBackgroundColor}; padding: 8px; border-radius: 4px; font-family: 'Monaco', 'Menlo', 'Ubuntu Mono', monospace; - font-size: ${fontSize - 2}px; + font-size: ${Math.max(fontSize - 2, 10)}px; }
47-54: Consider adding a CSP meta test (privacy hardening)Remote images/scripts in HTML could leak requests. You’ve mitigated script execution via javaScriptEnabled=false, but subresource loads (e.g.,
) can still occur. Consider injecting a restrictive Content Security Policy and add an assertion here to ensure it’s present.
Example CSP to add in the generator (outside this file):
<meta http-equiv="Content-Security-Policy" content="default-src 'none'; img-src data: blob:; style-src 'unsafe-inline'; frame-ancestors 'none'; form-action 'none'">Then assert its presence in this test.
71-81: Prop names and secure defaults look correct; add a couple more props if neededoriginWhitelist, javaScriptEnabled, and domStorageEnabled are asserted correctly. If your usage doesn’t rely on it, consider also setting allowsInlineMediaPlayback=false and allowingFileAccessFromFileURLs=false in defaultWebViewProps and asserting them here for defense-in-depth.
src/components/calendar/__tests__/calendar-card.test.tsx (2)
18-26: Avoid duplicating the WebView utility mock across test suitesYou redefine generateWebViewHtml/defaultWebViewProps here while similar mocks exist elsewhere (and jest-setup). Centralize this in jest-setup to reduce drift and keep behavior consistent between suites.
128-135: Add dark mode signal and status icon testsTwo targeted additions will increase confidence:
- Verify isDarkMode=true is forwarded to generateWebViewHtml when color scheme is dark.
- Verify the CheckCircle icon renders when Attending && canSignUp.
Apply this diff near the end of the suite (before the closing "});"):
+ it('passes isDarkMode=true to generateWebViewHtml when color scheme is dark', () => { + const item = createMockItem(); + const { useColorScheme } = require('nativewind'); + (useColorScheme as jest.Mock).mockReturnValue({ colorScheme: 'dark' }); + + render(<CalendarCard item={item} onPress={mockOnPress} />); + const { generateWebViewHtml } = require('@/utils/webview-html'); + expect(generateWebViewHtml).toHaveBeenCalledWith( + expect.objectContaining({ isDarkMode: true }) + ); + }); + + it('shows CheckCircle icon when user is signed up and can sign up', () => { + const item = createMockItem({ SignupType: 1, LockEditing: false, Attending: true }); + const { getByText } = render(<CalendarCard item={item} onPress={mockOnPress} />); + expect(getByText('CheckCircle')).toBeTruthy(); + });src/stores/calendar/store.ts (3)
121-135: Log range used for the fetch (helps triage mismatches)Include startDate/endDate in the success log to correlate server results with the client-side filter.
Apply this diff:
- context: { - totalCount: response.Data.length, - filteredCount: todayItems.length, - //startDate, - //endDate, + context: { + totalCount: response.Data.length, + filteredCount: todayItems.length, + startDate, + endDate, items: todayItems.map((item) => ({ id: item.CalendarItemId, title: item.Title, start: item.Start, startDate: new Date(item.Start).toDateString(), })), },
110-116: Client-side date filter is a good safety net; consider documenting API expectationsUsing isSameDate against item.Start adds resilience against backend TZ quirks. Add a brief comment noting the backend’s Start semantics (.NET timezone-aware) to prevent “simplifications” later.
171-179: Optional: unify date range format across loadersloadUpcomingCalendarItems uses 'yyyy-MM-dd HH:mm:ss', while loadCalendarItems sends 'YYYY-MM-DD' only. Consider normalizing to start-of-day/end-of-day with the same format for predictability.
Apply this optional diff:
- const startDate = subDays(new Date(), 90).toISOString().split('T')[0]; - const endDate = addDays(new Date(), 120).toISOString().split('T')[0]; + const startDate = format(startOfDay(subDays(new Date(), 90)), 'yyyy-MM-dd HH:mm:ss'); + const endDate = format(endOfDay(addDays(new Date(), 120)), 'yyyy-MM-dd HH:mm:ss');src/translations/ar.json (2)
62-62: Shortened calendar.confirmSignup stringThe change to "تأكيد" is consistent with the English “Confirm.” Verify button width/truncation in RTL layouts.
711-714: Remove unused translation key “generale” from ar.json
The key settings.generale is not referenced anywhere in the codebase and duplicates the existing settings.general. To keep the translations tidy, you can safely remove it.• File src/translations/ar.json
– Remove the line:
diff - "generale": "عام",src/components/calendar/__tests__/component-comparison.test.tsx (2)
24-27: Don’t override the global WebView mock hereThis local mock returns null and prevents asserting WebView presence. Remove it and rely on jest-setup’s WebView mock to enable richer assertions across suites.
Apply this diff to delete the local mock:
- jest.mock('react-native-webview', () => ({ - __esModule: true, - default: () => null, - }));
177-192: Turn the commentary into an assertionYou note the compact component shouldn’t rely on WebView. Assert it explicitly by checking CalendarCard has description-webview while CompactCalendarItem does not.
After removing the local WebView mock, append this test:
+ it('CalendarCard renders description WebView, CompactCalendarItem does not', () => { + const mockOnPress = jest.fn(); + const fullCard = render(<CalendarCard item={mockItem} onPress={mockOnPress} />); + expect(fullCard.getByTestId('description-webview')).toBeTruthy(); + + const compactCard = render(<CompactCalendarItem item={mockItem} onPress={mockOnPress} />); + expect(compactCard.queryByTestId('description-webview')).toBeNull(); + });docs/calendar-item-details-webview-implementation.md (2)
20-27: Doc/code drift: scroll indicator and security props differ from implementation
- Doc states showsVerticalScrollIndicator={false}, but defaultWebViewProps sets it to true. Either override it in components (as calendar-card does) or update the doc to say “components may override scroll indicators.”
- Doc doesn’t mention javaScriptEnabled/domStorageEnabled being disabled by default; add a note so readers don’t re-enable by accident.
I can submit a PR to this doc aligning it with src/utils/webview-html.ts defaults if you prefer.
93-98: Performance guidance: clarify height caps per componentDoc mentions a 120px cap; calendar-card uses 60px while details sheets may use taller. Clarify that the compact card preview uses 60px and the details sheet uses 120px to avoid confusion when QA verifies layouts.
src/components/calendar/calendar-card.tsx (2)
95-114: Memoize the HTML and explicitly hide scroll indicators to reduce WebView churnGenerating the HTML string inline causes a new string each render, which can trigger unnecessary WebView reloads in FlatLists. Also, defaultWebViewProps enables the vertical indicator; the card disables scrolling, so show no indicators.
Apply this refactor:
-import React from 'react'; +import React, { useMemo } from 'react'; @@ - const isDarkMode = colorScheme === 'dark'; + const isDarkMode = colorScheme === 'dark'; + + // Avoid regenerating HTML on every render + const descriptionHtml = useMemo( + () => + item.Description + ? generateWebViewHtml({ + content: item.Description, + isDarkMode, + fontSize: 14, + lineHeight: 1.4, + padding: 6, + }) + : '', + [item.Description, isDarkMode] + ); @@ - <Box className="w-full rounded bg-gray-50 p-1 dark:bg-gray-700"> + <Box className="w-full rounded bg-gray-50 p-1 dark:bg-gray-700"> <WebView style={styles.webView} {...defaultWebViewProps} - scrollEnabled={false} - source={{ - html: generateWebViewHtml({ - content: item.Description, - isDarkMode, - fontSize: 14, - lineHeight: 1.4, - padding: 6, - }), - }} + scrollEnabled={false} + showsVerticalScrollIndicator={false} + showsHorizontalScrollIndicator={false} + source={{ html: descriptionHtml }} testID="description-webview" /> </Box>
29-42: Timezone correctness for user-visible timesUsing new Date(dateString).toLocaleTimeString/DateString without an explicit timezone may render unexpected times if the backend date strings are not normalized to the device’s local zone. If your API guarantees ISO strings in local time, you’re fine. Otherwise, consider centralizing formatting in a small utility (e.g., using date-fns-tz) to ensure consistent local-time display and to simplify testing.
Would you like me to wire a tiny formatLocalDate/formatLocalTime helper and unit tests?
src/components/calendar/__tests__/calendar-item-details-sheet-analytics.test.tsx (1)
136-141: Add an assertion that personnel auto-fetch occurs on open when store is emptyThe mock component triggers fetchPersonnel when opened and the store is empty. The test suite sets up mockFetchPersonnel but never asserts it’s called. Adding a focused test increases confidence in the side effect.
Apply this test addition:
@@ it('tracks analytics when sheet becomes visible', () => { @@ }); + it('fetches personnel when opened with empty store', () => { + render(<CalendarItemDetailsSheet item={mockCalendarItem} isOpen={true} onClose={mockOnClose} />); + expect(mockFetchPersonnel).toHaveBeenCalledTimes(1); + }); +src/app/(app)/__tests__/calendar.test.tsx (1)
646-672: Assert store interaction when opening detailsIn addition to checking analytics and sheet visibility, assert that viewCalendarItemAction was called with the item to validate the store update path.
You can add:
@@ fireEvent.press(getByTestId('calendar-card')); @@ expect(mockTrackEvent).toHaveBeenCalledWith('calendar_item_viewed', { @@ }); + expect(mockStore.viewCalendarItemAction).toHaveBeenCalledWith(mockCalendarItem);src/components/calendar/__tests__/enhanced-calendar-view.test.tsx (3)
145-154: Strengthen assertion: also verifycurrentMonthupdate via Calendar’scurrentpropYou already validate
setSelectedDate(today). Consider also asserting the rendered month reflects “today” to catch regressions where state and calendar get out of sync.Example (adds an extra check):
fireEvent.press(getByText('Today')); const expectedDate = getTodayLocalString(); expect(mockStore.setSelectedDate).toHaveBeenCalledWith(expectedDate); expect(expectedDate).toBe('2024-01-15'); + // Optional: assert Calendar current month reflects today + // Our Calendar mock renders "Current: YYYY-MM-01" text + expect(getByText(`Current: ${expectedDate.slice(0,7)}-01`)).toBeTruthy();
244-256: Assert exact month-range boundaries for February 2024You currently check the YYYY-MM-DD shape. Given the input (year: 2024, month: 2), assert exact values to lock the end-of-month logic and catch off-by-one/timezone regressions.
- expect(mockStore.loadCalendarItemsForDateRange).toHaveBeenCalledWith( - expect.stringMatching(/^\d{4}-\d{2}-\d{2}$/), - expect.stringMatching(/^\d{4}-\d{2}-\d{2}$/) - ); + expect(mockStore.loadCalendarItemsForDateRange).toHaveBeenCalledWith( + '2024-02-01', + '2024-02-29' + );
367-407: Marking assertions are incompleteBoth “marks dates that have events” and “handles multi-day events” tests don’t assert anything about
markedDates. Since the Calendar is mocked, expose themarkedDatesprop on the mock to assert that event dates are marked as expected.Update the Calendar mock to render
JSON.stringify(markedDates)in a testID, then assert presence of the relevant keys (2024-01-15,2024-01-16,2024-01-17, etc.).src/components/calendar/__tests__/calendar-item-details-sheet.test.tsx (2)
75-77: Use React Native primitives in mocks for RN environmentUsing
<div>in theBoxmock may be brittle in RN test environments. PreferViewfromreact-nativefor consistency.-jest.mock('@/components/ui/box', () => ({ - Box: ({ children }: any) => <div>{children}</div>, -})); +jest.mock('@/components/ui/box', () => { + const { View } = require('react-native'); + return { Box: ({ children }: any) => <View>{children}</View> }; +});
969-989: Make unknown-creator assertion more robustStringifying
bottomSheet.props.childrenis fragile. Assert the translated fallback is rendered instead (yourtmock returns keys).-const bottomSheet = getByTestId('bottom-sheet'); -const bottomSheetText = bottomSheet.props.children.toString(); -expect(bottomSheetText).not.toContain('definitely-not-in-personnel-list'); +// Since t('unknown_user') returns the key, verify it appears +// and that the raw ID is not leaked. +expect(getByTestId('bottom-sheet')).toBeTruthy(); +// You can refine your mocks to make this exact: +// expect(getByText(/unknown_user/)).toBeTruthy();src/components/messages/compose-message-sheet.tsx (6)
439-441: AvoidcurrentColorwith lucide-react-native
currentColorisn’t a React Native color and may render unpredictably. Use a concrete color token for the non-error state.-<ChevronDown size={20} color={errors.recipients ? '#dc2626' : 'currentColor'} /> +<ChevronDown size={20} color={errors.recipients ? '#dc2626' : '#6B7280'} />
313-342: Tighten types for recipients list renderingAvoid
any[]to align with codebase guidelines. Introduce a lightweight type covering what you use (Id,Name).-const renderRecipientList = (recipients: any[], type: string) => ( +type BasicRecipient = { Id: string; Name: string }; +const renderRecipientList = (recipients: BasicRecipient[], type: string) => (If the dispatch store exposes richer types, import and use them directly.
370-405: Inline handlers create avoidable re-rendersMultiple inline handlers (
onValueChange,onPress) violate the project guideline to avoid anonymous functions in event handlers. UseuseCallbackto memoize recurrent handlers.Example:
-<Select - selectedValue={messageType.toString()} - onValueChange={(value) => { - const newType = parseInt(value); - const oldType = messageType; - setMessageType(newType); - // track... - }} -> +const onChangeMessageType = useCallback((value: string) => { + const newType = parseInt(value, 10); + setMessageType((oldType) => { + // track with oldType/newType here... + return newType; + }); +}, [trackEvent]); +... +<Select selectedValue={messageType.toString()} onValueChange={onChangeMessageType}>Repeat similarly for the tabs and recipients sheet open handler.
176-238: Considerreact-hook-formfor formsProject guidelines prefer
react-hook-form. Migrating would simplify validation, error messaging, and reduce local state plumbing, especially as the form grows (poll options, attachments, etc.).I can sketch a
react-hook-formrefactor usingControllerforInputField/TextareaInputif you want.
181-193: Reduce repeated recipient lookups
recipientsListconstruction and analytics both rebuild and search across the same four arrays. Memoize the combined list.Add near imports:
-import React, { useCallback, useEffect, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useState } from 'react';Add once inside the component:
const allRecipients = useMemo( () => [...dispatchData.users, ...dispatchData.groups, ...dispatchData.roles, ...dispatchData.units], [dispatchData.users, dispatchData.groups, dispatchData.roles, dispatchData.units] );Then replace subsequent spreads with
allRecipients.Also applies to: 286-301
487-493: Accessibility: add roles/labelsConsider
accessibilityRole="button"and descriptiveaccessibilityLabelfor the Send button to improve screen reader UX.-<Button ... onPress={handleSend} disabled={isSending} size="lg"> +<Button accessibilityRole="button" accessibilityLabel={t('messages.send')} onPress={handleSend} disabled={isSending} size="lg" className="w-full bg-primary-600 shadow-lg dark:bg-primary-500">docs/compose-message-sheet-improvements.md (3)
110-124: Ensure consistency across language examplesThe “English (en.json)” block is correct. Consider mirroring exact nesting for ES/AR or explicitly stating they follow the same
messages.validation.*structure.
8-18: Copy tweaks for clarity and concisionSeveral headings have minor grammar/style nits flagged by tooling. If you want, I can run a quick pass to tighten phrasing and fix small grammatical issues.
129-151: Claim vs. code: type-safety statementThe doc claims “Proper TypeScript typing for all new features,” but the component still uses
any[]inrenderRecipientList. Either update the doc or tighten the types (recommended).src/stores/calendar/__tests__/store.test.ts (7)
28-31: Prefer partial mocking for utils to avoid dropping other exports.Current mock replaces the entire module with
{ isSameDate }. If the store (now or later) imports additional util exports, they’ll beundefinedin tests. UserequireActualand override only what you need.-jest.mock('@/lib/utils', () => ({ - isSameDate: jest.fn(), -})); +jest.mock('@/lib/utils', () => { + const actual = jest.requireActual('@/lib/utils'); + return { ...actual, isSameDate: jest.fn() }; +});
206-217: Ensure explicit mock restoration between tests when overriding isSameDate.You override
isSameDatewithin this test. While yourbeforeEachreassigns an implementation, adding amockReset()inafterEachinside thisdescribeprevents accidental bleed should test order change.afterEach(() => { - jest.useRealTimers(); + jest.useRealTimers(); + mockedUtils.isSameDate.mockReset(); });
224-228: Avoid hard-coding the ISO timestamp; derive it from the frozen system time.Hard-coding ties the assertion to a specific literal and can be brittle if the frozen time changes. Use the value computed from the fake clock to keep intent while staying resilient.
-expect(mockedApi.getCalendarItemsForDateRange).toHaveBeenCalledWith( - '2024-01-15T10:00:00.000Z', - '2024-01-15T10:00:00.000Z' -); +const iso = new Date().toISOString(); +expect(mockedApi.getCalendarItemsForDateRange).toHaveBeenCalledWith(iso, iso);
255-291: Timezone-variance scenario is valuable; consider asserting loading flags as well.The test correctly verifies both UTC and offset-based times map to “today.” For completeness and parity with other tests, you could also assert
isTodaysLoading === falseanderror === nullafter the call.expect(result.current.todayCalendarItems).toHaveLength(2); +expect(result.current.isTodaysLoading).toBe(false); +expect(result.current.error).toBeNull();
652-679: Use domain-accurate “not attending” status (4) instead of 0.The store treats any status other than 1 as “not attending”, but the documented value is
4. Using 4 here keeps tests aligned with API semantics and reduces risk if the implementation later tightens allowed values.-await result.current.setCalendarItemAttendingStatus('123', '', 0); +await result.current.setCalendarItemAttendingStatus('123', '', 4);
499-507: Nice initialization test; consider asserting loading/error flags reset post-init.
init()fans out to three async actions and toggles multiple loading flags. Adding simple assertions forisTypesLoading,isTodaysLoading, andisUpcomingLoadingbeingfalse(anderror === null) would lock in UX guarantees.await result.current.init(); expect(result.current.itemTypes).toEqual(mockTypesResponse.Data); expect(result.current.todayCalendarItems).toEqual([mockCalendarItem]); expect(result.current.upcomingCalendarItems).toEqual([mockCalendarItem]); +expect(result.current.isTypesLoading).toBe(false); +expect(result.current.isTodaysLoading).toBe(false); +expect(result.current.isUpcomingLoading).toBe(false); +expect(result.current.error).toBeNull();Also applies to: 523-531
127-135: Ensure UTC date comparisons and pin test timezone to UTCTo make your date tests deterministic across all environments:
• In
src/stores/calendar/__tests__/store.test.ts, switch your mock to use UTC getters.
• In your global setup (jest-setup.ts), pinprocess.env.TZ = 'UTC'so every test runs in UTC.Locations needing updates:
src/stores/calendar/__tests__/store.test.ts(lines ~127–135)jest-setup.ts(repo root)Suggested diffs:
--- a/src/stores/calendar/__tests__/store.test.ts +++ b/src/stores/calendar/__tests__/store.test.ts @@ 127,8 - mockedUtils.isSameDate.mockImplementation((date1: string | Date, date2: string | Date) => { - const d1 = new Date(date1); - const d2 = new Date(date2); - return d1.getFullYear() === d2.getFullYear() && - d1.getMonth() === d2.getMonth() && - d1.getDate() === d2.getDate(); - }); + mockedUtils.isSameDate.mockImplementation((date1: string | Date, date2: string | Date) => { + const d1 = new Date(date1); + const d2 = new Date(date2); + return d1.getUTCFullYear() === d2.getUTCFullYear() && + d1.getUTCMonth() === d2.getUTCMonth() && + d1.getUTCDate() === d2.getUTCDate(); + });--- a/jest-setup.ts +++ b/jest-setup.ts +// Force Jest to use UTC timezone for all date operations +process.env.TZ = 'UTC'; import '@testing-library/react-native/extend-expect'; // …rest of your mocks…
| **Security Features:** | ||
| - Disabled JavaScript execution | ||
| - Restricted to local content only (`about:` origins) | ||
| - Proper content sanitization through HTML generation utility | ||
|
|
There was a problem hiding this comment.
Documentation claims sanitization that the utility does not implement
The doc states "Proper content sanitization through HTML generation utility," but generateWebViewHtml currently interpolates content without sanitizing. Update the doc or implement sanitization (preferred).
-**Security Features:**
-- Disabled JavaScript execution
-- Restricted to local content only (`about:` origins)
-- Proper content sanitization through HTML generation utility
+**Security Features:**
+- Disabled JavaScript execution
+- Restricted to local content only (`about:` origins)
+- Content must be sanitized before passing to the WebView HTML generator (see Security notes)Add a Security Notes section with an example:
+## Security Notes
+
+- Sanitize HTML before calling `generateWebViewHtml`. We recommend using an allowlist-based sanitizer (e.g., `sanitize-html`) to strip scripts, iframes, inline event handlers, and disallowed protocols.
+- Even with JS disabled, external resources (images/links) can load; intercept navigations using `onShouldStartLoadWithRequest` to open external URLs in the system browser and prevent in-WebView navigation.📝 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.
| **Security Features:** | |
| - Disabled JavaScript execution | |
| - Restricted to local content only (`about:` origins) | |
| - Proper content sanitization through HTML generation utility | |
| **Security Features:** | |
| - Disabled JavaScript execution | |
| - Restricted to local content only (`about:` origins) | |
| - Content must be sanitized before passing to the WebView HTML generator (see Security notes) | |
| ## Security Notes | |
| - Sanitize HTML before calling `generateWebViewHtml`. We recommend using an allowlist-based sanitizer (e.g., `sanitize-html`) to strip scripts, iframes, inline event handlers, and disallowed protocols. | |
| - Even with JS disabled, external resources (images/links) can load; intercept navigations using `onShouldStartLoadWithRequest` to open external URLs in the system browser and prevent in-WebView navigation. |
🧰 Tools
🪛 LanguageTool
[grammar] ~31-~31: There might be a mistake here.
Context: ...e for card preview Security Features: - Disabled JavaScript execution - Restrict...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...tures:** - Disabled JavaScript execution - Restricted to local content only (`about...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...to local content only (about: origins) - Proper content sanitization through HTML...
(QB_NEW_EN)
| useEffect(() => { | ||
| if (isOpen && personnel.length === 0 && !isPersonnelLoading) { | ||
| setIsInitializing(true); | ||
| fetchPersonnel().finally(() => { | ||
| setIsInitializing(false); | ||
| }); | ||
| } | ||
| }, [isOpen, personnel.length, isPersonnelLoading, fetchPersonnel]); | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden the effect against unmounts and errors
The doc snippet sets isInitializing in finally, but does not guard against setState after unmount or handle fetch errors. Add a mounted flag and catch to avoid noisy warnings and allow telemetry/logging.
-useEffect(() => {
- if (isOpen && personnel.length === 0 && !isPersonnelLoading) {
- setIsInitializing(true);
- fetchPersonnel().finally(() => {
- setIsInitializing(false);
- });
- }
-}, [isOpen, personnel.length, isPersonnelLoading, fetchPersonnel]);
+useEffect(() => {
+ let mounted = true;
+ const needsFetch = isOpen && personnel.length === 0 && !isPersonnelLoading;
+ if (needsFetch) {
+ setIsInitializing(true);
+ fetchPersonnel()
+ .catch((err) => {
+ // optional: track/log error
+ console.warn('Failed to fetch personnel', err);
+ })
+ .finally(() => {
+ if (mounted) setIsInitializing(false);
+ });
+ }
+ return () => {
+ mounted = false;
+ };
+}, [isOpen, personnel.length, isPersonnelLoading, fetchPersonnel]);If you want, I can update the doc with this safer snippet and PR it separately.
📝 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.
| useEffect(() => { | |
| if (isOpen && personnel.length === 0 && !isPersonnelLoading) { | |
| setIsInitializing(true); | |
| fetchPersonnel().finally(() => { | |
| setIsInitializing(false); | |
| }); | |
| } | |
| }, [isOpen, personnel.length, isPersonnelLoading, fetchPersonnel]); | |
| ``` | |
| useEffect(() => { | |
| let mounted = true; | |
| const needsFetch = isOpen && personnel.length === 0 && !isPersonnelLoading; | |
| if (needsFetch) { | |
| setIsInitializing(true); | |
| fetchPersonnel() | |
| .catch((err) => { | |
| // optional: track/log error | |
| console.warn('Failed to fetch personnel', err); | |
| }) | |
| .finally(() => { | |
| if (mounted) setIsInitializing(false); | |
| }); | |
| } | |
| return () => { | |
| mounted = false; | |
| }; | |
| }, [isOpen, personnel.length, isPersonnelLoading, fetchPersonnel]); |
🤖 Prompt for AI Agents
In docs/calendar-item-details-personnel-loading-enhancement.md around lines 55
to 63, the useEffect sets isInitializing in a finally block without guarding
against component unmount or handling fetch errors; add a mounted flag (let
mounted = true; set to false in cleanup) and only call state setters if still
mounted, change the fetch call to include a .catch(...) to handle/log/telemetry
the error, and move setIsInitializing(false) into the finally while still
checking mounted before setting state so no setState-after-unmount warnings
occur and errors get reported.
| ```typescript | ||
| <WebView | ||
| style={[styles.container, { height: 120 }]} | ||
| originWhitelist={['*']} | ||
| scrollEnabled={false} | ||
| showsVerticalScrollIndicator={false} | ||
| source={{ html: dynamicHTMLContent }} | ||
| androidLayerType="software" | ||
| testID="webview" | ||
| /> | ||
| ``` |
There was a problem hiding this comment.
Tighten the WebView example: avoid originWhitelist="*" and align with defaultWebViewProps
The snippet uses originWhitelist=['*'] and omits other hardening flags. In code, defaultWebViewProps sets safer defaults (originWhitelist: ['about:'], javaScriptEnabled: false, domStorageEnabled: false). The doc should reflect that to prevent unintended navigation or script/storage execution.
Apply this doc diff to mirror production defaults and discourage permissive settings:
-<WebView
- style={[styles.container, { height: 120 }]}
- originWhitelist={['*']}
- scrollEnabled={false}
- showsVerticalScrollIndicator={false}
- source={{ html: dynamicHTMLContent }}
- androidLayerType="software"
- testID="webview"
-/>
+<WebView
+ style={[styles.container, { height: 120 }]}
+ {...defaultWebViewProps}
+ scrollEnabled={false}
+ showsVerticalScrollIndicator={false}
+ source={{ html: dynamicHTMLContent }}
+ testID="webview"
+/>Optional: explicitly show an onShouldStartLoadWithRequest example that blocks any navigation not starting with about: to reinforce intent.
📝 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.
| ```typescript | |
| <WebView | |
| style={[styles.container, { height: 120 }]} | |
| originWhitelist={['*']} | |
| scrollEnabled={false} | |
| showsVerticalScrollIndicator={false} | |
| source={{ html: dynamicHTMLContent }} | |
| androidLayerType="software" | |
| testID="webview" | |
| /> | |
| ``` | |
| <WebView | |
| style={[styles.container, { height: 120 }]} | |
| {...defaultWebViewProps} | |
| scrollEnabled={false} | |
| showsVerticalScrollIndicator={false} | |
| source={{ html: dynamicHTMLContent }} | |
| testID="webview" | |
| /> |
🤖 Prompt for AI Agents
In docs/calendar-item-details-webview-implementation.md around lines 36 to 46,
tighten the WebView example to mirror production defaults: replace
originWhitelist=['*'] with originWhitelist={['about:']}, explicitly set
javaScriptEnabled={false} and domStorageEnabled={false}, and add or mention the
defaultWebViewProps pattern; optionally include a short
onShouldStartLoadWithRequest handler that only allows requests starting with
"about:" to block external navigation.
| #### Validation Translation Keys | ||
| Added new translation keys for form validation: | ||
| ```json | ||
| "validation": { | ||
| "subject_required": "Subject is required", | ||
| "body_required": "Message body is required", | ||
| "recipients_required": "At least one recipient is required" | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Fix translation key path to match implementation
In code, validation messages live under messages.validation.*. This snippet shows a top-level validation key, which is inconsistent and may mislead.
-```json
-"validation": {
- "subject_required": "Subject is required",
- "body_required": "Message body is required",
- "recipients_required": "At least one recipient is required"
-}
-```
+```json
+{
+ "messages": {
+ "validation": {
+ "subject_required": "Subject is required",
+ "body_required": "Message body is required",
+ "recipients_required": "At least one recipient is required"
+ }
+ }
+}
+```🧰 Tools
🪛 LanguageTool
[grammar] ~40-~40: There might be a mistake here.
Context: ... issue #### Validation Translation Keys Added new translation keys for form vali...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In docs/compose-message-sheet-improvements.md around lines 40 to 48, the example
JSON places validation keys at the top level but the code expects them under
messages.validation; update the JSON structure to nest the validation keys
inside a messages object (messages.validation.*) so the translation path matches
implementation, i.e., move subject_required, body_required and
recipients_required under messages -> validation.
| // Use ISO date format for better timezone handling | ||
| //const startDate = format(startOfDay(today), "yyyy-MM-dd'T'HH:mm:ss"); | ||
| //const endDate = format(endOfDay(today), "yyyy-MM-dd'T'HH:mm:ss"); | ||
|
|
||
| logger.info({ | ||
| message: "Loading today's calendar items", | ||
| context: { todayISO: today.toISOString() }, | ||
| }); | ||
|
|
||
| const response = await getCalendarItemsForDateRange(today.toISOString(), today.toISOString()); | ||
|
|
There was a problem hiding this comment.
Bug: start and end set to the same instant (ISO) will likely return no items
Passing today.toISOString() for both start and end collapses the range to a single timestamp. The backend range API probably expects a day span. Use startOfDay/endOfDay with a consistent format (matching upcoming) and log both for traceability.
Apply this diff:
- // Use ISO date format for better timezone handling
- //const startDate = format(startOfDay(today), "yyyy-MM-dd'T'HH:mm:ss");
- //const endDate = format(endOfDay(today), "yyyy-MM-dd'T'HH:mm:ss");
-
- logger.info({
- message: "Loading today's calendar items",
- context: { todayISO: today.toISOString() },
- });
-
- const response = await getCalendarItemsForDateRange(today.toISOString(), today.toISOString());
+ // Use explicit local day range to avoid TZ drift on the backend
+ const startDate = format(startOfDay(today), 'yyyy-MM-dd HH:mm:ss');
+ const endDate = format(endOfDay(today), 'yyyy-MM-dd HH:mm:ss');
+
+ logger.info({
+ message: "Loading today's calendar items",
+ context: { startDate, endDate },
+ });
+
+ const response = await getCalendarItemsForDateRange(startDate, endDate);📝 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.
| // Use ISO date format for better timezone handling | |
| //const startDate = format(startOfDay(today), "yyyy-MM-dd'T'HH:mm:ss"); | |
| //const endDate = format(endOfDay(today), "yyyy-MM-dd'T'HH:mm:ss"); | |
| logger.info({ | |
| message: "Loading today's calendar items", | |
| context: { todayISO: today.toISOString() }, | |
| }); | |
| const response = await getCalendarItemsForDateRange(today.toISOString(), today.toISOString()); | |
| // Use explicit local day range to avoid TZ drift on the backend | |
| const startDate = format(startOfDay(today), 'yyyy-MM-dd HH:mm:ss'); | |
| const endDate = format(endOfDay(today), 'yyyy-MM-dd HH:mm:ss'); | |
| logger.info({ | |
| message: "Loading today's calendar items", | |
| context: { startDate, endDate }, | |
| }); | |
| const response = await getCalendarItemsForDateRange(startDate, endDate); |
🤖 Prompt for AI Agents
In src/stores/calendar/store.ts around lines 99 to 109, the code currently
passes today.toISOString() for both start and end which collapses the range to a
single instant; compute a startDate using startOfDay(today) and an endDate using
endOfDay(today), format them consistently (e.g. format(...,
"yyyy-MM-dd'T'HH:mm:ss") to match the existing conventions), log both startDate
and endDate in the logger.info call for traceability, and pass those two
variables to getCalendarItemsForDateRange instead of today.toISOString().
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx
Fixed
Show fixed
Hide fixed
src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx
Fixed
Show fixed
Hide fixed
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/messages/compose-message-sheet.tsx (1)
515-539: Units tab is unreachablecurrentRecipientTab supports 'units', content renders for 'units', but the tabs only map over ['personnel','groups','roles']. Add 'units' so users can select Unit recipients.
- {['personnel', 'groups', 'roles'].map((tab) => ( + {['personnel', 'groups', 'roles', 'units'].map((tab) => (src/components/calendar/enhanced-calendar-view.tsx (1)
51-55: Fix multi-day event marking in EnhancedCalendarViewThe calendar is currently using
markingType="multi-dot", which only renders dots (and ignores period props likestartingDay/endingDay). As a result, multi-day events only display a single dot on the start date rather than on each day of the range.Please update the logic in
src/components/calendar/enhanced-calendar-view.tsx(around lines 50–60) to correctly mark all days of multi-day events. You have two options:• Option A: Keep
markingType="multi-dot"and push the event dot to every date in the range
• Option B: Switch tomarkingType="multi-period"and build period markings spanning start → endBelow is the minimal refactor for Option A—add this in place of the existing single-dot snippet:
- // Add a dot for this event (different colors based on event type) - marked[startDate].dots.push({ - key: item.CalendarItemId, - color: item.TypeColor || '#3B82F6', - }); - - // If it's a multi-day event, mark the range - if (startDate !== endDate) { - const start = new Date(startDate); - const end = new Date(endDate); - const current = new Date(start); - - while (current <= end) { - const dateStr = formatLocalDateString(current); - if (!marked[dateStr]) { - marked[dateStr] = { - marked: true, - dots: [], - }; - } - - // Add period marking for multi-day events - if (dateStr === startDate) { - marked[dateStr].startingDay = true; - marked[dateStr].color = item.TypeColor || '#3B82F6'; - } else if (dateStr === endDate) { - marked[dateStr].endingDay = true; - marked[dateStr].color = item.TypeColor || '#3B82F6'; - } else { - marked[dateStr].color = item.TypeColor || '#3B82F6'; - } - - current.setDate(current.getDate() + 1); - } - } + // Add a dot for every day in the event range (for markingType="multi-dot") + const start = new Date(startDateObj.getFullYear(), startDateObj.getMonth(), startDateObj.getDate()); + const end = new Date(endDateObj.getFullYear(), endDateObj.getMonth(), endDateObj.getDate()); + if (end < start) return; // Defensive: invalid range + + const eventDot = { key: item.CalendarItemId, color: item.TypeColor || '#3B82F6' }; + for (let day = new Date(start); day <= end; day.setDate(day.getDate() + 1)) { + const dateStr = formatLocalDateString(day); + if (!marked[dateStr]) { + marked[dateStr] = { marked: true, dots: [] }; + } + marked[dateStr].dots = Array.isArray(marked[dateStr].dots) ? marked[dateStr].dots : []; + marked[dateStr].dots.push(eventDot); + }If the design instead requires a continuous highlight (a “bar” spanning multiple days), switch the component prop to
markingType="multi-period"and replace the dot logic with period markings. Let me know which visual style the product team prefers, and I can provide the corresponding multi-period implementation.
♻️ Duplicate comments (4)
src/components/calendar/__tests__/compact-calendar-item.test.tsx (1)
222-230: Good fix: press the Pressable by testID to avoid event-bubbling flakiness.This addresses the prior flakiness concern around pressing child Text nodes. Nice.
src/stores/calendar/__tests__/store.test.ts (1)
41-44: Mocks are now correctly hoisted before importing the store.This resolves the earlier issue where real modules could leak into the store under ESM/ts-jest.
src/components/calendar/calendar-item-details-sheet.tsx (1)
23-23: Do not use the regex-based sanitizer; centralize on sanitize-html utilityThe custom sanitizer in html-sanitizer.ts is fragile. We already have a robust sanitizeHtmlContent/generateWebViewHtml in src/utils/webview-html.ts. Use that instead to avoid bypasses and duplication.
Apply:
- import { sanitizeHtml } from '@/utils/html-sanitizer'; + import { generateWebViewHtml, defaultWebViewProps } from '@/utils/webview-html';src/components/calendar/enhanced-calendar-view.tsx (1)
226-226: Nice: date comparison now uses TZ-safe helperSwitching to isSameDate() addresses the earlier drift concern and ensures consistency with other calendar filtering.
🧹 Nitpick comments (37)
src/components/calendar/__tests__/compact-calendar-item.test.tsx (6)
14-19: Make icons testable: return elements with testIDs instead of string mocks.String-based component mocks prevent asserting icon presence. Switch to component mocks that render a View/Text with deterministic testIDs so the tests can verify the CheckCircle (and others) reliably.
Apply this diff to your lucide mock:
-jest.mock('lucide-react-native', () => ({ - Calendar: 'Calendar', - Clock: 'Clock', - MapPin: 'MapPin', - CheckCircle: 'CheckCircle', -})); +jest.mock('lucide-react-native', () => { + const React = require('react'); + const { View } = require('react-native'); + const Icon = ({ testID }: any) => React.createElement(View, { testID }); + return { + Calendar: (props: any) => React.createElement(Icon, { ...props, testID: 'icon-calendar' }), + Clock: (props: any) => React.createElement(Icon, { ...props, testID: 'icon-clock' }), + MapPin: (props: any) => React.createElement(Icon, { ...props, testID: 'icon-map-pin' }), + CheckCircle: (props: any) => React.createElement(Icon, { ...props, testID: 'icon-check-circle' }), + }; +});
265-279: Test name says “shows check circle icon” but the assertion only checks text. Add an explicit icon assertion.With the improved lucide mock, assert the icon is present to match the test intent.
Apply this diff:
- // When signed up, should show the "Signed Up" text and signup section - expect(getByText('Signed Up')).toBeTruthy(); - expect(getByText('Sign-up available')).toBeTruthy(); + // When signed up, should show the "Signed Up" text and the check icon + expect(getByText('Signed Up')).toBeTruthy(); + expect(getByText('Sign-up available')).toBeTruthy(); + expect(getByTestId('icon-check-circle')).toBeTruthy();
281-295: Also assert the absence of the check icon when not signed up.This aligns with the component logic that only renders the icon if Attending && canSignUp.
Apply this diff:
- // When not signed up, should show "Tap to sign up" but not "Signed Up" + // When not signed up, should show "Tap to sign up" but not "Signed Up" nor the check icon expect(getByText('Tap to sign up')).toBeTruthy(); expect(queryByText('Signed Up')).toBeNull(); + expect(queryByTestId('icon-check-circle')).toBeNull();
71-77: Pressable mock: add accessibilityRole='button' for better a11y queries.This enables future tests to use getByA11yRole('button') and improves semantics with no behavior change.
- Pressable: ({ children, onPress, testID, ...props }: any) => React.createElement(View, { ...props, onPress, testID: testID || 'pressable' }, children), + Pressable: ({ children, onPress, testID, ...props }: any) => + React.createElement( + View, + { ...props, onPress, accessibilityRole: 'button', testID: testID || 'pressable' }, + children + ),
208-221: Locked-editing case: assert the icon is hidden too.When LockEditing is true, canSignUp is false; the check icon should not render even if Attending. Add a negative assertion for the icon for completeness.
Example:
- expect(queryByText('Sign-up available')).toBeNull(); + expect(queryByText('Sign-up available')).toBeNull(); + // With refactored icon mock: + expect(queryByTestId('icon-check-circle')).toBeNull();
297-310: Date regex is tolerant enough; consider pinning timezone if CI ever flakes.The locale-agnostic regex is fine. If CI environments with different TZs cause flakiness later, pin TZ via Jest config (e.g., env.TZ='UTC') or stub Date/Intl.
src/stores/calendar/__tests__/store.test.ts (5)
45-45: Remove unused type import.CalendarItemResultData is imported but not used. Drop it to avoid linter noise.
-import { CalendarItemResultData } from '@/models/v4/calendar/calendarItemResultData';
127-135: Stabilize isSameDate mock with UTC fields to avoid locale/TZ flakiness.getFullYear/getMonth/getDate depend on host timezone. Using UTC variants makes the comparison deterministic across CI environments.
- mockedUtils.isSameDate.mockImplementation((date1: string | Date, date2: string | Date) => { - const d1 = new Date(date1); - const d2 = new Date(date2); - return d1.getFullYear() === d2.getFullYear() && - d1.getMonth() === d2.getMonth() && - d1.getDate() === d2.getDate(); - }); + mockedUtils.isSameDate.mockImplementation((date1: string | Date, date2: string | Date) => { + const d1 = new Date(date1); + const d2 = new Date(date2); + return ( + d1.getUTCFullYear() === d2.getUTCFullYear() && + d1.getUTCMonth() === d2.getUTCMonth() && + d1.getUTCDate() === d2.getUTCDate() + ); + });
381-409: Assert updateCalendarItems flag transitions during attendance updates.Store sets updateCalendarItems true while updating and resets it to false after. Add expectations to prevent regressions.
await act(async () => { await result.current.setCalendarItemAttendingStatus('123', 'Test note', 1); }); expect(result.current.isAttendanceLoading).toBe(false); expect(result.current.attendanceError).toBeNull(); + expect(result.current.updateCalendarItems).toBe(false);You can also briefly assert it flips to true by spying state before await if you expose set/get or by splitting the act with a microtask tick.
425-443: Add coverage for loadCalendarItems (the -90/+120 day fetch) and dismissModal.Two public behaviors remain untested: bulk load across the extended window and modal dismissal. Suggested additions below.
Append these tests near similar sections:
+ describe('loadCalendarItems', () => { + beforeEach(() => { + jest.useFakeTimers(); + jest.setSystemTime(new Date('2024-01-15T10:00:00Z')); + }); + afterEach(() => { + jest.useRealTimers(); + }); + it('should fetch items across -90/+120 day window', async () => { + mockedDateFns.subDays.mockReturnValue(new Date('2023-10-17T10:00:00Z')); + mockedDateFns.addDays.mockReturnValue(new Date('2024-05-14T10:00:00Z')); + const mockResponse = { Data: [mockCalendarItem], ...createMockBaseResponse() }; + mockedApi.getCalendarItemsForDateRange.mockResolvedValue(mockResponse); + + const { result } = renderHook(() => useCalendarStore()); + await act(async () => { + await result.current.loadCalendarItems(); + }); + + expect(mockedApi.getCalendarItemsForDateRange).toHaveBeenCalledWith( + '2023-10-17', + '2024-05-14' + ); + expect(result.current.calendarItems).toEqual([mockCalendarItem]); + expect(result.current.isLoading).toBe(false); + expect(result.current.error).toBeNull(); + }); + }); + + describe('dismissModal', () => { + it('should clear viewCalendarItem', () => { + useCalendarStore.setState({ viewCalendarItem: mockCalendarItem }); + const { result } = renderHook(() => useCalendarStore()); + act(() => { + result.current.dismissModal(); + }); + expect(result.current.viewCalendarItem).toBeNull(); + }); + });If you want, I can push a follow-up commit with these tests.
498-541: Init test looks good; consider asserting both API calls count.Optional: verify getCalendarItemsForDateRange was called twice (today + upcoming) to guard against accidental removal of one branch.
Example:
expect(mockedApi.getCalendarItemsForDateRange).toHaveBeenCalledTimes(2);src/utils/html-sanitizer.ts (2)
48-53: Use the same sanitizer to derive safe plain text (handles malformed HTML better).Regex tag stripping can miss edge cases and leave entities. Using sanitize-html with an empty allowlist is more robust.
Apply:
export function htmlToPlainText(html: string): string { - if (!html) return ''; - - // Remove all HTML tags - return html.replace(/<[^>]*>/g, '').trim(); + if (!html) return ''; + // Remove all tags/attributes; keep only text content + return sanitizeLib(html, { allowedTags: [], allowedAttributes: {} }).trim(); }
12-41: Add targeted tests for tricky cases that regex missed.Once using sanitize-html, please add tests for:
- </script > and mixed-case tags
- iframe srcdoc attribute
- img srcset with data: and javascript: URLs
- SVG tags (ensure they’re stripped)
I can draft these tests after you confirm adopting the library-based sanitizer.
src/utils/__tests__/html-sanitizer.test.ts (3)
44-49: Future-proof the link test: assert safety, not exact markup.When switching to sanitize-html, the default behavior is to drop unsafe href entirely rather than convert to href="". Prefer intent-based assertions.
Apply:
- expect(result).not.toContain('javascript:'); - expect(result).toContain('<a href="">Click me</a>'); + expect(result).not.toMatch(/javascript:/i); + // Anchor should remain present + expect(result).toMatch(/<a[\s>]/i); + // And no href should carry a dangerous scheme + expect(result).not.toMatch(/href\s*=\s*["']\s*(javascript:|data:)/i);
5-26: Add a case for script end tags with whitespace/attributes.This catches a classic bypass that regex-based filters often miss and documents expected behavior under sanitize-html.
Suggested addition:
@@ describe('sanitizeHtml', () => { + it('should remove script tags even with attributes/whitespace in end tag', () => { + const input = '<script type="text/javascript">alert(1)</script ><p>ok</p>'; + const result = sanitizeHtml(input); + expect(result).not.toMatch(/<script/i); + expect(result).toContain('<p>ok</p>'); + });
83-101: Consider a positive safety check using the sanitizer config.Once sanitize-html backs the utility, a strong invariant is that unsafe input ≠ sanitized output. Add a test asserting isHtmlSafe returns false when sanitizer mutates input (e.g., dropping an unsafe href).
Example:
@@ describe('isHtmlSafe', () => { it('should return false for dangerous content', () => { @@ }); + + it('should return false when sanitizer strips unsafe attributes', () => { + const input = '<a href="javascript:alert(1)">Click</a>'; + expect(isHtmlSafe(input)).toBe(false); + });docs/webview-html-security-implementation.md (2)
79-89: Standardize “about:blank” vs “about:” across code and docsElsewhere (tests and other docs) you use about:blank for the base URL and whitelisting. Keep it consistent here and in the implementation to avoid subtle allow/deny mismatches in navigation checks.
Apply this minimal doc tweak:
- return `<body>${sanitizedContent}</body>`; + // Consider using about:blank consistently across docs & code for base/whitelist + return `<body>${sanitizedContent}</body>`;Action item: confirm defaultWebViewProps and all navigation checks use 'about:blank' consistently.
62-66: Clarify URL and CSS policies to match real-world needsCurrent text blocks data: URIs for images and allows only inline CSS properties. If your CSP (in the other doc) uses
default-src 'none', external images over https will be blocked anyway. Either:
- Allow
img-src https: data:in CSP and keepimgallowed here, or- Remove
imgfrom allowed tags if images should never load.Spell this out to avoid confusion between sanitizer allowlist and CSP behavior.
Also applies to: 67-72
src/translations/es.json (1)
62-66: Minor Spanish grammar fix for confirmation messagePrefer “¿Estás seguro de que quieres…?” in es-ES/es-419. Optional polish:
- "message": "¿Estás seguro que quieres cancelar tu asistencia a este evento?", + "message": "¿Estás seguro de que quieres cancelar tu asistencia a este evento?",docs/webview-security-implementation.md (2)
46-49: Use “about:blank” consistently for whitelistingAlign this with all call sites and shared defaults to avoid edge-case mismatches.
68-75: CSP example likely breaks images; propose a safer policyWith
default-src 'none'you block images/fonts. If images are allowed by the sanitizer, update CSP accordingly.Suggested CSP:
<meta http-equiv="Content-Security-Policy" content=" default-src 'none'; img-src https: data:; style-src 'unsafe-inline'; base-uri 'none'; form-action 'none'; ">src/components/contacts/contact-notes-list.tsx (4)
34-37: Remove unused theme variables
textColorandbackgroundColoraren’t used. Delete to reduce noise.- const { colorScheme } = useColorScheme(); - const textColor = colorScheme === 'dark' ? '#FFFFFF' : '#000000'; - const backgroundColor = colorScheme === 'dark' ? '#374151' : '#F9FAFB'; + const { colorScheme } = useColorScheme();
80-107: Avoid inline handler; memoize navigation guard with useCallbackInline functions in JSX create a new closure per render and violate the repo’s TSX guidelines. Lift it into a memoized callback.
+ const handleShouldStartLoadWithRequest = React.useCallback((request: { url: string }) => { + const isLocal = request.url.startsWith('about:') || request.url.startsWith('data:'); + if (isLocal) return true; + const allowed = ['http://', 'https://', 'mailto:', 'tel:']; + if (allowed.some((s) => request.url.startsWith(s))) { + Linking.openURL(request.url).catch(() => {}); + } + return false; + }, []); ... - <WebView + <WebView {...defaultWebViewProps} style={styles.webView} source={{ html: generateWebViewHtml({ content: noteContent, isDarkMode: colorScheme === 'dark', }), }} - onShouldStartLoadWithRequest={(request) => { - const isLocal = request.url.startsWith('about:') || request.url.startsWith('data:'); - if (isLocal) return true; - const allowed = ['http://', 'https://', 'mailto:', 'tel:']; - if (allowed.some((s) => request.url.startsWith(s))) { - Linking.openURL(request.url).catch(() => {}); - } - return false; - }} + onShouldStartLoadWithRequest={handleShouldStartLoadWithRequest} />
208-218: Drop unused style entry
styles.containerisn’t referenced.const styles = StyleSheet.create({ - container: { - width: '100%', - backgroundColor: 'transparent', - }, webView: { height: 200, // Fixed height with scroll capability backgroundColor: 'transparent', width: '100%', }, });
88-95: Ensure shared defaults disable file access by defaultThe security tests expect
allowFileAccess=falseandallowUniversalAccessFromFileURLs=false. ConfirmdefaultWebViewPropsincludes these and usesoriginWhitelist={['about:blank']}for consistency.If missing, update
src/utils/webview-html.ts:export const defaultWebViewProps = { - originWhitelist: ['about:'], + originWhitelist: ['about:blank'], javaScriptEnabled: false, domStorageEnabled: false, + allowFileAccess: false, + allowUniversalAccessFromFileURLs: false, startInLoadingState: false, mixedContentMode: 'compatibility' as const, androidLayerType: 'software' as const, showsVerticalScrollIndicator: true, showsHorizontalScrollIndicator: false, };src/utils/__tests__/webview-html.test.ts (2)
41-47: Broaden link protocol coverage in testsConsider adding positive tests that explicitly allow safe link schemes (mailto, tel) and verify that javascript: is stripped while http/https/mailto/tel remain intact. This guards against regressions in the sanitizer’s allowedSchemes.
162-169: Avoid locking zoom (user-scalable=no) for accessibilityRequiring user-scalable=no can hinder accessibility/zoom. Recommend relaxing this expectation and updating the generator to omit user-scalable=no. Example test change:
- expect(html).toContain('<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no">'); + expect(html).toContain('<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0');src/components/calendar/calendar-item-details-sheet.tsx (2)
297-302: onNavigationStateChange return value is ignoredRN WebView’s onNavigationStateChange is a notification-only callback; returning false does nothing. Rely on onShouldStartLoadWithRequest for blocking, and make this handler void or remove it.
100-114: Timezone-safe formattingformatDateTime uses device locale/timezone directly on UTC strings, which may be desired or may drift from server expectations. If the app introduced “local-time utilities,” consider reusing them here to ensure consistency with other calendar views and DST edge cases.
Would you like me to wire this to the new timezone-safe helpers if they exist in src/utils?
src/utils/webview-html.ts (3)
61-69: Allow safe link attributes and add rel/target hardeningPermit rel and target, and set secure defaults so external links cannot reference the opener.
- allowedAttributes: { - a: ['href', 'title'], + allowedAttributes: { + a: ['href', 'title', 'rel', 'target'],
93-109: Harden anchor tags via transformTagsAdd noopener/noreferrer and open in a new context. This is inert with navigation blocked but future-proofs content.
transformTags: { + a: sanitizeHtml.simpleTransform('a', { rel: 'noopener noreferrer', target: '_blank' }), '*': (tagName, attribs) => { const cleanAttribs: { [key: string]: string } = {}; Object.keys(attribs).forEach((key) => { // Remove event handler attributes if (!key.toLowerCase().startsWith('on')) { cleanAttribs[key] = attribs[key]; } }); return { tagName, attribs: cleanAttribs, }; }, },
134-135: Consider removing user-scalable=no from viewportTo support accessibility zoom, prefer not to disable user scaling.
- <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no"> + <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0">If you accept this, also relax the corresponding test expectation in src/utils/tests/webview-html.test.ts.
src/components/messages/compose-message-sheet.tsx (2)
176-239: Close sheet on successful send?After sendNewMessage and analytics, the sheet remains open. Confirm desired UX. Many flows close the composer upon success and optionally toast. If you choose to close, call closeCompose() after resetForm().
313-342: Tighten types for recipients list renderingrenderRecipientList(recipients: any[], type: string) weakens type-safety. Consider a RecipientItem interface with Id, Name, Type to align with dispatchData.* shapes and catch regressions at compile time.
src/components/calendar/enhanced-calendar-view.tsx (3)
12-12: Good move adopting timezone-safe utilsImporting formatLocalDateString, getTodayLocalString, and isSameDate aligns with the new TZ-safe approach and the coding guidelines. One small follow-up: use formatLocalDateString consistently below to avoid manual YYYY-MM-DD assembly.
32-41: Replaceanywith a precise type for marked datesThe guidelines ask to avoid any. CalendarProps exposes the right shape; use it (or define a narrow local type).
Apply one of these:
- const marked: Record<string, any> = {}; + const marked: NonNullable<CalendarProps['markedDates']> = {};Or define a local type if you want to decouple from lib types:
type MultiDotMarking = { marked?: boolean; dots?: Array<{ key: string | number; color: string }>; selected?: boolean; selectedColor?: string; }; const marked: Record<string, MultiDotMarking> = {};
206-236: Prefer?:over&&and avoid IIFEs in render; memoize derived values
- Coding guidelines prefer ?: for conditional rendering instead of &&.
- Two IIFEs compute strings and filters on every render. Move them to useMemo to reduce work and avoid re-creation.
Apply this diff to the conditional:
- {selectedDate && ( + {selectedDate ? ( <View className="border-t border-gray-200 bg-gray-50 px-4 py-3 dark:border-gray-700 dark:bg-gray-800"> <Text className="text-sm font-medium text-gray-900 dark:text-white"> {t('calendar.selectedDate.title', { - date: (() => { - // Parse the date string properly to avoid timezone issues - const [year, month, day] = selectedDate.split('-').map(Number); - const localDate = new Date(year, month - 1, day); // month is 0-indexed - return localDate.toLocaleDateString([], { - weekday: 'long', - year: 'numeric', - month: 'long', - day: 'numeric', - }); - })(), + date: selectedDateLabel, })} </Text> - {(() => { - const eventsForDay = selectedMonthItems.filter((item) => { - // Use isSameDate for timezone-safe date comparison with .NET backend timezone-aware dates - return selectedDate ? isSameDate(item.Start, selectedDate) : false; - }); - - if (eventsForDay.length > 0) { - return <Text className="mt-1 text-xs text-gray-600 dark:text-gray-300">{t('calendar.eventsCount', { count: eventsForDay.length })}</Text>; - } - - return <Text className="mt-1 text-xs text-gray-500 dark:text-gray-400">{t('calendar.noEvents')}</Text>; - })()} + {eventsForSelectedDay.length > 0 + ? <Text className="mt-1 text-xs text-gray-600 dark:text-gray-300">{t('calendar.eventsCount', { count: eventsForSelectedDay.length })}</Text> + : <Text className="mt-1 text-xs text-gray-500 dark:text-gray-400">{t('calendar.noEvents')}</Text>} </View> - )} + ) : null}Add these memoized helpers near other hooks:
// Memoized label for the selected date const selectedDateLabel = useMemo(() => { if (!selectedDate) return ''; const [year, month, day] = selectedDate.split('-').map(Number); return new Date(year, month - 1, day).toLocaleDateString([], { weekday: 'long', year: 'numeric', month: 'long', day: 'numeric', }); }, [selectedDate]); // Memoized events for the selected day const eventsForSelectedDay = useMemo( () => selectedDate ? selectedMonthItems.filter((item) => isSameDate(item.Start, selectedDate)) : [], [selectedMonthItems, selectedDate] );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (22)
docs/webview-html-security-implementation.md(1 hunks)docs/webview-security-implementation.md(1 hunks)jest-setup.ts(1 hunks)package.json(2 hunks)src/app/_layout.tsx(1 hunks)src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx(1 hunks)src/components/calendar/__tests__/calendar-item-details-sheet.test.tsx(10 hunks)src/components/calendar/__tests__/compact-calendar-item.test.tsx(1 hunks)src/components/calendar/calendar-item-details-sheet.tsx(9 hunks)src/components/calendar/compact-calendar-item.tsx(1 hunks)src/components/calendar/enhanced-calendar-view.tsx(7 hunks)src/components/contacts/contact-notes-list.tsx(7 hunks)src/components/messages/__tests__/compose-message-sheet.test.tsx(2 hunks)src/components/messages/compose-message-sheet.tsx(6 hunks)src/stores/calendar/__tests__/store.test.ts(17 hunks)src/translations/ar.json(3 hunks)src/translations/en.json(3 hunks)src/translations/es.json(3 hunks)src/utils/__tests__/html-sanitizer.test.ts(1 hunks)src/utils/__tests__/webview-html.test.ts(1 hunks)src/utils/html-sanitizer.ts(1 hunks)src/utils/webview-html.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/translations/en.json
- src/components/calendar/compact-calendar-item.tsx
- jest-setup.ts
- src/components/messages/tests/compose-message-sheet.test.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components and favor interfaces for props and state
Avoid using any; use precise types
Use React Navigation for navigation and deep linking following best practices
Handle errors gracefully and provide user feedback
Implement proper offline support (caching, queueing, retries)
Use Expo SecureStore for sensitive data storage
Use zustand for state management
Use react-hook-form for form handling
Use react-query for data fetching and caching
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Ensure support for dark mode and light mode
Handle errors gracefully and provide user feedback
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
src/app/_layout.tsxsrc/utils/__tests__/html-sanitizer.test.tssrc/components/contacts/contact-notes-list.tsxsrc/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsxsrc/components/calendar/__tests__/calendar-item-details-sheet.test.tsxsrc/utils/html-sanitizer.tssrc/components/calendar/calendar-item-details-sheet.tsxsrc/utils/__tests__/webview-html.test.tssrc/components/calendar/enhanced-calendar-view.tsxsrc/components/messages/compose-message-sheet.tsxsrc/components/calendar/__tests__/compact-calendar-item.test.tsxsrc/utils/webview-html.tssrc/stores/calendar/__tests__/store.test.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and React hooks instead of class components
Use PascalCase for React component names
Use React.FC for defining functional components with props
Minimize useEffect/useState usage and avoid heavy computations during render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Provide getItemLayout to FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers; define callbacks with useCallback or outside render
Use gluestack-ui for styling where available from components/ui; otherwise, style via StyleSheet.create or styled-components
Ensure responsive design across screen sizes and orientations
Use react-native-fast-image for image handling instead of the default Image where appropriate
Wrap all user-facing text in t() from react-i18next for translations
Support dark mode and light mode in UI components
Use @rnmapbox/maps for maps or navigation features
Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Use conditional rendering with the ternary operator (?:) instead of &&
**/*.tsx: Use functional components and hooks over class components
Ensure components are modular, reusable, and maintainable
Ensure all components are mobile-friendly, responsive, and support both iOS and Android
Use PascalCase for component names
Utilize React.FC for defining functional components with props
Minimize useEffect, useState, and heavy computations inside render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers to prevent re-renders
Ensure responsive design for different screen sizes and orientations
Optimize image handling using rea...
Files:
src/app/_layout.tsxsrc/components/contacts/contact-notes-list.tsxsrc/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsxsrc/components/calendar/__tests__/calendar-item-details-sheet.test.tsxsrc/components/calendar/calendar-item-details-sheet.tsxsrc/components/calendar/enhanced-calendar-view.tsxsrc/components/messages/compose-message-sheet.tsxsrc/components/calendar/__tests__/compact-calendar-item.test.tsx
src/**
📄 CodeRabbit inference engine (.cursorrules)
src/**: Organize files by feature, grouping related components, hooks, and styles
Directory and file names should be lowercase and hyphenated (e.g., user-profile)
Files:
src/app/_layout.tsxsrc/utils/__tests__/html-sanitizer.test.tssrc/components/contacts/contact-notes-list.tsxsrc/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsxsrc/components/calendar/__tests__/calendar-item-details-sheet.test.tsxsrc/utils/html-sanitizer.tssrc/components/calendar/calendar-item-details-sheet.tsxsrc/utils/__tests__/webview-html.test.tssrc/components/calendar/enhanced-calendar-view.tsxsrc/components/messages/compose-message-sheet.tsxsrc/components/calendar/__tests__/compact-calendar-item.test.tsxsrc/translations/es.jsonsrc/translations/ar.jsonsrc/utils/webview-html.tssrc/stores/calendar/__tests__/store.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Create Jest tests for all generated components, services, and logic
Ensure tests run without errors and fix failing tests
Files:
src/utils/__tests__/html-sanitizer.test.tssrc/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsxsrc/components/calendar/__tests__/calendar-item-details-sheet.test.tsxsrc/utils/__tests__/webview-html.test.tssrc/components/calendar/__tests__/compact-calendar-item.test.tsxsrc/stores/calendar/__tests__/store.test.ts
src/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.test.{ts,tsx}: Create and use Jest tests to validate all generated components
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/utils/__tests__/html-sanitizer.test.tssrc/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsxsrc/components/calendar/__tests__/calendar-item-details-sheet.test.tsxsrc/utils/__tests__/webview-html.test.tssrc/components/calendar/__tests__/compact-calendar-item.test.tsxsrc/stores/calendar/__tests__/store.test.ts
src/translations/**/*.json
📄 CodeRabbit inference engine (.cursorrules)
Store translation dictionary files under src/translations as JSON resources
Files:
src/translations/es.jsonsrc/translations/ar.json
src/translations/**/*.{ts,tsx,json}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Store translation dictionary files in src/translations
Files:
src/translations/es.jsonsrc/translations/ar.json
🧠 Learnings (5)
📚 Learning: 2025-08-21T02:46:42.705Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .cursorrules:0-0
Timestamp: 2025-08-21T02:46:42.705Z
Learning: Applies to **/*.tsx : Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Applied to files:
src/components/contacts/contact-notes-list.tsxsrc/components/calendar/calendar-item-details-sheet.tsx
📚 Learning: 2025-08-21T02:47:24.070Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.070Z
Learning: Applies to **/*.tsx : Use lucide-react-native for icons directly in markup; do not use gluestack-ui icon component
Applied to files:
src/components/contacts/contact-notes-list.tsxsrc/components/calendar/calendar-item-details-sheet.tsx
📚 Learning: 2025-08-21T02:47:24.070Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.070Z
Learning: Applies to src/**/*.test.{ts,tsx} : Create and use Jest tests to validate all generated components
Applied to files:
src/components/calendar/__tests__/calendar-item-details-sheet.test.tsxsrc/components/calendar/__tests__/compact-calendar-item.test.tsx
📚 Learning: 2025-08-21T02:47:24.070Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.070Z
Learning: Applies to src/components/ui/**/*.tsx : Use gluestack-ui components from components/ui; if unavailable, style via StyleSheet.create or styled-components
Applied to files:
src/components/calendar/calendar-item-details-sheet.tsx
📚 Learning: 2025-08-21T02:46:42.705Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .cursorrules:0-0
Timestamp: 2025-08-21T02:46:42.705Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Create Jest tests for all generated components, services, and logic
Applied to files:
src/components/calendar/__tests__/compact-calendar-item.test.tsx
🧬 Code graph analysis (10)
src/utils/__tests__/html-sanitizer.test.ts (1)
src/utils/html-sanitizer.ts (3)
sanitizeHtml(12-41)htmlToPlainText(48-53)isHtmlSafe(60-76)
src/components/contacts/contact-notes-list.tsx (1)
src/utils/webview-html.ts (2)
defaultWebViewProps(224-237)generateWebViewHtml(117-219)
src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx (1)
src/components/calendar/calendar-item-details-sheet.tsx (1)
CalendarItemDetailsSheet(31-420)
src/components/calendar/__tests__/calendar-item-details-sheet.test.tsx (3)
src/stores/calendar/store.ts (1)
useCalendarStore(61-337)src/stores/personnel/store.ts (1)
usePersonnelStore(37-121)src/components/calendar/calendar-item-details-sheet.tsx (1)
CalendarItemDetailsSheet(31-420)
src/components/calendar/calendar-item-details-sheet.tsx (3)
src/stores/calendar/store.ts (1)
useCalendarStore(61-337)src/stores/personnel/store.ts (1)
usePersonnelStore(37-121)src/utils/html-sanitizer.ts (1)
sanitizeHtml(12-41)
src/utils/__tests__/webview-html.test.ts (1)
src/utils/webview-html.ts (3)
sanitizeHtmlContent(18-110)generateWebViewHtml(117-219)defaultWebViewProps(224-237)
src/components/calendar/enhanced-calendar-view.tsx (1)
src/lib/utils.ts (3)
formatLocalDateString(243-248)getTodayLocalString(254-256)isSameDate(258-281)
src/components/calendar/__tests__/compact-calendar-item.test.tsx (1)
src/components/calendar/compact-calendar-item.tsx (1)
CompactCalendarItem(20-121)
src/utils/webview-html.ts (1)
src/utils/html-sanitizer.ts (1)
sanitizeHtml(12-41)
src/stores/calendar/__tests__/store.test.ts (1)
src/stores/calendar/store.ts (1)
useCalendarStore(61-337)
🪛 LanguageTool
docs/webview-security-implementation.md
[grammar] ~24-~24: There might be a mistake here.
Context: ... JavaScript execution within the WebView - Prevents script-based XSS attacks - Elim...
(QB_NEW_EN)
[grammar] ~25-~25: There might be a mistake here.
Context: ...View - Prevents script-based XSS attacks - Eliminates DOM manipulation through inje...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...s localStorage and sessionStorage access - Blocks potential data persistence attack...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...locks potential data persistence attacks - Eliminates cross-session data leakage #...
(QB_NEW_EN)
[grammar] ~42-~42: There might be a mistake here.
Context: ...file system - Blocks file:// URL schemes - Eliminates potential local file inclusio...
(QB_NEW_EN)
[grammar] ~49-~49: There might be a mistake here.
Context: ...``` - Restricts allowed origins to only about:blank - Prevents navigation to external domains ...
(QB_NEW_EN)
[grammar] ~50-~50: There might be a mistake here.
Context: ... Prevents navigation to external domains - Blocks potential redirect attacks #### ...
(QB_NEW_EN)
[grammar] ~93-~93: There might be a mistake here.
Context: ... dangerous HTML content: #### Features: - Script Tag Removal: Removes <script>...
(QB_NEW_EN)
[grammar] ~112-~112: There might be a mistake here.
Context: ...ips all HTML tags for plain text display - isHtmlSafe(): Checks if HTML content contains danger...
(QB_NEW_EN)
[grammar] ~113-~113: There might be a mistake here.
Context: ...HTML content contains dangerous patterns - sanitizeEventForLogging(): Sanitizes events for logging purposes ...
(QB_NEW_EN)
[grammar] ~120-~120: There might be a mistake here.
Context: ....test.tsx`) Comprehensive tests ensure: - WebView security properties are correctl...
(QB_NEW_EN)
[grammar] ~128-~128: There might be a mistake here.
Context: ..._/html-sanitizer.test.ts`) Tests cover: - Removal of dangerous HTML elements - Eve...
(QB_NEW_EN)
[grammar] ~129-~129: There might be a mistake here.
Context: ...er: - Removal of dangerous HTML elements - Event handler stripping - JavaScript URL...
(QB_NEW_EN)
[grammar] ~130-~130: There might be a mistake here.
Context: ... HTML elements - Event handler stripping - JavaScript URL blocking - CSS expression...
(QB_NEW_EN)
[grammar] ~131-~131: There might be a mistake here.
Context: ...dler stripping - JavaScript URL blocking - CSS expression prevention - Edge case ha...
(QB_NEW_EN)
[grammar] ~132-~132: There might be a mistake here.
Context: ...URL blocking - CSS expression prevention - Edge case handling ## Best Practices Im...
(QB_NEW_EN)
[grammar] ~137-~137: There might be a mistake here.
Context: ...ty (sanitization + WebView restrictions) 2. Least Privilege: Only minimum necessar...
(QB_NEW_EN)
[grammar] ~138-~138: There might be a mistake here.
Context: ...ly minimum necessary permissions granted 3. Input Validation: All HTML content san...
(QB_NEW_EN)
[grammar] ~139-~139: There might be a mistake here.
Context: ... HTML content sanitized before rendering 4. Content Security Policy: Strict CSP he...
(QB_NEW_EN)
[grammar] ~140-~140: There might be a mistake here.
Context: ...ict CSP headers prevent resource loading 5. Navigation Control: Explicit allow-lis...
(QB_NEW_EN)
[grammar] ~141-~141: There might be a mistake here.
Context: ...ol**: Explicit allow-list for navigation 6. Error Handling: Graceful degradation f...
(QB_NEW_EN)
[grammar] ~158-~158: There might be a mistake here.
Context: ...hen creating calendar item descriptions: - Avoid inline JavaScript or event handler...
(QB_NEW_EN)
docs/webview-html-security-implementation.md
[grammar] ~16-~16: There might be a mistake here.
Context: ...This allowed for several attack vectors: - Script injection: `<script>alert('XSS'...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ...attack vectors: - Script injection: <script>alert('XSS')</script> - iframe embedding: `<iframe src="https:...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ...XSS')</script>- **iframe embedding**:<iframe src="https://evil.com"></iframe>- **Meta refresh attacks**:<meta http-equi...
(QB_NEW_EN)
[grammar] ~19-~19: There might be a mistake here.
Context: ...></iframe>- **Meta refresh attacks**:- **Event handler attributes**:<img onclic...
(QB_NEW_EN)
[grammar] ~20-~20: There might be a mistake here.
Context: ....com">- **Event handler attributes**:- **Data URIs**:<img src="data:text/html,<...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...lert('XSS')" src="x">- **Data URIs**:- **JavaScript URIs**:<a href="javascript:...
(QB_NEW_EN)
[grammar] ~40-~40: There might be a mistake here.
Context: ...ecurity Configuration #### Allowed Tags Only safe, commonly used HTML elements a...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...mmonly used HTML elements are permitted: - Text elements: p, div, span, br,...
(QB_NEW_EN)
[grammar] ~42-~42: There might be a mistake here.
Context: ...ext elements: p, div, span, br, hr - Headers: h1 through h6 - Formatting:...
(QB_NEW_EN)
[grammar] ~43-~43: There might be a mistake here.
Context: ...an, br, hr- Headers:h1throughh6- Formatting:strong, b, em, i, u...
(QB_NEW_EN)
[grammar] ~44-~44: There might be a mistake here.
Context: ..., em, i, u, s, small, sub, sup- Lists:ul, ol, li- Other:blockq...
(QB_NEW_EN)
[grammar] ~45-~45: There might be a mistake here.
Context: ...all, sub, sup- Lists:ul, ol, li- Other:blockquote, pre, code, tab...
(QB_NEW_EN)
[grammar] ~48-~48: There might be a mistake here.
Context: ...mg, dl, dt, dd` #### Blocked Tags Dangerous elements are completely remove...
(QB_NEW_EN)
[grammar] ~55-~55: There might be a mistake here.
Context: ...esh - Redirects #### Allowed Attributes Strict allowlist per tag: - Links (a):...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ...wed Attributes Strict allowlist per tag: - Links (a): href, title - Images (`...
(QB_NEW_EN)
[grammar] ~57-~57: There might be a mistake here.
Context: ...lowlist per tag: - Links (a): href, title - Images (img): src, alt, title, `...
(QB_NEW_EN)
[grammar] ~58-~58: There might be a mistake here.
Context: ...img): src, alt, title, width, height - Tables (table, th, td): Basic tabl...
(QB_NEW_EN)
[grammar] ~59-~59: There might be a mistake here.
Context: ...le, th, td): Basic table attributes - All elements (*): style, class` (wi...
(QB_NEW_EN)
[grammar] ~62-~62: There might be a mistake here.
Context: ...trictions) #### URL Scheme Restrictions - General allowed schemes: http, `http...
(QB_NEW_EN)
[grammar] ~63-~63: There might be a mistake here.
Context: ...d schemes**: http, https, mailto, tel - Images: Only http and https (block...
(QB_NEW_EN)
[grammar] ~64-~64: There might be a mistake here.
Context: ...ttps(blocksdata:andjavascript:) - **Links**: http, https, mailto, tel...
(QB_NEW_EN)
[grammar] ~67-~67: There might be a mistake here.
Context: ...vascript:`) #### CSS Property Allowlist Only safe CSS properties are permitted i...
(QB_NEW_EN)
[grammar] ~68-~68: There might be a mistake here.
Context: ...ies are permitted in style attributes: - Colors: color, background-color (val...
(QB_NEW_EN)
[grammar] ~69-~69: There might be a mistake here.
Context: ...ound-color(validated hex/rgb patterns) - Typography:font-size, font-weight`, ...
(QB_NEW_EN)
[grammar] ~73-~73: There might be a mistake here.
Context: ...h, height` #### Event Handler Removal All attributes starting with "on" are au...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: ...rity measures: ### XSS Prevention Tests - Script tag removal - Event handler attri...
(QB_NEW_EN)
[grammar] ~96-~96: There might be a mistake here.
Context: ...SS Prevention Tests - Script tag removal - Event handler attribute stripping - Java...
(QB_NEW_EN)
[grammar] ~97-~97: There might be a mistake here.
Context: ...oval - Event handler attribute stripping - JavaScript URI neutralization - Data URI...
(QB_NEW_EN)
[grammar] ~98-~98: There might be a mistake here.
Context: ...tripping - JavaScript URI neutralization - Data URI blocking ### Content Preservat...
(QB_NEW_EN)
[grammar] ~101-~101: There might be a mistake here.
Context: ...blocking ### Content Preservation Tests - Safe HTML elements are preserved - Basic...
(QB_NEW_EN)
[grammar] ~129-~129: There might be a mistake here.
Context: ...duction dependency for HTML sanitization - @types/sanitize-html (v2.16.0): Develo...
(QB_NEW_EN)
[grammar] ~153-~153: There might be a mistake here.
Context: ...n**: Eliminates script injection attacks 2. Content Isolation: Prevents iframe-bas...
(QB_NEW_EN)
[grammar] ~154-~154: There might be a mistake here.
Context: ...olation**: Prevents iframe-based attacks 3. URL Safety: Blocks dangerous data: and...
(QB_NEW_EN)
[grammar] ~155-~155: There might be a mistake here.
Context: ...cks dangerous data: and javascript: URIs 4. Event Blocking: Removes all event hand...
(QB_NEW_EN)
[grammar] ~156-~156: There might be a mistake here.
Context: ...**: Removes all event handler attributes 5. CSS Safety: Restricts styling to safe ...
(QB_NEW_EN)
[grammar] ~157-~157: There might be a mistake here.
Context: ...estricts styling to safe properties only 6. Redirect Prevention: Blocks meta refre...
(QB_NEW_EN)
[grammar] ~162-~162: There might be a mistake here.
Context: ...n maintains full backward compatibility: - All existing function signatures unchang...
(QB_NEW_EN)
[grammar] ~170-~170: There might be a mistake here.
Context: ...l performance overhead from sanitization - One-time processing per content render -...
(QB_NEW_EN)
[grammar] ~171-~171: There might be a mistake here.
Context: ...- One-time processing per content render - Cached sanitization library - No impact ...
(QB_NEW_EN)
[grammar] ~172-~172: There might be a mistake here.
Context: ...ent render - Cached sanitization library - No impact on React Native WebView perfor...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
docs/webview-security-implementation.md
93-93: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
102-102: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🪛 GitHub Check: CodeQL
src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx
[failure] 130-130: Incomplete multi-character sanitization
This string may still contain <script, which may cause an HTML element injection vulnerability.
[failure] 130-130: Bad HTML filtering regexp
This regular expression does not match script end tags like </script >.
src/utils/html-sanitizer.ts
[failure] 16-16: Incomplete multi-character sanitization
This string may still contain <script, which may cause an HTML element injection vulnerability.
[failure] 16-16: Bad HTML filtering regexp
This regular expression does not match script end tags like </script >.
[failure] 19-19: Incomplete multi-character sanitization
This string may still contain <iframe, which may cause an HTML element injection vulnerability.
[failure] 29-29: Incomplete multi-character sanitization
This string may still contain on, which may cause an HTML attribute injection vulnerability.
[failure] 52-52: Incomplete multi-character sanitization
This string may still contain <script, which may cause an HTML element injection vulnerability.
⏰ 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). (1)
- GitHub Check: test
🔇 Additional comments (18)
src/stores/calendar/__tests__/store.test.ts (1)
308-339: Upcoming range expectations are precise and correctly stubbed.The date-fns mocks for startOfDay/addDays/endOfDay/format drive a deterministic range. LGTM.
package.json (2)
161-161: Dependency addition looks correct; aligns with WebView sanitization goal.Adding sanitize-html is a good move for untrusted content. This will let us drop brittle regex-based sanitization. I’m proposing concrete refactors in src/utils/html-sanitizer.ts to actually use this lib.
180-180: Types added appropriately.@types/sanitize-html matches the new runtime dep and will improve DX. No issues.
src/app/_layout.tsx (1)
75-75: Verified: splash screen is hidden elsewhereThe audit script confirms that after calling
SplashScreen.preventAutoHideAsync()insrc/app/_layout.tsx(line 75), the app subsequently callsSplashScreen.hideAsync()in the alternate layout file. No further changes are needed.• Hide call found at
src/app/(app)/_layout.tsx:67→await SplashScreen.hideAsync();src/translations/ar.json (3)
62-62: Terminology tweak LGTM.calendar.confirmSignup → "تأكيد" reads fine and matches the shorter English "Confirm".
342-342: New common.discard key LGTM.Matches the unsaved-changes flow introduced elsewhere.
607-613: No outdated translation key references foundI’ve searched the entire codebase for calls to the old flat keys (
body_required,recipients_required,subject_required) as well as the previous namespace (messages.body_required,messages.recipients_required,messages.subject_required). There are no remaining references—everything now correctly usesmessages.validation.<key>.src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx (1)
191-197: Keep origin whitelist consistent with implementationYou assert
['about:blank']. Verify the component (and shared defaults) actually setabout:blank, notabout:. If defaults useabout:, this test will be brittle.src/translations/es.json (1)
607-613: Validation key relocation verifiedI ran searches across
.ts,.tsx,.js, and.jsxfiles for any lingering references to the old flat keys (messages.body_required,messages.recipients_required,messages.subject_required, and theirt('messages.<key>')usages) and found no matches. It appears all calls have been updated to use the nestedmessages.validation.*paths. No further changes are needed here.src/utils/__tests__/webview-html.test.ts (1)
5-13: Solid XSS coverage for script-stripping — LGTMThese assertions correctly validate script removal while preserving safe content.
src/components/calendar/calendar-item-details-sheet.tsx (1)
62-71: Personnel auto-fetch logic — LGTMThe guard on isOpen, empty personnel, and not loading avoids duplicate fetches and churn.
src/components/calendar/__tests__/calendar-item-details-sheet.test.tsx (2)
830-840: WebView prop assertion may need alignment with shared defaultsIf you adopt defaultWebViewProps in the component (recommended), update this test to assert those defaults (e.g., originWhitelist ['about:']). Otherwise, keep the current expectation of ['about:blank'].
818-828: Dark theme CSS assertions — LGTMThe checks for dark text and background colors align with the generator and component behavior.
src/utils/webview-html.ts (2)
3-11: Typed options and clear API — LGTMInterface and defaults are straightforward and consistent with usage.
221-237: Secure defaults object — LGTMThe defaults are appropriately locked down for inline HTML (about:), no JS, no DOM storage, and software layer type for Android.
src/components/calendar/enhanced-calendar-view.tsx (3)
111-111: Month-end calculation is TZ-safe and correctUsing new Date(year, month, 0) with a 1-based month from DateData yields the last day of that month. Paired with formatLocalDateString this avoids UTC drift.
129-130: LGTM on initial load rangeUsing formatLocalDateString(new Date(now.getFullYear(), now.getMonth() + 1, 0)) correctly computes the last day of the current month in local time.
120-123: Ensure local date is used for initialcurrentMonthstateThe initial state in
src/components/calendar/enhanced-calendar-view.tsxstill appears to use a UTC-based derivation (new Date().toISOString().slice(0, 7)), which can drift around local midnight. This should be updated to use the same local-safe helper asgoToToday.• Location to verify: around line 29 in
enhanced-calendar-view.tsx
• Apply this diff to align withgetTodayLocalString():- const [currentMonth, setCurrentMonth] = useState(new Date().toISOString().slice(0, 7)); // YYYY-MM format + const [currentMonth, setCurrentMonth] = useState(getTodayLocalString().slice(0, 7)); // YYYY-MM format (local TZ-safe)This ensures both the initial render and the “go to today” action use local time and avoid UTC month drift.
| #### 5. Navigation Control | ||
| ```tsx | ||
| onShouldStartLoadWithRequest={(request) => { | ||
| return request.url === 'about:blank' || request.url.startsWith('data:'); | ||
| }} | ||
| onNavigationStateChange={(navState) => { | ||
| if (navState.url !== 'about:blank' && !navState.url.startsWith('data:')) { | ||
| return false; | ||
| } | ||
| }} | ||
| ``` |
There was a problem hiding this comment.
onNavigationStateChange cannot cancel navigation; remove misleading return
onNavigationStateChange is informational. return false here has no effect. Rely on onShouldStartLoadWithRequest to block.
-onNavigationStateChange={(navState) => {
- if (navState.url !== 'about:blank' && !navState.url.startsWith('data:')) {
- return false;
- }
-}}
+// Keep this purely for analytics/logging if needed; blocking happens in onShouldStartLoadWithRequest
+onNavigationStateChange={() => {}}📝 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.
| #### 5. Navigation Control | |
| ```tsx | |
| onShouldStartLoadWithRequest={(request) => { | |
| return request.url === 'about:blank' || request.url.startsWith('data:'); | |
| }} | |
| onNavigationStateChange={(navState) => { | |
| if (navState.url !== 'about:blank' && !navState.url.startsWith('data:')) { | |
| return false; | |
| } | |
| }} | |
| ``` | |
| #### 5. Navigation Control |
🤖 Prompt for AI Agents
In docs/webview-security-implementation.md around lines 53 to 63, the
onNavigationStateChange handler contains a misleading "return false" which
cannot cancel navigation; remove the return statement (or remove the handler
entirely) and update the text to state that onNavigationStateChange is
informational only and that onShouldStartLoadWithRequest must be used to block
navigation.
| #### Usage: | ||
| ```tsx | ||
| import { sanitizeHtml } from '@/utils/html-sanitizer'; | ||
|
|
||
| // In the WebView HTML | ||
| <body>${sanitizeHtml(item.Description)}</body> | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Fix API name in example to match util export
Docs show sanitizeHtml(...); the util exports sanitizeHtmlContent(...). Align the example (or re-export an alias).
-import { sanitizeHtml } from '@/utils/html-sanitizer';
+import { sanitizeHtmlContent } from '@/utils/html-sanitizer';
...
-<body>${sanitizeHtml(item.Description)}</body>
+<body>${sanitizeHtmlContent(item.Description)}</body>📝 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.
| #### Usage: | |
| ```tsx | |
| import { sanitizeHtml } from '@/utils/html-sanitizer'; | |
| // In the WebView HTML | |
| <body>${sanitizeHtml(item.Description)}</body> | |
| ``` | |
| #### Usage: |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
102-102: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🤖 Prompt for AI Agents
In docs/webview-security-implementation.md around lines 102 to 109, the example
uses sanitizeHtml(...) but the utility actually exports
sanitizeHtmlContent(...); update the example to call
sanitizeHtmlContent(item.Description) to match the exported API, or
alternatively add a re-export/alias named sanitizeHtml in the utility module and
keep the doc unchanged; ensure the import statement and usage in the code block
both reflect the chosen fix.
| //useAuth().hydrate(); | ||
| // Prevent the splash screen from auto-hiding before asset loading is complete. | ||
| //SplashScreen.preventAutoHideAsync(); | ||
| SplashScreen.preventAutoHideAsync(); |
There was a problem hiding this comment.
preventAutoHideAsync without a guaranteed hide can leave the app stuck on the splash screen.
Right now we prevent auto-hide but I don’t see a corresponding SplashScreen.hideAsync() that’s guaranteed to run after initial work. Add a safe hide on the first root layout to avoid regressions while keeping your current initialization intact.
Apply this minimal diff to ensure the splash is always hidden once the UI mounts:
@@
- return (
- <SafeAreaProvider>
- <GestureHandlerRootView>
- <KeyboardProvider>{Env.APTABASE_APP_KEY && !__DEV__ ? <AptabaseProviderWrapper appKey={Env.APTABASE_APP_KEY}>{renderContent()}</AptabaseProviderWrapper> : renderContent()}</KeyboardProvider>
- </GestureHandlerRootView>
- </SafeAreaProvider>
- );
+ return (
+ <SafeAreaProvider>
+ <GestureHandlerRootView
+ onLayout={() => {
+ // idempotent; safe to call multiple times
+ SplashScreen.hideAsync().catch(() => {});
+ }}
+ >
+ <KeyboardProvider>
+ {Env.APTABASE_APP_KEY && !__DEV__ ? (
+ <AptabaseProviderWrapper appKey={Env.APTABASE_APP_KEY}>{renderContent()}</AptabaseProviderWrapper>
+ ) : (
+ renderContent()
+ )}
+ </KeyboardProvider>
+ </GestureHandlerRootView>
+ </SafeAreaProvider>
+ );If you prefer to wait for all startup promises, we can also gate hideAsync behind a Promise.allSettled of the tasks initiated in useEffect. I can sketch that out if you want.
📝 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.
| SplashScreen.preventAutoHideAsync(); | |
| return ( | |
| <SafeAreaProvider> | |
| <GestureHandlerRootView | |
| onLayout={() => { | |
| // idempotent; safe to call multiple times | |
| SplashScreen.hideAsync().catch(() => {}); | |
| }} | |
| > | |
| <KeyboardProvider> | |
| {Env.APTABASE_APP_KEY && !__DEV__ ? ( | |
| <AptabaseProviderWrapper appKey={Env.APTABASE_APP_KEY}> | |
| {renderContent()} | |
| </AptabaseProviderWrapper> | |
| ) : ( | |
| renderContent() | |
| )} | |
| </KeyboardProvider> | |
| </GestureHandlerRootView> | |
| </SafeAreaProvider> | |
| ); |
🤖 Prompt for AI Agents
In src/app/_layout.tsx around line 75, you call
SplashScreen.preventAutoHideAsync() but never guarantee a corresponding
SplashScreen.hideAsync(), which can leave the app stuck on the splash screen;
fix this by adding a hide on the first root layout mount (e.g., in a useEffect
that runs once) that calls SplashScreen.hideAsync() and handles errors
(try/catch or .catch), and include a safe timeout fallback (clearable) so
hideAsync is forced after a short delay if other initialization hangs;
alternatively, if you prefer waiting for startup tasks, gate hideAsync behind
Promise.allSettled of those tasks and then call hideAsync in the settled
handler.
src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx
Show resolved
Hide resolved
| it('should sanitize HTML content in description', () => { | ||
| const onCloseMock = jest.fn(); | ||
|
|
||
| const { getByTestId } = render( | ||
| <CalendarItemDetailsSheet | ||
| item={mockItem} | ||
| isOpen={true} | ||
| onClose={onCloseMock} | ||
| /> | ||
| ); | ||
|
|
||
| // Check that WebView is present | ||
| const webview = getByTestId('webview'); | ||
| expect(webview).toBeTruthy(); | ||
|
|
||
| // Verify WebView props include security settings | ||
| expect(webview.props['data-js-enabled']).toBe(false); | ||
| expect(webview.props['data-dom-storage']).toBe(false); | ||
| expect(webview.props['data-file-access']).toBe(false); | ||
| expect(webview.props['data-universal-access']).toBe(false); | ||
| expect(JSON.parse(webview.props['data-origin-whitelist'])).toEqual(['about:blank']); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Assert that sanitized HTML actually reaches the WebView
The test title says “sanitize HTML…”, but it only checks security props. Add a content assertion to prove <script> is gone while safe content remains.
// Check that WebView is present
const webview = getByTestId('webview');
expect(webview).toBeTruthy();
+ // Verify sanitized HTML was passed to WebView
+ expect(webview.props.source?.html).toBeDefined();
+ expect(webview.props.source.html).not.toMatch(/<script/i);
+ expect(webview.props.source.html).toMatch(/Safe content/);📝 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.
| it('should sanitize HTML content in description', () => { | |
| const onCloseMock = jest.fn(); | |
| const { getByTestId } = render( | |
| <CalendarItemDetailsSheet | |
| item={mockItem} | |
| isOpen={true} | |
| onClose={onCloseMock} | |
| /> | |
| ); | |
| // Check that WebView is present | |
| const webview = getByTestId('webview'); | |
| expect(webview).toBeTruthy(); | |
| // Verify WebView props include security settings | |
| expect(webview.props['data-js-enabled']).toBe(false); | |
| expect(webview.props['data-dom-storage']).toBe(false); | |
| expect(webview.props['data-file-access']).toBe(false); | |
| expect(webview.props['data-universal-access']).toBe(false); | |
| expect(JSON.parse(webview.props['data-origin-whitelist'])).toEqual(['about:blank']); | |
| }); | |
| it('should sanitize HTML content in description', () => { | |
| const onCloseMock = jest.fn(); | |
| const { getByTestId } = render( | |
| <CalendarItemDetailsSheet | |
| item={mockItem} | |
| isOpen={true} | |
| onClose={onCloseMock} | |
| /> | |
| ); | |
| // Check that WebView is present | |
| const webview = getByTestId('webview'); | |
| expect(webview).toBeTruthy(); | |
| // Verify sanitized HTML was passed to WebView | |
| expect(webview.props.source?.html).toBeDefined(); | |
| expect(webview.props.source.html).not.toMatch(/<script/i); | |
| expect(webview.props.source.html).toMatch(/Safe content/); | |
| // Verify WebView props include security settings | |
| expect(webview.props['data-js-enabled']).toBe(false); | |
| expect(webview.props['data-dom-storage']).toBe(false); | |
| expect(webview.props['data-file-access']).toBe(false); | |
| expect(webview.props['data-universal-access']).toBe(false); | |
| expect(JSON.parse(webview.props['data-origin-whitelist'])).toEqual(['about:blank']); | |
| }); |
🤖 Prompt for AI Agents
In
src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx
around lines 176 to 197, the test only verifies WebView security props but does
not assert that sanitized HTML is actually passed to the WebView; update the
test to inspect the WebView's content prop (e.g., source.html or the prop your
component uses to pass HTML) and add assertions that any injected <script> tags
are removed/absent while expected safe content (e.g., specific text or markup
from mockItem.description) remains present, using string checks or parsing to
validate both the absence of dangerous tags and presence of safe content.
| <Box className="w-full rounded-lg bg-gray-50 p-1 dark:bg-gray-700"> | ||
| <WebView | ||
| style={[styles.container, { height: 120 }]} | ||
| originWhitelist={['about:blank']} | ||
| scrollEnabled={false} | ||
| showsVerticalScrollIndicator={false} | ||
| javaScriptEnabled={false} | ||
| domStorageEnabled={false} | ||
| allowFileAccess={false} | ||
| allowUniversalAccessFromFileURLs={false} | ||
| onShouldStartLoadWithRequest={(request) => { | ||
| // Only allow the initial HTML load with about:blank or data URLs | ||
| return request.url === 'about:blank' || request.url.startsWith('data:'); | ||
| }} | ||
| onNavigationStateChange={(navState) => { | ||
| // Prevent any navigation away from the initial HTML | ||
| if (navState.url !== 'about:blank' && !navState.url.startsWith('data:')) { | ||
| return false; | ||
| } | ||
| }} | ||
| source={{ | ||
| html: ` | ||
| <!DOCTYPE html> | ||
| <html> | ||
| <head> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0"> | ||
| <meta http-equiv="Content-Security-Policy" content="default-src 'none'; style-src 'unsafe-inline';"> | ||
| <style> | ||
| body { | ||
| color: ${colorScheme === 'dark' ? '#E5E7EB' : '#1F2937'}; | ||
| font-family: system-ui, -apple-system, sans-serif; | ||
| margin: 0; | ||
| padding: 8px; | ||
| font-size: 16px; | ||
| line-height: 1.5; | ||
| background-color: ${colorScheme === 'dark' ? '#374151' : '#F9FAFB'}; | ||
| } | ||
| * { | ||
| max-width: 100%; | ||
| } | ||
| </style> | ||
| </head> | ||
| <body>${sanitizeHtml(item.Description)}</body> | ||
| </html> | ||
| `, | ||
| baseUrl: 'about:blank', | ||
| }} | ||
| androidLayerType="software" | ||
| testID="webview" | ||
| /> | ||
| </Box> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace inlined HTML and ad-hoc WebView props with shared utilities
Leverage generateWebViewHtml + defaultWebViewProps to standardize security, styling, and behavior. Also remove the onNavigationStateChange boolean return (ignored by RN WebView).
- <Box className="w-full rounded-lg bg-gray-50 p-1 dark:bg-gray-700">
- <WebView
- style={[styles.container, { height: 120 }]}
- originWhitelist={['about:blank']}
- scrollEnabled={false}
- showsVerticalScrollIndicator={false}
- javaScriptEnabled={false}
- domStorageEnabled={false}
- allowFileAccess={false}
- allowUniversalAccessFromFileURLs={false}
- onShouldStartLoadWithRequest={(request) => {
- // Only allow the initial HTML load with about:blank or data URLs
- return request.url === 'about:blank' || request.url.startsWith('data:');
- }}
- onNavigationStateChange={(navState) => {
- // Prevent any navigation away from the initial HTML
- if (navState.url !== 'about:blank' && !navState.url.startsWith('data:')) {
- return false;
- }
- }}
- source={{
- html: `
- <!DOCTYPE html>
- <html>
- <head>
- <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0">
- <meta http-equiv="Content-Security-Policy" content="default-src 'none'; style-src 'unsafe-inline';">
- <style>
- body {
- color: ${colorScheme === 'dark' ? '#E5E7EB' : '#1F2937'};
- font-family: system-ui, -apple-system, sans-serif;
- margin: 0;
- padding: 8px;
- font-size: 16px;
- line-height: 1.5;
- background-color: ${colorScheme === 'dark' ? '#374151' : '#F9FAFB'};
- }
- * { max-width: 100%; }
- </style>
- </head>
- <body>${sanitizeHtml(item.Description)}</body>
- </html>
- `,
- baseUrl: 'about:blank',
- }}
- androidLayerType="software"
- testID="webview"
- />
- </Box>
+ <Box className="w-full rounded-lg bg-gray-50 p-1 dark:bg-gray-700">
+ <WebView
+ style={[styles.container, { minHeight: 120 }]}
+ {...defaultWebViewProps}
+ // Keep scroll disabled for layout expectations
+ scrollEnabled={false}
+ showsVerticalScrollIndicator={false}
+ onShouldStartLoadWithRequest={(request) => request.url === 'about:blank' || request.url.startsWith('data:')}
+ source={{
+ html: generateWebViewHtml({
+ content: item.Description,
+ isDarkMode: colorScheme === 'dark',
+ }),
+ baseUrl: 'about:blank',
+ }}
+ androidLayerType="software"
+ testID="webview"
+ />
+ </Box>📝 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.
| <Box className="w-full rounded-lg bg-gray-50 p-1 dark:bg-gray-700"> | |
| <WebView | |
| style={[styles.container, { height: 120 }]} | |
| originWhitelist={['about:blank']} | |
| scrollEnabled={false} | |
| showsVerticalScrollIndicator={false} | |
| javaScriptEnabled={false} | |
| domStorageEnabled={false} | |
| allowFileAccess={false} | |
| allowUniversalAccessFromFileURLs={false} | |
| onShouldStartLoadWithRequest={(request) => { | |
| // Only allow the initial HTML load with about:blank or data URLs | |
| return request.url === 'about:blank' || request.url.startsWith('data:'); | |
| }} | |
| onNavigationStateChange={(navState) => { | |
| // Prevent any navigation away from the initial HTML | |
| if (navState.url !== 'about:blank' && !navState.url.startsWith('data:')) { | |
| return false; | |
| } | |
| }} | |
| source={{ | |
| html: ` | |
| <!DOCTYPE html> | |
| <html> | |
| <head> | |
| <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0"> | |
| <meta http-equiv="Content-Security-Policy" content="default-src 'none'; style-src 'unsafe-inline';"> | |
| <style> | |
| body { | |
| color: ${colorScheme === 'dark' ? '#E5E7EB' : '#1F2937'}; | |
| font-family: system-ui, -apple-system, sans-serif; | |
| margin: 0; | |
| padding: 8px; | |
| font-size: 16px; | |
| line-height: 1.5; | |
| background-color: ${colorScheme === 'dark' ? '#374151' : '#F9FAFB'}; | |
| } | |
| * { | |
| max-width: 100%; | |
| } | |
| </style> | |
| </head> | |
| <body>${sanitizeHtml(item.Description)}</body> | |
| </html> | |
| `, | |
| baseUrl: 'about:blank', | |
| }} | |
| androidLayerType="software" | |
| testID="webview" | |
| /> | |
| </Box> | |
| <Box className="w-full rounded-lg bg-gray-50 p-1 dark:bg-gray-700"> | |
| <WebView | |
| style={[styles.container, { minHeight: 120 }]} | |
| {...defaultWebViewProps} | |
| // Keep scroll disabled for layout expectations | |
| scrollEnabled={false} | |
| showsVerticalScrollIndicator={false} | |
| onShouldStartLoadWithRequest={(request) => | |
| request.url === 'about:blank' || request.url.startsWith('data:') | |
| } | |
| source={{ | |
| html: generateWebViewHtml({ | |
| content: item.Description, | |
| isDarkMode: colorScheme === 'dark', | |
| }), | |
| baseUrl: 'about:blank', | |
| }} | |
| androidLayerType="software" | |
| testID="webview" | |
| /> | |
| </Box> |
| export function isHtmlSafe(html: string): boolean { | ||
| if (!html) return true; | ||
|
|
||
| // Check for dangerous patterns | ||
| const dangerousPatterns = [ | ||
| /<script/i, | ||
| /<iframe/i, | ||
| /<object/i, | ||
| /<embed/i, | ||
| /<form/i, | ||
| /javascript:/i, | ||
| /on\w+\s*=/i, // Event handlers | ||
| /expression\s*\(/i, // CSS expressions | ||
| ]; | ||
|
|
||
| return !dangerousPatterns.some((pattern) => pattern.test(html)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make safety check policy-driven rather than regex-driven.
Return whether the sanitized output equals the input under the same SANITIZE_CONFIG. This avoids chasing new vectors with ad-hoc patterns.
Apply:
export function isHtmlSafe(html: string): boolean {
- if (!html) return true;
-
- // Check for dangerous patterns
- const dangerousPatterns = [
- /<script/i,
- /<iframe/i,
- /<object/i,
- /<embed/i,
- /<form/i,
- /javascript:/i,
- /on\w+\s*=/i, // Event handlers
- /expression\s*\(/i, // CSS expressions
- ];
-
- return !dangerousPatterns.some((pattern) => pattern.test(html));
+ if (!html) return true;
+ const cleaned = sanitizeLib(html, SANITIZE_CONFIG).trim();
+ // If sanitization changed anything, input wasn't safe
+ return cleaned === html.trim();
}This keeps tests conceptually intact while eliminating regex maintenance and covering attributes like srcdoc, srcset, malformed end-tags, etc.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/utils/html-sanitizer.ts around lines 60 to 76, replace the regex-based
isHtmlSafe check with a policy-driven comparison: import/require the project
SANITIZE_CONFIG and the same sanitization function used elsewhere, run the
sanitizer on the input using that config, and return true only if the sanitized
output strictly equals the original input (handle empty/null input by returning
true). Ensure you use the identical config instance (not a hardcoded set of
patterns) so attributes like srcdoc/srcset and malformed tags are covered and
tests remain unchanged.
| sanitizeHtmlContent: (html: string) => { | ||
| if (!html) return ''; | ||
| // Strip script tags robustly with non-greedy pattern that handles newlines and nested content | ||
| return html.replace(/<script\s*[^>]*>[\s\S]*?<\/script\s*>/gi, ''); |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 months ago
The most reliable fix is to repeatedly apply the regular expression replacement until no further <script> tags remain. This technique ensures that dangerous tags and their remnants cannot reappear in the output due to partial matching or nested/malformed HTML. To achieve this, we should change the mocked sanitizeHtmlContent implementation to use a loop, as recommended in the Reference section. Specifically, replace the script tag matching line so that it repeatedly replaces matches until the input is unchanged, and then returns the fully sanitized string. Only the mock implementation at lines 130–134 of the test file needs updating.
| @@ -129,8 +129,13 @@ | ||
| jest.mock('@/utils/webview-html', () => ({ | ||
| sanitizeHtmlContent: (html: string) => { | ||
| if (!html) return ''; | ||
| // Strip script tags robustly with non-greedy pattern that handles newlines and nested content | ||
| return html.replace(/<script\s*[^>]*>[\s\S]*?<\/script\s*>/gi, ''); | ||
| // Strip script tags robustly by repeatedly replacing until no matches remain | ||
| let previous; | ||
| do { | ||
| previous = html; | ||
| html = html.replace(/<script\s*[^>]*>[\s\S]*?<\/script\s*>/gi, ''); | ||
| } while (html !== previous); | ||
| return html; | ||
| }, | ||
| })); | ||
|
|
| sanitizeHtmlContent: (html: string) => { | ||
| if (!html) return ''; | ||
| // Strip script tags robustly with non-greedy pattern that handles newlines and nested content | ||
| return html.replace(/<script\s*[^>]*>[\s\S]*?<\/script\s*>/gi, ''); |
Check failure
Code scanning / CodeQL
Bad HTML filtering regexp High test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 months ago
The best way to fix this problem is to avoid custom regex-based HTML sanitization altogether and directly use a well-tested third-party sanitization library like sanitize-html, even in mock/test situations. However, if third-party libraries cannot be used in the test context, the regex should be improved to match closing tags with any characters between </script and > (e.g., attributes, whitespace), matching how browsers parse HTML. This can be done by changing the closing tag pattern from </script\s*> to </script[^>]*>. Specifically, in src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx, line 133, replace:
html.replace(/<script\s*[^>]*>[\s\S]*?<\/script\s*>/gi, '');with:
html.replace(/<script\b[^>]*>[\s\S]*?<\/script[^>]*>/gi, '');No further changes are needed, as this robustifies the regex in place.
| @@ -129,8 +129,8 @@ | ||
| jest.mock('@/utils/webview-html', () => ({ | ||
| sanitizeHtmlContent: (html: string) => { | ||
| if (!html) return ''; | ||
| // Strip script tags robustly with non-greedy pattern that handles newlines and nested content | ||
| return html.replace(/<script\s*[^>]*>[\s\S]*?<\/script\s*>/gi, ''); | ||
| // Strip script tags robustly with non-greedy pattern that matches permissive script closing tags | ||
| return html.replace(/<script\b[^>]*>[\s\S]*?<\/script[^>]*>/gi, ''); | ||
| }, | ||
| })); | ||
|
|
| function isSafeScheme(url: string): boolean { | ||
| if (!url) return false; | ||
| const normalizedUrl = url.toLowerCase().trim(); | ||
| return !normalizedUrl.startsWith('javascript:') && !normalizedUrl.startsWith('data:'); |
Check failure
Code scanning / CodeQL
Incomplete URL scheme check High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 months ago
To resolve the issue, the function isSafeScheme should be updated to also check and block URLs that start with the "vbscript:" scheme (case-insensitive, trimmed, as with the others). This can be accomplished by extending the logical check in the return statement on line 16 to include !normalizedUrl.startsWith('vbscript:'). No further structural changes are necessary, as this is a simple string prefix check as for the other schemes.
Changes:
- Edit the return statement (line 16) in
isSafeScheme(url: string): booleanto add "vbscript:" to the blocked schemes, alongside "javascript:" and "data:". - No new imports, method definitions, or variable definitions are needed.
| @@ -13,7 +13,7 @@ | ||
| function isSafeScheme(url: string): boolean { | ||
| if (!url) return false; | ||
| const normalizedUrl = url.toLowerCase().trim(); | ||
| return !normalizedUrl.startsWith('javascript:') && !normalizedUrl.startsWith('data:'); | ||
| return !normalizedUrl.startsWith('javascript:') && !normalizedUrl.startsWith('data:') && !normalizedUrl.startsWith('vbscript:'); | ||
| } | ||
|
|
||
| /** |
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/calendar/enhanced-calendar-view.tsx (2)
186-186: Bug: period fields are ignored with markingType="multi-dot"You set startingDay, endingDay, and color (period marking) but the Calendar is configured with markingType="multi-dot". In react-native-calendars, period fields are ignored unless markingType is "period" (single period per day) or "multi-period" (multiple overlapping periods). Result: multi-day range highlighting won’t render.
Quick fix (keeps current data structure and single-period semantics):
- markingType="multi-dot" + markingType="period"Alternative (if you need multiple overlapping ranges and dots): switch to "multi-period" and build per-day
periods: [{ startingDay, endingDay, color }]arrays, or consider "custom" marking to combine dots and shading. I can provide a concrete refactor if desired.
29-29: Avoid UTC month drift in currentMonth initializationnew Date().toISOString().slice(0, 7) uses UTC and can select the wrong month for users near month boundaries in negative/positive time zones. Prefer a local YYYY-MM.
Apply this diff:
- const [currentMonth, setCurrentMonth] = useState(new Date().toISOString().slice(0, 7)); // YYYY-MM format + const [currentMonth, setCurrentMonth] = useState(formatLocalDateString(new Date()).slice(0, 7)); // YYYY-MM (local)
♻️ Duplicate comments (4)
src/components/calendar/enhanced-calendar-view.tsx (2)
37-41: TZ-safe parsing and local range iteration looks correct; add a defensive end<start guardParsing Start/End as Date and formatting via formatLocalDateString is the right fix. While iterating the multi-day range, add a fast-fail for bad data (end before start) to avoid unnecessary work and subtle marks.
Apply this diff:
if (startDate !== endDate) { // Use local Date constructors to avoid timezone issues const start = new Date(startDateObj.getFullYear(), startDateObj.getMonth(), startDateObj.getDate()); const end = new Date(endDateObj.getFullYear(), endDateObj.getMonth(), endDateObj.getDate()); + if (end < start) { + return; // skip invalid ranges defensively + } const current = new Date(start); while (current <= end) { const dateStr = formatLocalDateString(current);Also applies to: 59-66
227-227: Switched to isSameDate for equality — good callThis matches the backend’s timezone semantics and prevents split('T') pitfalls addressed previously.
src/utils/html-sanitizer.ts (1)
232-251: isHtmlSafe should be policy-driven equality, not length heuristicHeuristic ratios can yield false positives/negatives (e.g., short content with one unsafe attr). Safer: sanitize and compare strict equality.
Apply:
export function isHtmlSafe(html: string): boolean { if (!html) return true; - - // Sanitize the content and compare with original - const sanitized = sanitizeDescription(html); - const originalLength = html.replace(/\s+/g, ' ').trim().length; - const sanitizedLength = sanitized.replace(/\s+/g, ' ').trim().length; - - // If sanitization significantly reduced the content length, it likely contained unsafe elements - // Based on testing: - // - Safe content: ratio = 1.0 (no change) - // - onclick removal: ratio = ~0.39 - // - expression removal: ratio = ~0.27 - // - javascript: removal: ratio = ~0.36 - const lengthRatio = originalLength > 0 ? sanitizedLength / originalLength : 1; - - // Content is considered unsafe if the length ratio drops below 0.8 - // This catches cases where significant content was removed due to security concerns - return lengthRatio >= 0.8 && sanitized.trim().length > 0; + const sanitized = sanitizeDescription(html); + return sanitized.trim() === html.trim(); }src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx (1)
191-201: Actually assert sanitized HTML reaches WebViewThe test name claims sanitization, but assertions only inspect security flags. Verify that <script> is removed and safe content remains.
Apply:
// Check that WebView is present const webview = getByTestId('webview'); expect(webview).toBeTruthy(); // Verify WebView props include security settings expect(webview.props['data-js-enabled']).toBe(false); expect(webview.props['data-dom-storage']).toBe(false); expect(webview.props['data-file-access']).toBe(false); expect(webview.props['data-universal-access']).toBe(false); expect(JSON.parse(webview.props['data-origin-whitelist'])).toEqual(['about:blank']); + + // Verify sanitized HTML was passed to WebView + expect(webview.props.source?.html).toBeDefined(); + expect(webview.props.source.html).not.toMatch(/<script\b/i); + expect(webview.props.source.html).toMatch(/Safe content/); + // Base URL should remain constrained + expect(webview.props.baseUrl).toBe('about:blank');
🧹 Nitpick comments (8)
src/components/calendar/enhanced-calendar-view.tsx (3)
33-33: Type markedDates precisely; avoid anyUse the prop’s declared type to tighten typing and get IDE help.
- const marked: Record<string, any> = {}; + const marked: NonNullable<CalendarProps['markedDates']> = {};
51-55: Ensure dot key is a stringreact-native-calendars expects dot keys as strings. CalendarItemId may be numeric.
- marked[startDate].dots.push({ - key: item.CalendarItemId, + marked[startDate].dots.push({ + key: String(item.CalendarItemId), color: item.TypeColor || '#3B82F6', // Use event type color or default blue });
211-221: Move inline computations out of render; use i18n locale for date formatting; prefer ternary over &&The IIFEs allocate work on every render and violate our guideline to avoid anonymous functions/heavy computations during render. Also, use the active i18n locale for toLocaleDateString. Replace the render-time IIFEs with useMemo and the conditional with a ternary.
Apply this diff in the render block:
- {/* Selected Date Info */} - {selectedDate && ( + {/* Selected Date Info */} + {selectedDate ? ( <View className="border-t border-gray-200 bg-gray-50 px-4 py-3 dark:border-gray-700 dark:bg-gray-800"> <Text className="text-sm font-medium text-gray-900 dark:text-white"> {t('calendar.selectedDate.title', { - date: (() => { - // Parse the date string properly to avoid timezone issues - const [year, month, day] = selectedDate.split('-').map(Number); - const localDate = new Date(year, month - 1, day); // month is 0-indexed - return localDate.toLocaleDateString([], { - weekday: 'long', - year: 'numeric', - month: 'long', - day: 'numeric', - }); - })(), + date: selectedDateLabel, })} </Text> - {(() => { - const eventsForDay = selectedMonthItems.filter((item) => { - // Use isSameDate for timezone-safe date comparison with .NET backend timezone-aware dates - return selectedDate ? isSameDate(item.Start, selectedDate) : false; - }); - - if (eventsForDay.length > 0) { - return <Text className="mt-1 text-xs text-gray-600 dark:text-gray-300">{t('calendar.eventsCount', { count: eventsForDay.length })}</Text>; - } - - return <Text className="mt-1 text-xs text-gray-500 dark:text-gray-400">{t('calendar.noEvents')}</Text>; - })()} + {eventsForDay.length > 0 ? ( + <Text className="mt-1 text-xs text-gray-600 dark:text-gray-300"> + {t('calendar.eventsCount', { count: eventsForDay.length })} + </Text> + ) : ( + <Text className="mt-1 text-xs text-gray-500 dark:text-gray-400"> + {t('calendar.noEvents')} + </Text> + )} </View> - )} + ) : null}And add these memoized values above the return (for example right after the markedDates useMemo):
// use the active i18n locale for formatting const { i18n } = useTranslation(); const selectedDateLabel = useMemo(() => { if (!selectedDate) return ''; const [year, month, day] = selectedDate.split('-').map(Number); const localDate = new Date(year, month - 1, day); return localDate.toLocaleDateString(i18n.language ? [i18n.language] : [], { weekday: 'long', year: 'numeric', month: 'long', day: 'numeric', }); }, [selectedDate, i18n.language]); const eventsForDay = useMemo( () => (selectedDate ? selectedMonthItems.filter((item) => isSameDate(item.Start, selectedDate)) : []), [selectedDate, selectedMonthItems] );Also applies to: 224-235
src/utils/html-sanitizer.ts (3)
22-42: Attribute value assumptions — guard against non-string valuesfilterDangerousAttributes assumes attribute values are strings (uses toLowerCase, includes). sanitize-html typically provides strings, but defensive checks avoid accidental crashes if a boolean/number slips in.
Apply:
-function filterDangerousAttributes(tag: string, name: string, value: string): boolean { +function filterDangerousAttributes(tag: string, name: string, value: unknown): boolean { + // Coerce to string for uniform checks; undefined/null -> '' + const strVal = String(value ?? ''); // Block all event handlers (onclick, onload, etc.) if (name.toLowerCase().startsWith('on')) { return false; } // Block style attributes with expressions or javascript if (name.toLowerCase() === 'style') { - const normalizedValue = value.toLowerCase(); + const normalizedValue = strVal.toLowerCase(); if (normalizedValue.includes('expression(') || normalizedValue.includes('javascript:')) { return false; } } // For URL attributes, ensure safe schemes if (['href', 'src', 'srcset', 'srcdoc'].includes(name.toLowerCase())) { - return isSafeScheme(value); + return isSafeScheme(strVal); } return true; }Also applies to: 165-184
118-128: Make allowedStyles color/font patterns precise (optional polish)The color regex accepts arbitrary hex lengths (e.g., #ff). Tighten to 3/6/8 hex; add rgba/hsla for common safe usage. Allow fractional font-sizes.
Apply:
allowedStyles: { '*': { - color: [/^#(0x)?[0-9a-f]+$/i, /^rgb\(\s*(\d+)\s*,\s*(\d+)\s*,\s*(\d+)\s*\)$/], + color: [ + /^#(?:[0-9a-f]{3}|[0-9a-f]{6}|[0-9a-f]{8})$/i, + /^rgb\(\s*(\d{1,3})\s*,\s*(\d{1,3})\s*,\s*(\d{1,3})\s*\)$/i, + /^rgba\(\s*(\d{1,3})\s*,\s*(\d{1,3})\s*,\s*(\d{1,3})\s*,\s*(0|1|0?\.\d+)\s*\)$/i + ], 'text-align': [/^left$/, /^right$/, /^center$/, /^justify$/], - 'font-size': [/^\d+(?:px|em|%)$/], + 'font-size': [/^\d*\.?\d+(?:px|em|%)$/], 'font-weight': [/^(?:normal|bold|bolder|lighter|\d+)$/], margin: [/^\d+(?:px|em|%)$/], padding: [/^\d+(?:px|em|%)$/], }, },
102-110: Keep config self-consistent and minimal
- allowedIframeHostnames/allowedScriptHostnames are redundant since iframe/script are disallowed; harmless but can be removed for clarity.
- Consider adding tel to allowedSchemes only if you intend to support dial links. WebView navigation is blocked for anything except about:blank and data:, so leaving tel out is consistent with current behavior.
If you want to keep them, no change required. If you prefer minimalism, remove these keys. Also confirm product intent for tel links.
Also applies to: 114-117, 130-133
src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx (2)
216-222: Broaden navigation security assertions (optional)Confirm tel/mailto are blocked by onShouldStartLoadWithRequest to match current policy.
Apply:
expect(shouldLoad({ url: 'about:blank' })).toBe(true); expect(shouldLoad({ url: 'data:text/html,<html></html>' })).toBe(true); expect(shouldLoad({ url: 'http://evil.com' })).toBe(false); expect(shouldLoad({ url: 'javascript:alert(1)' })).toBe(false); + expect(sh ouldLoad({ url: 'mailto:test@example.com' })).toBe(false); + expect(shouldLoad({ url: 'tel:+15551234567' })).toBe(false);
59-74: WebView mock is solid; small improvement to expose source.html in data- for easier debugging (optional)*You already pass through all props, which is sufficient. If you want easier snapshots/logging later, you can mirror source.html into a data attribute. Optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx(1 hunks)src/components/calendar/__tests__/calendar-item-details-sheet.test.tsx(10 hunks)src/components/calendar/calendar-item-details-sheet.tsx(9 hunks)src/components/calendar/enhanced-calendar-view.tsx(6 hunks)src/utils/__tests__/html-sanitizer.test.ts(1 hunks)src/utils/html-sanitizer.ts(1 hunks)src/utils/webview-html.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/utils/webview-html.ts
- src/utils/tests/html-sanitizer.test.ts
- src/components/calendar/calendar-item-details-sheet.tsx
- src/components/calendar/tests/calendar-item-details-sheet.test.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components and favor interfaces for props and state
Avoid using any; use precise types
Use React Navigation for navigation and deep linking following best practices
Handle errors gracefully and provide user feedback
Implement proper offline support (caching, queueing, retries)
Use Expo SecureStore for sensitive data storage
Use zustand for state management
Use react-hook-form for form handling
Use react-query for data fetching and caching
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Ensure support for dark mode and light mode
Handle errors gracefully and provide user feedback
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
src/components/calendar/enhanced-calendar-view.tsxsrc/utils/html-sanitizer.tssrc/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and React hooks instead of class components
Use PascalCase for React component names
Use React.FC for defining functional components with props
Minimize useEffect/useState usage and avoid heavy computations during render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Provide getItemLayout to FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers; define callbacks with useCallback or outside render
Use gluestack-ui for styling where available from components/ui; otherwise, style via StyleSheet.create or styled-components
Ensure responsive design across screen sizes and orientations
Use react-native-fast-image for image handling instead of the default Image where appropriate
Wrap all user-facing text in t() from react-i18next for translations
Support dark mode and light mode in UI components
Use @rnmapbox/maps for maps or navigation features
Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Use conditional rendering with the ternary operator (?:) instead of &&
**/*.tsx: Use functional components and hooks over class components
Ensure components are modular, reusable, and maintainable
Ensure all components are mobile-friendly, responsive, and support both iOS and Android
Use PascalCase for component names
Utilize React.FC for defining functional components with props
Minimize useEffect, useState, and heavy computations inside render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers to prevent re-renders
Ensure responsive design for different screen sizes and orientations
Optimize image handling using rea...
Files:
src/components/calendar/enhanced-calendar-view.tsxsrc/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx
src/**
📄 CodeRabbit inference engine (.cursorrules)
src/**: Organize files by feature, grouping related components, hooks, and styles
Directory and file names should be lowercase and hyphenated (e.g., user-profile)
Files:
src/components/calendar/enhanced-calendar-view.tsxsrc/utils/html-sanitizer.tssrc/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Create Jest tests for all generated components, services, and logic
Ensure tests run without errors and fix failing tests
Files:
src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx
src/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.test.{ts,tsx}: Create and use Jest tests to validate all generated components
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx
🧬 Code graph analysis (2)
src/components/calendar/enhanced-calendar-view.tsx (1)
src/lib/utils.ts (3)
formatLocalDateString(243-248)getTodayLocalString(254-256)isSameDate(258-281)
src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx (1)
src/components/calendar/calendar-item-details-sheet.tsx (1)
CalendarItemDetailsSheet(31-420)
🪛 GitHub Check: CodeQL
src/utils/html-sanitizer.ts
[failure] 16-16: Incomplete URL scheme check
This check does not consider vbscript:.
src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx
[failure] 133-133: Incomplete multi-character sanitization
This string may still contain <script, which may cause an HTML element injection vulnerability.
[failure] 133-133: Bad HTML filtering regexp
This regular expression does not match script end tags like </script\t\n bar>.
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
src/components/calendar/enhanced-calendar-view.tsx (4)
12-12: Good: centralized, timezone-safe date utils adoptionImporting formatLocalDateString, getTodayLocalString, and isSameDate aligns the file with the new locale-aware helpers and prior feedback.
110-113: LGTM: local month boundariesConstructing start/end via local Date constructors and formatting with formatLocalDateString correctly avoids TZ drift.
121-121: LGTM: goToToday uses local helperUsing getTodayLocalString keeps the selection/month sync correct across timezones.
129-131: LGTM: initial load range uses local month boundariesCorrectly mirrors the month-change logic on mount.
| // Mock HTML sanitizer | ||
| jest.mock('@/utils/webview-html', () => ({ | ||
| sanitizeHtmlContent: (html: string) => { | ||
| if (!html) return ''; | ||
| // Strip script tags robustly with non-greedy pattern that handles newlines and nested content | ||
| return html.replace(/<script\s*[^>]*>[\s\S]*?<\/script\s*>/gi, ''); | ||
| }, | ||
| })); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace regex-based sanitizer mock with pass-through to the real util
The test mock reintroduces brittle regex sanitization flagged by CodeQL. For this suite, use the production sanitizer to avoid regex maintenance and get realistic behavior.
Apply:
-// Mock HTML sanitizer
-jest.mock('@/utils/webview-html', () => ({
- sanitizeHtmlContent: (html: string) => {
- if (!html) return '';
- // Strip script tags robustly with non-greedy pattern that handles newlines and nested content
- return html.replace(/<script\s*[^>]*>[\s\S]*?<\/script\s*>/gi, '');
- },
-}));
+// Use real sanitizer to avoid regex pitfalls and align with prod policy
+jest.mock('@/utils/webview-html', () => {
+ const actual = jest.requireActual('@/utils/webview-html');
+ return { ...actual, sanitizeHtmlContent: actual.sanitizeHtmlContent };
+});📝 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.
| // Mock HTML sanitizer | |
| jest.mock('@/utils/webview-html', () => ({ | |
| sanitizeHtmlContent: (html: string) => { | |
| if (!html) return ''; | |
| // Strip script tags robustly with non-greedy pattern that handles newlines and nested content | |
| return html.replace(/<script\s*[^>]*>[\s\S]*?<\/script\s*>/gi, ''); | |
| }, | |
| })); | |
| // Use real sanitizer to avoid regex pitfalls and align with prod policy | |
| jest.mock('@/utils/webview-html', () => { | |
| const actual = jest.requireActual('@/utils/webview-html'); | |
| return { ...actual, sanitizeHtmlContent: actual.sanitizeHtmlContent }; | |
| }); |
🧰 Tools
🪛 GitHub Check: CodeQL
[failure] 133-133: Incomplete multi-character sanitization
This string may still contain <script, which may cause an HTML element injection vulnerability.
[failure] 133-133: Bad HTML filtering regexp
This regular expression does not match script end tags like </script\t\n bar>.
🤖 Prompt for AI Agents
In
src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx
around lines 128-135, the test currently mocks the sanitizer with a brittle
regex; replace that mock so the test uses the real production sanitizer instead.
Remove or replace the jest.mock block and either import the real
sanitizeHtmlContent from '@/utils/webview-html' or implement the mock to forward
to jest.requireActual('@/utils/webview-html') so the real sanitizeHtmlContent
implementation is used in this test; ensure no other tests rely on the regex
mock by scoping/removing the mock only in this file.
| function isSafeScheme(url: string): boolean { | ||
| if (!url) return false; | ||
| const normalizedUrl = url.toLowerCase().trim(); | ||
| return !normalizedUrl.startsWith('javascript:') && !normalizedUrl.startsWith('data:'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Blocklist misses vbscript: (and similar) — tighten scheme guard
Current check only blocks javascript: and data:. CodeQL flagged vbscript:. Tighten the guard to cover vbscript: as well (case-insensitive). This is low-risk and aligns with the rest of the policy.
Apply:
-function isSafeScheme(url: string): boolean {
- if (!url) return false;
- const normalizedUrl = url.toLowerCase().trim();
- return !normalizedUrl.startsWith('javascript:') && !normalizedUrl.startsWith('data:');
-}
+function isSafeScheme(url: string): boolean {
+ if (!url) return false;
+ const normalizedUrl = url.trim().toLowerCase();
+ // Block common scriptable schemes; sanitize-html allowlist enforces final acceptance.
+ return !/^(?:javascript|vbscript|data)\s*:/i.test(normalizedUrl);
+}📝 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.
| function isSafeScheme(url: string): boolean { | |
| if (!url) return false; | |
| const normalizedUrl = url.toLowerCase().trim(); | |
| return !normalizedUrl.startsWith('javascript:') && !normalizedUrl.startsWith('data:'); | |
| } | |
| function isSafeScheme(url: string): boolean { | |
| if (!url) return false; | |
| const normalizedUrl = url.trim().toLowerCase(); | |
| // Block common scriptable schemes; sanitize-html allowlist enforces final acceptance. | |
| return !/^(?:javascript|vbscript|data)\s*:/i.test(normalizedUrl); | |
| } |
🧰 Tools
🪛 GitHub Check: CodeQL
[failure] 16-16: Incomplete URL scheme check
This check does not consider vbscript:.
🤖 Prompt for AI Agents
In src/utils/html-sanitizer.ts around lines 13 to 17, the scheme guard only
blocks "javascript:" and "data:"; update it to also reject "vbscript:"
(case-insensitive) so URLs starting with "vbscript:" are treated as unsafe.
Modify the normalized check to include a startsWith('vbscript:') condition (or a
small RegExp/array of blocked schemes) and return false for those cases, keeping
the existing lowercase/trim normalization and behavior.
| allowedAttributes: { | ||
| a: ['href', 'title'], | ||
| img: ['src', 'alt', 'title', 'width', 'height'], | ||
| span: ['style'], | ||
| div: ['style'], | ||
| p: ['style'], | ||
| table: ['width', 'cellpadding', 'cellspacing'], | ||
| th: ['scope', 'colspan', 'rowspan'], | ||
| td: ['colspan', 'rowspan'], | ||
| '*': ['class'], | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
rel/target are stripped by allowedAttributes — add them to anchor allowlist
transformTags sets target="_blank" and rel="noopener noreferrer", but allowedAttributes.a excludes both, so they are dropped post-transform. Include them explicitly so the intended hardening actually reaches output.
Apply:
allowedAttributes: {
- a: ['href', 'title'],
+ a: ['href', 'title', 'rel', 'target', 'name'],No other changes needed in transformTags; once allowed, the attributes will persist.
Also applies to: 135-157
🤖 Prompt for AI Agents
In src/utils/html-sanitizer.ts around lines 90 to 100 (and also apply the same
change at lines 135 to 157), the allowedAttributes configuration for anchors
omits rel and target so transformTags' target="_blank" and rel="noopener
noreferrer" are stripped; update the allowlist to include 'rel' and 'target' for
the 'a' tag so those attributes are preserved in the final output—no changes to
transformTags required, just add 'rel' and 'target' to the list for a (and
mirror the same addition in the later block at 135-157).
Summary by CodeRabbit
New Features
Bug Fixes / UX
Documentation
Tests
Chores