-
Notifications
You must be signed in to change notification settings - Fork 198
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
fix: fixed document cropping & layout fixes #2866
Conversation
|
WalkthroughThe pull request introduces several modifications primarily focused on routing, document handling, and component styling within the backoffice application. Key changes include the restructuring of routing configurations to enhance modularity and error handling, the addition of new props for components to allow for more flexible styling, and updates to the logic within hooks and components to support new functionalities such as OCR processing. Overall, these changes aim to improve the clarity and functionality of the application. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Warning Rate limit exceeded@chesterkmr has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 34 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (8)
apps/backoffice-v2/src/pages/Document/Document.page.tsx (1)
4-6
: Add JSDoc documentation for the interface.While the interface is well-structured, it would benefit from documentation explaining the purpose and impact of the
isPage
prop, especially since it affects document layout behavior.+/** + * Props for the Document component + * @property {boolean} [isPage] - Controls the document layout behavior + */ interface IDocumentProps { isPage?: boolean; }apps/backoffice-v2/src/common/components/molecules/ImageEditor/ImageEditor.tsx (2)
51-52
: Consider simplifying the className constructionWhile the current implementation works correctly, we could make it more concise.
- className={ctw('h-full w-full overflow-hidden', { - 'flex flex-row': isPdf(image), - })} + className={ctw('h-full w-full overflow-hidden', isPdf(image) && 'flex flex-row')}
55-63
: Consider performance optimization for rotationsWhile the rotation implementation is correct, consider using CSS transform with hardware acceleration for better performance.
className={ctw('flex h-full', { - 'rotate-90': imageRotation === 90, - 'rotate-180': imageRotation === 180, - 'rotate-[270deg]': imageRotation === 270, + 'transform rotate-90 will-change-transform': imageRotation === 90, + 'transform rotate-180 will-change-transform': imageRotation === 180, + 'transform rotate-[270deg] will-change-transform': imageRotation === 270, })}apps/backoffice-v2/src/common/components/organisms/ImageViewer/ImageViewer.SelectedImage.tsx (1)
Line range hint
36-43
: Consider enhancing PDF viewer robustness.The iframe implementation could benefit from additional error handling and accessibility improvements:
<iframe src={selectedImage?.imageUrl + '#toolbar=0&navpanes=0'} ref={ref} + title="PDF Document Viewer" + onError={onError} + sandbox="allow-scripts allow-same-origin" className={ctw(className, `d-full mx-auto`, { 'h-[600px] w-[441px]': isPlaceholder, })} + aria-label="PDF document viewer with disabled navigation" {...props} />These changes:
- Add proper title and ARIA label for accessibility
- Include error handling consistent with image viewing
- Add sandbox attribute for security
apps/backoffice-v2/src/pages/Entity/components/Case/hooks/useDocumentsToolbarLogic/useDocumentsToolbarLogic.tsx (1)
Line range hint
47-57
: Consider improving the document opening mechanismThe current implementation has several potential issues:
- The hardcoded 100ms timeout might be insufficient for slower systems
- Missing error handling for BroadcastChannel communication
- The
isDocumentTabOpen.current
state might persist incorrectly between hook instancesConsider these improvements:
useLayoutEffect(() => { if (hideOpenExternalButton) { return; } - broadcastChannel.addEventListener('message', handler); + try { + broadcastChannel.addEventListener('message', handler); + } catch (error) { + console.error('Failed to setup broadcast channel:', error); + } return () => { - broadcastChannel.removeEventListener('message', handler); + try { + broadcastChannel.removeEventListener('message', handler); + isDocumentTabOpen.current = false; // Reset the state on cleanup + } catch (error) { + console.error('Failed to cleanup broadcast channel:', error); + } }; }, [broadcastChannel, handler, hideOpenExternalButton]);Also, consider making the timeout configurable:
+const DOCUMENT_OPEN_TIMEOUT = 500; // Increased and made configurable + const onOpenInNewTabClick = useCallback(() => { broadcastChannel.postMessage({ type: CommunicationChannelEvent.OPEN_DOCUMENT_IN_NEW_TAB, data: { entityId: workflow?.id, documentId: imageId }, }); setTimeout(() => { if (isDocumentTabOpen.current) { isDocumentTabOpen.current = false; return; } onOpenDocumentInNewTab(imageId); - }, 100); + }, DOCUMENT_OPEN_TIMEOUT); }, [broadcastChannel, imageId, onOpenDocumentInNewTab, workflow?.id]);apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.tsx (3)
3-3
: Add documentation for the newisPage
propThe component's JSDoc should be updated to include documentation for the new
isPage
prop that affects the layout behavior.Add the following to the JSDoc props section:
* @param props.documents - An array of objects containing the document's image URL and caption to pass into {@link ImageViewer.Item}. * @param props.isLoading - Whether the documents are still loading. * @param props.hideOpenExternalButton - Whether to hide the open external button. +* @param props.isPage - Whether the component is rendered in page context, affecting layout alignment.
Also applies to: 33-33
Line range hint
95-97
: Clean up commented-out propsThere are several commented-out props being passed to
DocumentsToolbar
. These should either be removed or uncommented if they're still needed.- // isRotatedOrTransformed={isRotatedOrTransformed} - // isCropping={isCropping} - // onCancelCrop={onCancelCrop}
Line range hint
66-93
: Consider splitting the image editing logic into a separate componentThe component handles multiple concerns (cropping, rotation, downloading) which increases complexity. Consider extracting the image editing logic into a separate component to improve maintainability and readability.
This would help separate the layout concerns from the image manipulation logic. For example:
// ImageEditorWrapper.tsx export const ImageEditorWrapper: FC<{ shouldDownload: boolean; selectedImage: ImageType; // ... other props }> = ({ shouldDownload, selectedImage, ...props }) => { if (shouldDownload) { return <DownloadFileView fileName={selectedImage?.fileName} />; } return <ImageEditorView {...props} />; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
apps/backoffice-v2/src/Router/Router.tsx
(2 hunks)apps/backoffice-v2/src/common/components/molecules/ImageEditor/ImageEditor.tsx
(2 hunks)apps/backoffice-v2/src/common/components/organisms/ImageViewer/ImageViewer.SelectedImage.tsx
(2 hunks)apps/backoffice-v2/src/pages/Document/Document.page.tsx
(1 hunks)apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.module.css
(1 hunks)apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.tsx
(3 hunks)apps/backoffice-v2/src/pages/Entity/components/Case/hooks/useDocumentsToolbarLogic/useDocumentsToolbarLogic.tsx
(1 hunks)apps/backoffice-v2/src/pages/Entity/components/Case/interfaces.ts
(1 hunks)services/workflows-service/prisma/data-migrations
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.module.css
- services/workflows-service/prisma/data-migrations
🔇 Additional comments (10)
apps/backoffice-v2/src/pages/Document/Document.page.tsx (1)
8-8
: Verify prop handling in Case.Documents component.
The component correctly passes the isPage
prop, but we should verify its proper handling in the Case.Documents component to ensure layout changes work as intended.
Also applies to: 15-15
✅ Verification successful
The isPage
prop is properly handled in Case.Documents component
The verification shows that the isPage
prop is correctly implemented in the Documents component and affects the layout as intended. Specifically, it controls the justification of content through CSS classes:
- When
isPage
is true, content is center-justified (justify-center
) - When
isPage
is false, content is end-justified (justify-end
)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation of isPage prop in Case.Documents
# Check how isPage is used in Case.Documents
ast-grep --pattern 'interface IDocumentsProps {
$$$
isPage?: boolean;
$$$
}'
# Look for conditional rendering based on isPage
rg -A 5 'isPage[^a-zA-Z]' --type tsx
Length of output: 1516
Script:
#!/bin/bash
# Let's try a different approach to find the usage of isPage prop
# Search for Documents component implementation and its usage of isPage
ast-grep --pattern 'const Documents = ({ $$$, isPage $$$}) => {
$$$
}'
# Search for any file containing 'isPage' with proper file extension
rg -t ts -t tsx 'isPage' -A 5
# Find the Documents component implementation
fd "Documents\.(tsx|ts)" --exec cat {}
Length of output: 5688
apps/backoffice-v2/src/pages/Entity/components/Case/interfaces.ts (1)
54-54
: Add JSDoc documentation for the new property.
The addition of the optional isPage
property is well-structured and maintains backward compatibility. However, its purpose could be better documented.
Add JSDoc documentation to clarify the property's purpose:
export interface IDocumentsProps {
documents: Array<{
id: string;
imageUrl: string;
fileType: string;
fileName: string;
title: string;
}>;
onOcrPressed: () => void;
isLoading?: boolean;
isLoadingOCR?: boolean;
isDocumentEditable?: boolean;
hideOpenExternalButton?: boolean;
+ /** Indicates whether the documents are being displayed on a dedicated page view,
+ * affecting the layout and navigation behavior */
isPage?: boolean;
}
Let's verify how this property is being used across components:
✅ Verification successful
Add JSDoc documentation for the new property and consider its layout impact.
The isPage
property is used to control the layout justification of documents in the viewer:
- When
isPage
is true, documents are centered (justify-center
) - When
isPage
is false, documents are aligned to the end (justify-end
)
export interface IDocumentsProps {
documents: Array<{
id: string;
imageUrl: string;
fileType: string;
fileName: string;
title: string;
}>;
onOcrPressed: () => void;
isLoading?: boolean;
isLoadingOCR?: boolean;
isDocumentEditable?: boolean;
hideOpenExternalButton?: boolean;
+ /** Controls the document layout justification:
+ * - When true, documents are centered (justify-center)
+ * - When false, documents are aligned to the end (justify-end)
+ */
isPage?: boolean;
}
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check usage of isPage prop in Document components
# Expected: Find implementations showing how isPage affects layout
# Search for Document components using isPage
ast-grep --pattern 'const $_ = ({ isPage }: $_) => {
$$$
}'
# Search for CSS classes or styles that might be conditionally applied based on isPage
rg -A 3 'isPage.*\?' apps/backoffice-v2/src/pages/Entity/components/Case/
Length of output: 1268
Script:
#!/bin/bash
# Let's check the implementation of Case.Documents to understand how isPage is used
ast-grep --pattern 'const Documents = ({ isPage, $$ }: IDocumentsProps) => {
$$$
}'
# Also search for any conditional rendering or styling based on isPage
rg -B 2 -A 5 'isPage' apps/backoffice-v2/src/pages/Entity/components/Case/Documents
Length of output: 273
Script:
#!/bin/bash
# Let's find the Documents component file first
fd Documents.tsx -t f apps/backoffice-v2/src/pages/Entity/components/Case/
# Then search for any file containing Documents component implementation
rg -B 2 -A 5 'const Documents.*=.*IDocumentsProps' apps/backoffice-v2/
# Also check for any file importing and using isPage prop
rg -l "isPage.*boolean" apps/backoffice-v2/
Length of output: 388
Script:
#!/bin/bash
# Let's check the implementation in Case.Documents.tsx
cat apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.tsx
# Also check Document.page.tsx to understand the complete usage context
cat apps/backoffice-v2/src/pages/Document/Document.page.tsx
Length of output: 4962
apps/backoffice-v2/src/common/components/molecules/ImageEditor/ImageEditor.tsx (2)
44-44
: LGTM: Appropriate display mode handling for PDFs vs images
The conditional display mode (flex for PDFs, block for images) provides proper layout handling for different document types.
31-31
: LGTM: Improved container dimensions and overflow handling
The increased dimensions (600px) and overflow handling will help documents fit better in the viewing area while preventing unwanted content spillover.
Let's verify there are no other hardcoded dimensions that need adjustment:
Also applies to: 37-37, 39-39
✅ Verification successful
Based on the search results, I can now generate the final response:
LGTM: The dimensions are consistent with other image-related components
The increased dimensions (600px) align well with other image-related components in the codebase:
- ImageViewer uses similar dimensions (
h-[600px] w-[441px]
) - MapCell component also uses 600px (
h-[600px] w-[600px]
) - No conflicting or inconsistent dimensions found that would require adjustment
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining px dimensions that might need updating
rg -g '*.{tsx,css,scss}' '\d+px' apps/backoffice-v2/src/
Length of output: 43191
apps/backoffice-v2/src/common/components/organisms/ImageViewer/ImageViewer.SelectedImage.tsx (1)
37-37
: LGTM! Consider additional PDF viewer parameters for better control.
The addition of #toolbar=0&navpanes=0
parameters aligns with the PR objective to disable navigation. However, consider adding more parameters for better control and compatibility:
- src={selectedImage?.imageUrl + '#toolbar=0&navpanes=0'}
+ src={selectedImage?.imageUrl + '#toolbar=0&navpanes=0&scrollbar=0&statusbar=0'}
Let's verify browser compatibility of these PDF parameters:
apps/backoffice-v2/src/pages/Entity/components/Case/hooks/useDocumentsToolbarLogic/useDocumentsToolbarLogic.tsx (2)
4-4
: LGTM: Import changes are appropriate
The addition of useFilterId
hook and reordering of imports maintain code organization while adding necessary functionality.
Also applies to: 6-6
Line range hint 20-22
: Verify the impact of filterId on workflow queries
The addition of filterId
to the workflow query could affect document filtering behavior. Please ensure this change:
- Maintains backward compatibility
- Correctly filters documents as expected
✅ Verification successful
Let me gather more specific information about the workflow query usage.
Based on the search results, I can now generate the final response:
The addition of filterId to workflow queries maintains consistency across the codebase
The filterId
parameter is consistently used across all workflow-related queries and mutations in the codebase. The useFilterId
hook is properly integrated in:
- All workflow query implementations
- Related mutation hooks (approve, reject, revision)
- Parent-child workflow relationships
- Document handling components
This change aligns with the existing pattern and maintains backward compatibility as the hook handles null cases with proper fallbacks.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other usages of useWorkflowByIdQuery to ensure consistent usage of filterId
ast-grep --pattern 'useWorkflowByIdQuery({ $$ })'
# Check for any existing filter-related test files
fd -e test.tsx -e spec.tsx -e test.ts -e spec.ts . | grep -i "filter\|workflow"
Length of output: 4079
Script:
#!/bin/bash
# Search for useWorkflowByIdQuery usage with context
rg "useWorkflowByIdQuery" -A 3 -B 3
# Search for the useFilterId hook definition and usage
rg "useFilterId" -A 3 -B 3
# Look for the workflow query type definition
ast-grep --pattern 'type $_ = {
workflowId: $_
$$$
}'
Length of output: 81763
apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.tsx (1)
61-63
: LGTM: Layout changes align with PR objectives
The conditional classes implementation effectively handles different layout requirements based on the context (isPage
), which aligns with the PR's goal of fixing layout issues.
apps/backoffice-v2/src/Router/Router.tsx (2)
1-2
: Verify error handling component usage consistency
Both RouteError
and RouteErrorWithProviders
are imported, but the codebase should consistently use one approach for error handling.
Let's verify the usage of these components:
✅ Verification successful
Based on the search results, I can now generate the final response:
Error handling components are used consistently and correctly
The codebase shows a clear and intentional pattern where:
RouteErrorWithProviders
is used only once at the root level route to ensure error boundaries have access to providersRouteError
is used consistently for all child routes where providers are already available through parent components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check usage patterns of RouteError vs RouteErrorWithProviders
echo "Checking RouteError usage:"
rg "RouteError" -A 2
echo "\nChecking RouteErrorWithProviders usage:"
rg "RouteErrorWithProviders" -A 2
Length of output: 9664
Line range hint 179-183
: Verify the impact of isPage
prop on document rendering
The addition of isPage
prop to the Document component appears to be related to the document rendering improvements mentioned in the PR objectives. However, we should verify its implementation and document its purpose.
Let's check the Document component implementation:
Consider adding a comment or documentation explaining the purpose of the isPage
prop and its effect on document rendering and navigation.
✅ Verification successful
Let me search for the Document component implementation and its usage of isPage prop.
Let me check how the isPage prop is used in the Case.Documents component.
Let me check the Case.Documents component implementation to see how isPage affects the rendering.
The isPage
prop is properly implemented and serves a clear purpose
The isPage
prop is used to control the layout justification of the document viewer:
- When
isPage
is true, it centers the document viewer (justify-center) - When
isPage
is false, it aligns the document viewer to the end (justify-end)
This aligns with the PR objectives as it provides proper layout control when viewing documents in different contexts (as a page vs as a component).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find Document component implementation and usage of isPage prop
ast-grep --pattern 'interface $interface {
$$$
isPage$_
$$$
}'
echo "\nChecking Document component implementation:"
rg -A 10 "export.*Document.*="
Length of output: 83183
Script:
#!/bin/bash
# Find Document component implementation and usage of isPage prop
rg -A 5 "interface.*Document.*Props" apps/backoffice-v2/
rg -A 5 "type.*Document.*Props" apps/backoffice-v2/
# Find Document component implementation
rg -A 10 "Document.*=.*\{.*isPage" apps/backoffice-v2/
Length of output: 4133
Script:
#!/bin/bash
# Find Case.Documents implementation and usage of isPage prop
rg -A 15 "Documents.*=.*\{.*isPage" apps/backoffice-v2/src/pages/Entity/components/Case/
# Find where isPage affects behavior
rg -B 5 -A 10 "isPage\?" apps/backoffice-v2/src/pages/Entity/components/Case/
Length of output: 1326
Script:
#!/bin/bash
# Find Case.Documents component implementation
rg -A 20 "export const Documents.*=.*\{" apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.tsx
# Find any conditional logic using isPage
rg -C 10 "isPage" apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.tsx
Length of output: 1673
apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.tsx
Outdated
Show resolved
Hide resolved
apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.module.css
Outdated
Show resolved
Hide resolved
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.Toolbar.tsx (1)
Line range hint
56-67
: Consider enhancing PDF-specific controlsWhile the code correctly identifies PDFs using
isPdf(image)
, consider adding PDF-specific controls to better support the PR's objective of handling PDF documents in the backoffice.You could enhance the PDF handling by:
- Adding a page navigation component for PDFs
- Including zoom controls specific to PDF viewing
- Adding a fullscreen toggle for better document viewing
Example enhancement:
{!isPdf(image) && !isLoading && ( <> <button type={`button`} className={ctw( `btn btn-circle btn-ghost btn-sm bg-base-300/70 text-[0.688rem] focus:outline-primary`, )} onClick={onRotateDocument} disabled={shouldDownload} > <FileText className={`rotate-90 p-0.5`} /> </button> </> + )} + {isPdf(image) && !isLoading && ( + <button + type={`button`} + className={ctw( + `btn btn-circle btn-ghost btn-sm bg-base-300/70 text-[0.688rem] focus:outline-primary`, + )} + onClick={onToggleFullscreen} + disabled={shouldDownload} + > + <Maximize2 className={`p-0.5`} /> + </button> )}apps/backoffice-v2/src/lib/blocks/hooks/useDocumentBlocks/useDocumentBlocks.tsx (2)
Line range hint
70-80
: Consider enhancing user feedback for OCR operationsWhile the OCR integration is technically sound, consider adding user feedback for success/failure states to improve the user experience.
Consider adding toast notifications:
const { mutate: mutateOCRDocument, isLoading: isLoadingOCRDocument, data: ocrResult, } = useDocumentOcr({ workflowId: workflow?.id, + onSuccess: () => { + toast.success('Document processed successfully'); + }, + onError: (error) => { + toast.error(`Failed to process document: ${error.message}`); + }, });
516-519
: Consider refactoring grid layout for better maintainabilityWhile the responsive grid implementation is solid, it could be simplified using CSS custom properties.
Consider refactoring to:
- contentClassName: - 'grid grid-cols-[1fr_minmax(240px,280px)] md:grid-cols-[1fr_minmax(240px,360px)] lg:grid-cols-[1fr_minmax(240px,441px)] 2xl:grid-cols-[1fr_minmax(240px,600px)] grid-rows-[auto_1fr] gap-4 [&>*:first-child]:col-span-2', + contentClassName: ` + grid + grid-rows-[auto_1fr] + gap-4 + [&>*:first-child]:col-span-2 + grid-cols-[1fr_minmax(240px,var(--sidebar-width,280px))] + md:grid-cols-[1fr_minmax(240px,var(--sidebar-width-md,360px))] + lg:grid-cols-[1fr_minmax(240px,var(--sidebar-width-lg,441px))] + 2xl:grid-cols-[1fr_minmax(240px,var(--sidebar-width-2xl,600px))]`.replace(/\s+/g, ' ').trim(),This change:
- Makes sidebar widths configurable via CSS variables
- Improves readability with template literal
- Maintains the same responsive behavior
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
apps/backoffice-v2/src/common/components/molecules/ImageEditor/ImageEditor.tsx
(2 hunks)apps/backoffice-v2/src/common/components/organisms/ImageViewer/ImageViewer.SelectedImage.tsx
(3 hunks)apps/backoffice-v2/src/lib/blocks/components/BlockCell/BlockCell.tsx
(2 hunks)apps/backoffice-v2/src/lib/blocks/create-blocks-typed/create-blocks-typed.ts
(1 hunks)apps/backoffice-v2/src/lib/blocks/hooks/useDocumentBlocks/useDocumentBlocks.tsx
(3 hunks)apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.Toolbar.tsx
(2 hunks)apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.tsx
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/backoffice-v2/src/common/components/molecules/ImageEditor/ImageEditor.tsx
- apps/backoffice-v2/src/common/components/organisms/ImageViewer/ImageViewer.SelectedImage.tsx
- apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.tsx
🔇 Additional comments (8)
apps/backoffice-v2/src/lib/blocks/components/BlockCell/BlockCell.tsx (2)
12-12
: LGTM: Well-structured interface extension
The addition of the optional contentClassName
property maintains backward compatibility while enabling more granular styling control, which aligns with the PR's layout improvement objectives.
24-30
: LGTM: Improved layout handling with flexible styling
The className composition is well-structured and the conditional grid layout improves document presentation.
Let's verify the 'multiDocuments' type is consistently used across the codebase:
✅ Verification successful
'multiDocuments' type is consistently used across the codebase
The search results confirm that 'multiDocuments' is a well-defined type in the codebase and is consistently used:
- It's properly defined as
TMultiDocumentsCell
type - It's consistently used in block cell configurations
- The grid-cols-2 layout is appropriately applied when this type is present
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of 'multiDocuments' type in the codebase
# Expected: Find all occurrences to ensure type consistency
# Search for multiDocuments string in TypeScript/JavaScript files
rg -t typescript -t javascript "multiDocuments" -A 2 -B 2
# Search for type definitions using AST
ast-grep --pattern 'type = "multiDocuments"'
Length of output: 16216
Script:
#!/bin/bash
# Let's try a different approach to search for multiDocuments type
# First, let's search in TypeScript files specifically using fd
fd -e ts -e tsx -x rg "multiDocuments" -A 2 -B 2 {}
# Also search for type definitions that might define this type
fd -e ts -e tsx -x rg "type.*=.*'multiDocuments'|type.*=.*\"multiDocuments\"" {}
Length of output: 1698
apps/backoffice-v2/src/pages/Entity/components/Case/Case.Documents.Toolbar.tsx (2)
4-4
: LGTM!
The import reorganization improves code organization without affecting functionality.
40-40
: Verify toolbar positioning with different document sizes
The positioning change from bottom-right-6
to bottom-4 right-4
aligns with the PR's layout fixes. However, let's ensure it works well across different scenarios.
✅ Verification successful
The previous command failed because the file type wasn't recognized. Let's try with more generic TypeScript/React file types and a broader pattern to catch similar positioning classes.
Let's try without specifying file types since ripgrep's file type detection seems to be having issues in the sandbox environment.
Toolbar positioning change is consistent with existing patterns
The change from bottom-right-6
to bottom-4 right-4
aligns with existing usage in the codebase. The same positioning pattern is used in:
- WebsiteLineOfBusiness component for positioning elements
- Dev sidebar in the headless example
- The Case Documents Toolbar itself
This consistent usage across components suggests the positioning is a standardized approach and works well in different scenarios.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other components that might be affected by this positioning
rg -l "bottom-right-6|bottom-4 right-4" --type tsx
Length of output: 80
Script:
#!/bin/bash
# Search for similar positioning patterns in TypeScript/React files
rg -l "bottom-right-|bottom-\d+ right-\d+" --type ts --type tsx --type jsx
# Also search for the specific class combination
rg "bottom-4 right-4" --type ts --type tsx --type jsx
Length of output: 188
Script:
#!/bin/bash
# Search for similar positioning patterns without specifying file types
rg -l "bottom-right-|bottom-\d+ right-\d+"
# Also search for the specific class combination
rg "bottom-4 right-4"
Length of output: 1019
apps/backoffice-v2/src/lib/blocks/create-blocks-typed/create-blocks-typed.ts (2)
Line range hint 37-61
: LGTM! Cell mappings are properly configured.
The new components (dataTable
, image
, readOnlyDetails
) are correctly mapped in the cells
object, maintaining consistency with the existing pattern and type safety.
8-8
: LGTM! Verify component integration.
The new imports for DataTableCell
, ImageCell
, and ReadOnlyDetailsCell
are well-structured and align with the PR's document rendering improvements.
Let's verify these components are properly integrated:
Also applies to: 13-13, 20-20
✅ Verification successful
Components properly integrated and mapped
The verification shows that:
- All three components (
DataTableCell
,ImageCell
,ReadOnlyDetailsCell
) are properly implemented as React components - They are correctly mapped in the
cells
object increate-blocks-typed.ts
- Their TypeScript types are properly defined in
types.ts
with appropriate props and values
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the new components are properly implemented and used
# Check if the components exist and export expected interfaces
ast-grep --pattern 'export const $name = $$$' apps/backoffice-v2/src/lib/blocks/components/DataTableCell/DataTableCell.ts* apps/backoffice-v2/src/lib/blocks/components/ImageCell/ImageCell.ts* apps/backoffice-v2/src/lib/blocks/components/ReadOnlyDetailsCell/ReadOnlyDetailsCell.ts*
# Check for usage of these components in the codebase
rg -l "DataTableCell|ImageCell|ReadOnlyDetailsCell" apps/backoffice-v2/src/
Length of output: 735
Script:
#!/bin/bash
# Let's verify the component implementations and their integration
# Check DataTableCell implementation
ast-grep --pattern 'export const DataTableCell = $$$'
# Check ImageCell implementation
ast-grep --pattern 'export const ImageCell = $$$'
# Check ReadOnlyDetailsCell implementation
ast-grep --pattern 'export const ReadOnlyDetailsCell = $$$'
# Check how these components are mapped in create-blocks-typed
rg -A 5 "DataTableCell|ImageCell|ReadOnlyDetailsCell" apps/backoffice-v2/src/lib/blocks/create-blocks-typed/create-blocks-typed.ts
# Check type definitions
rg -A 3 "DataTableCell|ImageCell|ReadOnlyDetailsCell" apps/backoffice-v2/src/lib/blocks/create-blocks-typed/types.ts
Length of output: 51155
apps/backoffice-v2/src/lib/blocks/hooks/useDocumentBlocks/useDocumentBlocks.tsx (2)
2-2
: LGTM: Import changes align with new feature requirements
The new imports properly support OCR functionality and workflow state management.
Also applies to: 5-5, 26-26
Line range hint 1-532
: Verify alignment with PR objectives
The changes successfully address the layout and document display requirements mentioned in the PR objectives. However, please verify:
- That the new grid layout properly contains documents without overflow
- Document cropping behavior across different viewport sizes
✅ Verification successful
Let me gather more information about the layout changes and grid implementation.
Based on the gathered information, I can now generate the final response as the code analysis shows clear evidence of proper responsive grid implementation and document handling.
Grid layout and document display implementation verified successfully
The changes implement a robust responsive grid layout with proper document handling:
- Responsive grid with dynamic column sizing:
grid-cols-[1fr_minmax(240px,280px)] md:grid-cols-[1fr_minmax(240px,360px)] lg:grid-cols-[1fr_minmax(240px,441px)] 2xl:grid-cols-[1fr_minmax(240px,600px)]
- Proper overflow handling: Multiple components use
overflow-auto
andoverflow-hidden
appropriately - Document container sizing: Uses
minmax()
to ensure proper document display across viewports - Grid gap handling: Consistent
gap-4
usage for spacing
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any existing overflow-related styles or issues
# Search for overflow-related styles
echo "Checking for overflow-related styles:"
rg -g '*.{css,scss}' 'overflow'
# Search for potential layout-related issues in commit history
echo "Checking recent layout-related fixes:"
git log -n 5 --grep="layout\|overflow\|crop" --pretty=format:"%h %s"
Length of output: 562
Script:
#!/bin/bash
# Check for grid-related styles and implementations
echo "Checking grid-related styles:"
rg -g '*.{css,scss,tsx}' 'grid-'
# Check for responsive design implementations
echo -e "\nChecking responsive design patterns:"
rg -g '*.{tsx,jsx}' 'md:|lg:|sm:|xl:'
# Check for container and sizing related classes
echo -e "\nChecking container and sizing patterns:"
rg -g '*.{tsx,jsx}' 'className="[^"]*(?:w-|h-|min-|max-)'
Length of output: 65022
Summary by CodeRabbit
Release Notes
New Features
wrapperClassName
andcontentClassName
.Bug Fixes
Style
Chores