-
Notifications
You must be signed in to change notification settings - Fork 5
CU-868ex18rd Working on calendar and bug fixes #62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,133 @@ | ||
| # Calendar Card WebView Refactoring | ||
|
|
||
| ## Overview | ||
| This document describes the refactoring of the `CalendarCard` component to use WebView for rendering the `item.Description` field, ensuring consistent styling with other WebView instances throughout the app. | ||
|
|
||
| ## Changes Made | ||
|
|
||
| ### 1. Added WebView Utility (`src/utils/webview-html.ts`) | ||
| Created a reusable utility to generate consistent HTML content for WebView components across the app: | ||
|
|
||
| - **`generateWebViewHtml`**: Generates HTML with proper theming, responsive design, and security considerations | ||
| - **`defaultWebViewProps`**: Provides secure default props for WebView components | ||
|
|
||
| **Features:** | ||
| - Dark/light theme support | ||
| - Responsive design with proper viewport meta tags | ||
| - Security-first approach (disabled JavaScript, restricted origins) | ||
| - Consistent typography and styling | ||
| - Support for various HTML elements (tables, links, code blocks, etc.) | ||
|
|
||
| ### 2. Refactored CalendarCard Component | ||
| Updated `src/components/calendar/calendar-card.tsx` to use WebView for description rendering: | ||
|
|
||
| **Changes:** | ||
| - Added WebView import and nativewind useColorScheme hook | ||
| - Replaced Text component with WebView for description display | ||
| - Added proper styling container (Box with rounded background) | ||
| - Integrated with the WebView utility for consistent HTML generation | ||
| - Maintained compact height (60px) appropriate for card preview | ||
|
|
||
| **Security Features:** | ||
| - Disabled JavaScript execution | ||
| - Restricted to local content only (`about:` origins) | ||
| - Proper content sanitization through HTML generation utility | ||
|
|
||
| ### 3. Updated Tests | ||
| Enhanced `src/components/calendar/__tests__/calendar-card.test.tsx`: | ||
|
|
||
| - Added WebView and utility mocks | ||
| - Updated test assertions to check for WebView component instead of direct text | ||
| - Added specific tests for WebView rendering scenarios | ||
| - Ensured all existing functionality continues to work | ||
|
|
||
| ### 4. Added Utility Tests | ||
| Created `src/utils/__tests__/webview-html.test.ts`: | ||
|
|
||
| - Comprehensive testing of HTML generation utility | ||
| - Theme switching verification | ||
| - Custom styling options testing | ||
| - Security props validation | ||
|
|
||
| ## Benefits | ||
|
|
||
| ### 1. Consistent Styling | ||
| - All WebView instances now use the same HTML template and styling | ||
| - Proper dark/light mode support across all WebViews | ||
| - Consistent typography and responsive behavior | ||
|
|
||
| ### 2. Security | ||
| - Standardized security settings prevent potential XSS vulnerabilities | ||
| - Disabled JavaScript execution unless explicitly needed | ||
| - Restricted origin whitelist for content loading | ||
|
|
||
| ### 3. Maintainability | ||
| - Centralized WebView configuration and styling | ||
| - Easy to update styling across all WebView instances | ||
| - Consistent approach for future WebView implementations | ||
|
|
||
| ### 4. Better HTML Rendering | ||
| - Proper rendering of rich HTML content in calendar descriptions | ||
| - Support for formatting, links, lists, tables, and other HTML elements | ||
| - Better text wrapping and responsive behavior | ||
|
|
||
| ## Usage Examples | ||
|
|
||
| ### Basic WebView Implementation | ||
| ```tsx | ||
| import WebView from 'react-native-webview'; | ||
| import { defaultWebViewProps, generateWebViewHtml } from '@/utils/webview-html'; | ||
|
|
||
| <WebView | ||
| {...defaultWebViewProps} | ||
| source={{ | ||
| html: generateWebViewHtml({ | ||
| content: htmlContent, | ||
| isDarkMode: colorScheme === 'dark', | ||
| }), | ||
| }} | ||
| /> | ||
| ``` | ||
|
|
||
| ### Custom Styling | ||
| ```tsx | ||
| <WebView | ||
| {...defaultWebViewProps} | ||
| source={{ | ||
| html: generateWebViewHtml({ | ||
| content: htmlContent, | ||
| isDarkMode: colorScheme === 'dark', | ||
| fontSize: 16, | ||
| lineHeight: 1.6, | ||
| padding: 12, | ||
| }), | ||
| }} | ||
| /> | ||
| ``` | ||
|
|
||
| ## Testing Strategy | ||
| - Mocked WebView component for unit tests | ||
| - Comprehensive utility function testing | ||
| - Integration testing with existing calendar card functionality | ||
| - Security props validation | ||
|
|
||
| ## Compatibility | ||
| - Works with both iOS and Android platforms | ||
| - Consistent behavior across different screen sizes | ||
| - Proper dark/light mode support | ||
| - Maintains existing calendar card functionality | ||
|
|
||
| ## Future Considerations | ||
| - Could be extended to support additional HTML features if needed | ||
| - Security settings can be adjusted per component if required | ||
| - Styling can be customized through utility parameters | ||
| - Easy to add analytics or performance monitoring if needed | ||
|
|
||
| ## Migration Guide | ||
| For other components using WebView: | ||
|
|
||
| 1. Import the utility: `import { defaultWebViewProps, generateWebViewHtml } from '@/utils/webview-html';` | ||
| 2. Replace custom HTML generation with utility function | ||
| 3. Use `defaultWebViewProps` spread for consistent security settings | ||
| 4. Update tests to mock the utility functions | ||
| 5. Ensure proper theme handling with `useColorScheme` | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,79 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Calendar Item Details Personnel Loading Enhancement | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ## Overview | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Enhanced the calendar item details sheet to include loading states and automatic personnel data fetching to improve the "Created by" name resolution functionality. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ## Changes Made | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ### Component Enhancement (`calendar-item-details-sheet.tsx`) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 1. **Added Loading State Management**: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - Added `isInitializing` state to track personnel fetching | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - Updated `getCreatorName` function to show loading state during data fetching | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - Added `isPersonnelLoading` from personnel store | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 2. **Auto-fetch Personnel Data**: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - Added useEffect to automatically fetch personnel when: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - Sheet is opened (`isOpen` is true) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - Personnel store is empty (`personnel.length === 0`) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - Not already loading (`!isPersonnelLoading`) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - Prevents redundant fetches when data already exists | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 3. **Improved User Experience**: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - Shows "Loading" text while fetching personnel data | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - Graceful fallback to "Unknown User" when data unavailable | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - Maintains existing functionality for all other scenarios | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ### Test Coverage (`calendar-item-details-sheet.test.tsx`) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 1. **Updated Existing Tests**: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - Added `fetchPersonnel` function and `isLoading` property to all mock setups | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - Updated test descriptions to be more specific | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 2. **Added New Test Cases**: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - `shows loading state when fetching personnel`: Verifies loading state display | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - `auto-fetches personnel when store is empty and sheet opens`: Tests automatic data fetching | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - `does not fetch personnel when store already has data`: Ensures no redundant fetches | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - `does not fetch personnel when already loading`: Prevents duplicate fetch calls | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ## Technical Implementation | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ### Loading States | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ```typescript | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Component state | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [isInitializing, setIsInitializing] = useState(false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Personnel store integration | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { personnel, fetchPersonnel, isLoading: isPersonnelLoading } = usePersonnelStore(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Loading detection in getCreatorName | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (isInitializing || isPersonnelLoading) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return t('loading'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ### Auto-fetch Logic | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ```typescript | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (isOpen && personnel.length === 0 && !isPersonnelLoading) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setIsInitializing(true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fetchPersonnel().finally(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setIsInitializing(false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, [isOpen, personnel.length, isPersonnelLoading, fetchPersonnel]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+55
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ## Benefits | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 1. **Better User Experience**: Users see meaningful loading states instead of "Unknown User" while data loads | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 2. **Proactive Data Loading**: Personnel data is automatically fetched when needed | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 3. **Performance Optimization**: Prevents unnecessary API calls when data already exists | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 4. **Robust Error Handling**: Graceful fallbacks for all edge cases | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ## Translation Keys Used | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - `loading`: Shows during personnel data fetching | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - `unknown_user`: Fallback when creator cannot be identified | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ## Test Results | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - ✅ All 40 tests passing in main test suite | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - ✅ TypeScript compilation successful | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - ✅ No breaking changes to existing functionality | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - ✅ Comprehensive coverage of new loading and auto-fetch features | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,107 @@ | ||||||||||||||||||||||||||||||||||||||||
| # Calendar Item Details WebView Implementation | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| ## Overview | ||||||||||||||||||||||||||||||||||||||||
| Refactored the `CalendarItemDetailsSheet` component to use WebView for rendering the item description instead of a plain Text component. This ensures consistent styling and better HTML content rendering across the application. | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| ## Changes Made | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| ### Component Updates | ||||||||||||||||||||||||||||||||||||||||
| - **File**: `src/components/calendar/calendar-item-details-sheet.tsx` | ||||||||||||||||||||||||||||||||||||||||
| - Added WebView import from `react-native-webview` | ||||||||||||||||||||||||||||||||||||||||
| - Added `useColorScheme` hook from `nativewind` for theme detection | ||||||||||||||||||||||||||||||||||||||||
| - Added `StyleSheet` import for WebView styling | ||||||||||||||||||||||||||||||||||||||||
| - Added `Box` component import for WebView container | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| ### Description Rendering | ||||||||||||||||||||||||||||||||||||||||
| - Replaced the simple `Text` component with a `WebView` component wrapped in a `Box` | ||||||||||||||||||||||||||||||||||||||||
| - WebView is only rendered when `item.Description` exists | ||||||||||||||||||||||||||||||||||||||||
| - Consistent styling with other WebView implementations in the app | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| ### WebView Configuration | ||||||||||||||||||||||||||||||||||||||||
| - **HTML Structure**: Full HTML document with DOCTYPE, viewport meta tag, and embedded styles | ||||||||||||||||||||||||||||||||||||||||
| - **Theme Support**: Dynamic color scheme detection for light/dark mode | ||||||||||||||||||||||||||||||||||||||||
| - Light theme: `#1F2937` text on `#F9FAFB` background | ||||||||||||||||||||||||||||||||||||||||
| - Dark theme: `#E5E7EB` text on `#374151` background | ||||||||||||||||||||||||||||||||||||||||
| - **Typography**: Uses system fonts (`system-ui, -apple-system, sans-serif`) | ||||||||||||||||||||||||||||||||||||||||
| - **Responsive**: `max-width: 100%` for all elements | ||||||||||||||||||||||||||||||||||||||||
| - **Performance**: `androidLayerType="software"` for Android optimization | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| ### Styling Consistency | ||||||||||||||||||||||||||||||||||||||||
| The WebView implementation follows the same patterns used in: | ||||||||||||||||||||||||||||||||||||||||
| - `protocol-details-sheet.tsx` | ||||||||||||||||||||||||||||||||||||||||
| - `note-details-sheet.tsx` | ||||||||||||||||||||||||||||||||||||||||
| - `call-card.tsx` | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| ### WebView Properties | ||||||||||||||||||||||||||||||||||||||||
| ```typescript | ||||||||||||||||||||||||||||||||||||||||
| <WebView | ||||||||||||||||||||||||||||||||||||||||
| style={[styles.container, { height: 120 }]} | ||||||||||||||||||||||||||||||||||||||||
| originWhitelist={['*']} | ||||||||||||||||||||||||||||||||||||||||
| scrollEnabled={false} | ||||||||||||||||||||||||||||||||||||||||
| showsVerticalScrollIndicator={false} | ||||||||||||||||||||||||||||||||||||||||
| source={{ html: dynamicHTMLContent }} | ||||||||||||||||||||||||||||||||||||||||
| androidLayerType="software" | ||||||||||||||||||||||||||||||||||||||||
| testID="webview" | ||||||||||||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+36
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| ## Test Updates | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| ### New Test Suite: "WebView Description Rendering" | ||||||||||||||||||||||||||||||||||||||||
| Added comprehensive tests covering: | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| 1. **Conditional Rendering** | ||||||||||||||||||||||||||||||||||||||||
| - Renders WebView when description is provided | ||||||||||||||||||||||||||||||||||||||||
| - Does not render WebView when description is empty | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| 2. **HTML Structure Validation** | ||||||||||||||||||||||||||||||||||||||||
| - Proper DOCTYPE, html, head, meta viewport | ||||||||||||||||||||||||||||||||||||||||
| - Style tag inclusion | ||||||||||||||||||||||||||||||||||||||||
| - Body content with description | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| 3. **Theme Support** | ||||||||||||||||||||||||||||||||||||||||
| - Light theme CSS colors and styling | ||||||||||||||||||||||||||||||||||||||||
| - Dark theme CSS colors and styling | ||||||||||||||||||||||||||||||||||||||||
| - Font family, size, and line-height | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| 4. **Configuration** | ||||||||||||||||||||||||||||||||||||||||
| - WebView props validation (originWhitelist, scrollEnabled, etc.) | ||||||||||||||||||||||||||||||||||||||||
| - Content inclusion verification | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| ### Mock Setup | ||||||||||||||||||||||||||||||||||||||||
| - Added WebView mock in test setup | ||||||||||||||||||||||||||||||||||||||||
| - Added `useColorScheme` mock with theme switching | ||||||||||||||||||||||||||||||||||||||||
| - Added `Box` component mock | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| ### Test Results | ||||||||||||||||||||||||||||||||||||||||
| All 28 tests pass successfully, including the new 7 WebView-specific tests. | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| ## Benefits | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| 1. **Consistent Styling**: Matches other WebView implementations across the app | ||||||||||||||||||||||||||||||||||||||||
| 2. **Better HTML Rendering**: Properly renders HTML content in descriptions | ||||||||||||||||||||||||||||||||||||||||
| 3. **Theme Support**: Automatic light/dark mode adaptation | ||||||||||||||||||||||||||||||||||||||||
| 4. **Mobile Optimized**: Better text rendering and performance | ||||||||||||||||||||||||||||||||||||||||
| 5. **Accessibility**: Improved screen reader support for HTML content | ||||||||||||||||||||||||||||||||||||||||
| 6. **Future-Proof**: Easier to extend with additional HTML features | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| ## Browser Compatibility | ||||||||||||||||||||||||||||||||||||||||
| - **iOS**: Uses WKWebView | ||||||||||||||||||||||||||||||||||||||||
| - **Android**: Uses software layer type for optimal performance | ||||||||||||||||||||||||||||||||||||||||
| - **Web**: Falls back to appropriate web rendering | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| ## Performance Considerations | ||||||||||||||||||||||||||||||||||||||||
| - Height is limited to 120px to prevent excessive scrolling | ||||||||||||||||||||||||||||||||||||||||
| - Scrolling is disabled for controlled layout | ||||||||||||||||||||||||||||||||||||||||
| - Software rendering on Android for better performance | ||||||||||||||||||||||||||||||||||||||||
| - Minimal HTML structure for fast loading | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| ## Accessibility | ||||||||||||||||||||||||||||||||||||||||
| - Maintains existing accessibility features | ||||||||||||||||||||||||||||||||||||||||
| - Supports screen readers through WebView accessibility | ||||||||||||||||||||||||||||||||||||||||
| - Proper semantic HTML structure | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| ## Future Enhancements | ||||||||||||||||||||||||||||||||||||||||
| - Could support rich text editing if needed | ||||||||||||||||||||||||||||||||||||||||
| - Could add support for images or links in descriptions | ||||||||||||||||||||||||||||||||||||||||
| - Could implement print functionality through WebView | ||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).
Add a Security Notes section with an example:
📝 Committable suggestion
🧰 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)