feat(views): create field service groups pdf #3235
Conversation
|
Task linked: CU-86c1aa2w8 [FEAT] Create 'Service groups PDF |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces functionality for exporting field service groups as a PDF document. It enhances the Changes
Possibly related PRs
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (5)src/pages/congregation/field_service_groups/useFieldServiceGroups.tsx (5)
The new imports are logically grouped and provide all necessary dependencies for the PDF export functionality. Also applies to: 3-3, 7-11
The new Recoil state values are correctly retrieved and will provide the necessary data for PDF generation.
The export button is:
The dependency array correctly includes handleExport to ensure the buttons are re-rendered when the export functionality changes.
Let's ensure that the TemplateFieldServiceGroupsDoc component exists and has the correct prop types defined. Also applies to: 36-41 ✅ Verification successfulPDF template component is properly implemented The TemplateFieldServiceGroupsDoc component exists at
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify the PDF template component implementation
# Check if the component file exists and contains the expected prop types
ast-grep --pattern 'interface $Props {
fieldServiceGroups: $_
persons: $_
fullnameOption: $_
congregationName: $_
}'
# Check the component implementation
ast-grep --pattern 'const TemplateFieldServiceGroupsDoc = ({ fieldServiceGroups, persons, fullnameOption, congregationName }: $Props) => {
$$$
}'
Length of output: 276 Script: #!/bin/bash
# Let's first locate the template file
fd "field_service_groups" --type f
# Then examine its content
rg -l "TemplateFieldServiceGroupsDoc" --type ts --type tsx
# Let's also check the actual implementation
rg "TemplateFieldServiceGroupsDoc" -A 5 --type ts --type tsx
Length of output: 372 Script: #!/bin/bash
# Let's check the views directory for the template component
fd "field_service_groups" --type f --full-path ./src/views
# Let's try searching without file type restrictions
rg "TemplateFieldServiceGroupsDoc" -A 5
Length of output: 2532 Script: #!/bin/bash
# Let's check the type definition for the component
cat src/views/congregation/field_service_groups/index.tsx
Length of output: 1857 Script: #!/bin/bash
# Let's check the type definition file
cat src/views/congregation/field_service_groups/shared/index.types.ts
Length of output: 805 Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…Max-Makaluk Signed-off-by: Max Makaluk <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (10)
src/views/components/icons/index.types.ts (1)
1-5: LGTM! Consider adding JSDoc comments.The
IconPropstype definition is well-structured with appropriate optional properties. Consider adding JSDoc comments to document the expected format/units for each prop (e.g., color format: hex/rgb/named).+/** + * Props for icon components + * @property {number} size - Icon size in pixels + * @property {string} color - Icon color (hex, rgb, or named color) + * @property {string} backgroundColor - Background color (hex, rgb, or named color) + */ export type IconProps = { size?: number; color?: string; backgroundColor?: string; };src/views/congregation/field_service_groups/shared/index.types.ts (1)
19-24: Consider adding validation constraints to the types.For
FSGGroupTypeandFSGGroupMemberType, consider adding:
- Optional fields for error states
- Validation for
groupNumber(should be positive)- Type guards for data integrity
Example enhancement:
export type FSGGroupType = { data: FieldServiceGroupType; - groupNumber: number; + groupNumber: Positive<number>; // Using utility type persons: PersonType[]; fullnameOption: FullnameOption; + error?: string; // For error handling };Also applies to: 26-30
src/views/congregation/field_service_groups/shared/FSGTitle.tsx (1)
19-19: Fix typo in style property name.There's a typo in the style property name:
congragationNameshould becongregationName.- <Text style={styles.congragationName}>{congregationName}</Text> + <Text style={styles.congregationName}>{congregationName}</Text>src/views/congregation/field_service_groups/shared/useFSGGroup.tsx (1)
29-34: Consider memoizing the return value.The returned object is recreated on every render. Consider using
useMemofor better performance.+ import { useMemo } from 'react'; - return { - group_overseer, - group_overseer_assistent, - getMemberName, - }; + return useMemo(() => ({ + group_overseer, + group_overseer_assistent, + getMemberName, + }), [group_overseer, group_overseer_assistent]);src/views/congregation/field_service_groups/index.tsx (1)
31-46: Consider adding pagination for large groupsThe current implementation renders all groups on a single A4 page. For congregations with many groups, this might lead to content overflow or reduced readability.
Consider implementing pagination logic to split groups across multiple pages when they exceed a certain threshold.
src/pages/congregation/field_service_groups/useFieldServiceGroups.tsx (1)
64-72: Consider adding loading state for PDF generationThe PDF generation process might take time for large groups. Consider adding a loading state to provide feedback to users.
+ const [isExporting, setIsExporting] = useState(false); + const handleExport = useCallback(async () => { + setIsExporting(true); try { // ... PDF generation code ... + } finally { + setIsExporting(false); + } }, [...]); // In the button JSX: <Button variant="secondary" onClick={handleExport} + disabled={isExporting} startIcon={<IconPrint color="var--accent-main" />} > - {t('tr_export')} + {isExporting ? t('tr_exporting') : t('tr_export')} </Button>src/views/congregation/field_service_groups/shared/FSGGroup.tsx (1)
78-89: Optimize member filtering logicThe current implementation filters out overseers and assistants during rendering. Consider filtering them earlier for better performance.
+ const regularMembers = useMemo( + () => groupMembers.filter( + member => !member.isOverseer && !member.isAssistant + ), + [groupMembers] + ); // In the JSX: <View style={styles.groupMemberList}> - {groupMembers.map( - (member, index) => - !member.isOverseer && - !member.isAssistant && ( - <FSGGroupMember - data={member} - persons={persons} - fullnameOption={fullnameOption} - key={`fsg-group-member-${index}`} - /> - ) + {regularMembers.map((member, index) => ( + <FSGGroupMember + data={member} + persons={persons} + fullnameOption={fullnameOption} + key={`fsg-group-member-${index}`} + /> + ))} </View>src/views/components/icons/IconGroups.tsx (2)
4-8: Add prop types validationConsider adding PropTypes validation for runtime type checking, especially since this component is used in PDF generation where runtime errors might be harder to debug.
+import PropTypes from 'prop-types'; const IconGroups = ({ size = 24, color = '#222222', backgroundColor = '#FFFFFF', }: IconProps) => ( // ... component implementation ... ); + +IconGroups.propTypes = { + size: PropTypes.number, + color: PropTypes.string, + backgroundColor: PropTypes.string, +};
9-17: Add accessibility attributes to SVGWhile this is used for PDF generation, adding accessibility attributes is a good practice and might be useful if the component is reused in the web interface.
- <Svg width={size} height={size} viewBox="0 0 24 24" fill={backgroundColor}> + <Svg + width={size} + height={size} + viewBox="0 0 24 24" + fill={backgroundColor} + role="img" + aria-label="Groups icon" + >src/utils/common.ts (1)
214-218: Consider caching CSS property valuesSince this function is used in PDF generation, consider caching the values to improve performance when generating multiple PDFs.
+const cssPropertyCache = new Map<string, string>(); + export const getCSSPropertyValue = (key: string): string => { + if (cssPropertyCache.has(key)) { + return cssPropertyCache.get(key)!; + } + if (typeof window === 'undefined' || !document?.documentElement) { console.warn(`getCSSPropertyValue: document not available for key ${key}`); return ''; } - return getComputedStyle(document.documentElement) + const value = getComputedStyle(document.documentElement) .getPropertyValue(key) .trim(); + + cssPropertyCache.set(key, value); + return value; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/pages/congregation/field_service_groups/useFieldServiceGroups.tsx(4 hunks)src/utils/common.ts(1 hunks)src/views/components/icons/IconGroups.tsx(1 hunks)src/views/components/icons/index.ts(1 hunks)src/views/components/icons/index.types.ts(1 hunks)src/views/congregation/field_service_groups/index.tsx(1 hunks)src/views/congregation/field_service_groups/shared/FSGGroup.tsx(1 hunks)src/views/congregation/field_service_groups/shared/FSGGroupMember.tsx(1 hunks)src/views/congregation/field_service_groups/shared/FSGTitle.tsx(1 hunks)src/views/congregation/field_service_groups/shared/index.styles.ts(1 hunks)src/views/congregation/field_service_groups/shared/index.types.ts(1 hunks)src/views/congregation/field_service_groups/shared/useFSGGroup.tsx(1 hunks)src/views/congregation/field_service_groups/shared/useFSGGroupMember.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/views/components/icons/index.ts
- src/views/congregation/field_service_groups/shared/index.styles.ts
🔇 Additional comments (5)
src/views/congregation/field_service_groups/shared/useFSGGroupMember.tsx (1)
10-12: Add error handling for person lookup.
The person lookup should handle potential data inconsistencies. Consider adding error boundaries or logging.
const person = useMemo(() => {
- return persons.find((record) => record.person_uid === data.person_uid);
+ try {
+ return persons.find((record) => record.person_uid === data.person_uid);
+ } catch (error) {
+ console.error('Error finding person:', error);
+ return undefined;
+ }
}, [persons, data]);src/views/congregation/field_service_groups/shared/FSGGroupMember.tsx (1)
1-4: Verify styles import path and add prop types validation.
Ensure the styles import path is correct and consider adding prop types validation.
✅ Verification successful
Let me check the actual content of the styles file to verify the import path and styles definition.
Let me check the component's props and usage to verify if prop types validation is needed.
Let me check the type definition to complete the verification.
The styles import path is correct and prop types are properly defined.
The verification shows that:
- The styles import path
./index.stylesis correct and the file contains the requiredgroupMemberstyle - Props validation is implemented using TypeScript with
FSGGroupMemberTypeinterface, which properly defines the component's props structure - The component correctly uses the imported types and styles
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if styles file exists and contains required style
fd -e ts -e tsx "index.styles" | xargs rg "groupMember"
Length of output: 225
Script:
#!/bin/bash
# Check the content of the styles file
cat src/views/congregation/field_service_groups/shared/index.styles.ts
Length of output: 2794
Script:
#!/bin/bash
# Check the FSGGroupMember component implementation and its props type
cat src/views/congregation/field_service_groups/shared/FSGGroupMember.tsx
Length of output: 470
Script:
#!/bin/bash
# Check the type definition for FSGGroupMemberType
cat src/views/congregation/field_service_groups/shared/index.types.ts
Length of output: 805
src/views/congregation/field_service_groups/shared/index.types.ts (1)
8-13: LGTM! Well-structured type definition for the PDF document.
The FieldServiceGroupsDocType includes all necessary data for PDF generation and maintains good type safety.
src/views/congregation/field_service_groups/shared/FSGTitle.tsx (1)
13-16: Verify icon size compatibility with PDF renderer.
The IconGroups component is being used within a PDF context. Ensure that:
- The SVG icon renders correctly in PDF output
- The size (18) is appropriate for PDF resolution
✅ Verification successful
Icon is properly implemented for PDF rendering
The IconGroups component is correctly implemented using @react-pdf/renderer's SVG components and is fully compatible with PDF output. The size prop (18) being used in FSGTitle is appropriate as:
- The component properly scales using the viewBox attribute
- The implementation uses PDF-compatible primitives (Svg, G, Path, Rect)
- The size prop is correctly passed and applied to the width/height attributes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if IconGroups is PDF-compatible
ast-grep --pattern 'const IconGroups = ({ size, color }: IconProps) => {
$$$
}'
Length of output: 83
Script:
#!/bin/bash
# Let's try to find the IconGroups component definition and its usage in PDF context
rg -l "IconGroups" --type tsx --type ts
# Then check for PDF-related imports and components
rg "@react-pdf" --type tsx --type ts -A 3
Length of output: 137
Script:
#!/bin/bash
# Search for IconGroups component without file type restrictions
rg -l "IconGroups"
# Check for PDF-related imports
rg "@react-pdf"
# Look for SVG-related code in IconGroups
rg "IconGroups.*svg" -A 5
Length of output: 15911
Script:
#!/bin/bash
# Let's check the IconGroups implementation in the component file
cat src/views/components/icons/IconGroups.tsx
Length of output: 3537
src/utils/common.ts (1)
214-218:
Add type safety and error handling to getCSSPropertyValue
The function needs improvements in type safety and error handling, especially since it's used in PDF generation where failures might be silent.
-export const getCSSPropertyValue = (key: string) => {
+export const getCSSPropertyValue = (key: string): string => {
+ if (typeof window === 'undefined' || !document?.documentElement) {
+ console.warn(`getCSSPropertyValue: document not available for key ${key}`);
+ return '';
+ }
+
return getComputedStyle(document.documentElement)
.getPropertyValue(key)
.trim();
};Also, consider adding a test to verify the CSS property exists:
✅ Verification successful
Let me try a different approach to verify the CSS properties usage and definitions.
Add type safety and error handling to getCSSPropertyValue
The suggested improvements for type safety and error handling are valid. From the verification:
- The function is used in a styling context where CSS properties are defined in global.css
- All CSS properties used (
--accent-200,--accent-dark,--grey-200,--grey-300,--radius-xs) are properly defined in the global styles - The SSR check is important since document might be undefined during server-side rendering
The suggested changes will improve reliability:
- Return type annotation adds type safety
- SSR environment check prevents runtime errors
- Empty string fallback maintains graceful degradation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find usages of getCSSPropertyValue and corresponding CSS custom properties
# Find CSS custom properties used with getCSSPropertyValue
echo "Searching for getCSSPropertyValue usages..."
rg "getCSSPropertyValue\(['\"](.+?)['\"]" -o | cut -d'"' -f2 | sort -u > /tmp/css_props.txt
# Find CSS custom property definitions
echo "Searching for CSS custom property definitions..."
fd -e css -e scss | xargs rg "^[^/]*--" -o | sort -u
# Compare to ensure all used properties are defined
echo "Checking if all used properties are defined..."
while read -r prop; do
rg "^[^/]*--${prop}" $(fd -e css -e scss) || echo "Warning: CSS property '--${prop}' might be undefined"
done < /tmp/css_props.txt
Length of output: 6436
Script:
#!/bin/bash
# Find actual usages of getCSSPropertyValue
echo "Searching for getCSSPropertyValue usages..."
rg "getCSSPropertyValue\(['\"]--[^'\"]+['\"]" -l
echo "\nChecking the content of files using getCSSPropertyValue..."
rg "getCSSPropertyValue\(['\"]--[^'\"]+['\"]" -B1 -A1
echo "\nListing CSS custom properties definitions..."
rg "^\s*--[^:]+:" $(fd -e css -e scss)
Length of output: 36759
src/views/congregation/field_service_groups/shared/useFSGGroupMember.tsx
Show resolved
Hide resolved
…center | change group name font size
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/pages/congregation/field_service_groups/useFieldServiceGroups.tsx (1)
34-47:⚠️ Potential issueAdd error handling for PDF generation.
The PDF generation lacks error handling, which could lead to silent failures.
Additionally, consider wrapping the export functionality in a loading state to provide user feedback during PDF generation:
const [reorderOpen, setReorderOpen] = useState(false); + const [isExporting, setIsExporting] = useState(false); const handleExport = useCallback(async () => { + setIsExporting(true); try { const blob = await pdf( <TemplateFieldServiceGroupsDoc fieldServiceGroups={groups} persons={persons} fullnameOption={fullnameOption} congregationName={congregationName} /> ).toBlob(); const filename = `Field-service-groups.pdf`; saveAs(blob, filename); } catch (error) { console.error('Failed to generate or save PDF:', error); // Show user-friendly error message + } finally { + setIsExporting(false); } }, [groups, persons, fullnameOption, congregationName]);
🧹 Nitpick comments (2)
src/views/congregation/field_service_groups/shared/FSGGroup.tsx (2)
23-30: Add type safety to getGroupTitle functionWhile the logic is correct, consider adding type safety for the groupNumber parameter in the translation call.
const getGroupTitle = () => { + const safeGroupNumber = groupNumber?.toString() ?? ''; if (groupName !== '') { return ( - t('tr_groupNumber', { groupNumber: groupNumber }) + ` – ${groupName}` + t('tr_groupNumber', { groupNumber: safeGroupNumber }) + ` – ${groupName}` ); } - return t('tr_groupNumber', { groupNumber: groupNumber }); + return t('tr_groupNumber', { groupNumber: safeGroupNumber }); };
33-92: Optimize style compositionThe component has multiple instances of style spreading and color application. Consider creating a custom hook or utility function to manage these styles consistently.
const useGroupStyles = (groupNumber: number) => { const safeGroupNumber = Math.min(Math.max(1, groupNumber), 10); const groupColor = getCSSPropertyValue(`--group-${safeGroupNumber}`); return { titleContainer: { ...styles.groupTitleContainer, backgroundColor: groupColor, }, count: { ...styles.membersCount, color: groupColor, }, overseerText: { ...styles.groupOverseerText, color: groupColor, }, assistantText: { ...styles.groupOverseerAssistentText, color: groupColor, }, }; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/pages/congregation/field_service_groups/useFieldServiceGroups.tsx(4 hunks)src/views/congregation/field_service_groups/index.tsx(1 hunks)src/views/congregation/field_service_groups/shared/FSGGroup.tsx(1 hunks)src/views/congregation/field_service_groups/shared/index.styles.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/views/congregation/field_service_groups/index.tsx
- src/views/congregation/field_service_groups/shared/index.styles.ts
🔇 Additional comments (5)
src/views/congregation/field_service_groups/shared/FSGGroup.tsx (2)
1-8: LGTM! Dependencies are properly structured
The imports are well-organized and include all necessary dependencies for PDF generation and component functionality.
35-38: Validate group number for CSS property access
Using unconstrained group numbers with getCSSPropertyValue could lead to CSS injection if the group number is user-controlled.
Also applies to: 43-46, 58-61, 66-69
src/pages/congregation/field_service_groups/useFieldServiceGroups.tsx (3)
1-1: LGTM: Imports are well-organized and necessary.
The new imports are properly structured and essential for the PDF export functionality.
Also applies to: 3-3, 7-11
83-83: LGTM: Dependencies array is complete.
The useMemo dependencies array correctly includes all required dependencies including the new handleExport callback.
19-21: Verify state initialization.
The new Recoil state values look good, but let's verify their initialization.
src/pages/congregation/field_service_groups/useFieldServiceGroups.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Max Makaluk <[email protected]>
|
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
Checklist: