feat: scrollable report page (BAL-3550) #3060
Conversation
WalkthroughThis update implements a new business report interface by introducing the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BusinessReport
participant useReportSections
participant Observer
User->>BusinessReport: Toggle sidebar / Select section
BusinessReport->>useReportSections: Retrieve report sections and refs
Observer->>BusinessReport: Notify active section via Intersection Observer
BusinessReport->>User: Update active section highlight and scroll into view
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (10)
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 (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (4)
packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx (1)
485-485:⚠️ Potential issueAdd missing key prop to list items.
The list items in the website structure risk indicators mapping lack a key prop, which could cause React reconciliation issues.
Add a unique key:
-<li className="list-decimal">{reason}</li> +<li key={reason} className="list-decimal">{reason}</li>🧰 Tools
🪛 Biome (1.9.4)
[error] 485-485: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
packages/ui/src/components/templates/report/components/Ecosystem/Ecosystem.tsx (1)
4-4: 🛠️ Refactor suggestionRemove unused import.
The
ContentTooltipimport is not used in the component and should be removed.-import { ContentTooltip } from '@/components/molecules/ContentTooltip/ContentTooltip';apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/hooks/useMerchantMonitoringBusinessReportLogic/useMerchantMonitoringBusinessReportLogic.tsx (2)
156-159: 🛠️ Refactor suggestionDestructure unused variable.
The
activeTabis destructured but not used in the component after the removal of tab-related functionality.Apply this diff to remove the unused variable:
-const [{ activeTab, isNotesOpen }] = useZodSearchParams( +const [{ isNotesOpen }] = useZodSearchParams( MerchantMonitoringBusinessReportSearchSchema, { replace: true }, );
187-187: 🛠️ Refactor suggestionRemove unused property.
The
activeTabis included in the return object but is no longer used after removing tab functionality.Apply this diff to remove the unused property:
- activeTab,
🧹 Nitpick comments (13)
packages/ui/src/components/molecules/RiskIndicator/RiskIndicator.tsx (1)
28-28: Consider optimizing the Link rendering condition.While the current check
search && Link && <Link search={search} />is correct, you could make it more concise by leveraging optional chaining.-{search && Link && <Link search={search} />} +{search && Link?.({ search })}packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx (2)
239-243: Chart container dimensions need adjustment for better visualization.The current chart dimensions might cause display issues:
- The pie chart's width is set to
90%which might cause unnecessary whitespace- The legend wrapper style uses a fixed
50%width which could cause text overflow on smaller screensConsider these improvements:
-<ResponsiveContainer width="90%" height="100%"> +<ResponsiveContainer width="100%" height="100%"> -wrapperStyle={{ width: '50%', maxHeight: '100%' }} +wrapperStyle={{ width: 'auto', maxWidth: '50%', maxHeight: '100%' }}Also applies to: 344-344, 354-354, 370-370
499-499: Fix typo in tooltip description.There's a typo in the word "website" in the pricing analysis tooltip.
-Analyzes webiste pricing strategies +Analyzes website pricing strategiespackages/ui/src/components/templates/report/components/Transactions/Transactions.tsx (2)
9-16: Simplify className strings by removing unnecessary template literals.Template literals are used for static strings which can be simplified.
- <div className={`relative flex`}> + <div className="relative flex"> <Image width={1236} height={567} alt={`Transaction Analysis`} src={'/images/transaction-analysis.png'} - className={`d-full max-w-[1236px]`} + className="d-full max-w-[1236px]" /> ... - <p className={`mt-3 text-xs`}> + <p className="mt-3 text-xs"> ... - className={`mt-3 flex items-center text-sm text-[#007AFF]`} + className="mt-3 flex items-center text-sm text-[#007AFF]"Also applies to: 20-23, 28-29
10-16: Consider externalizing hardcoded strings.The component contains several hardcoded strings that could be externalized for better maintainability and localization support.
Also applies to: 20-23, 26-32
apps/backoffice-v2/src/domains/business-reports/components/BusinessReport/BusinessReport.tsx (2)
52-58: Enhance keyboard navigation and ARIA attributes.While the implementation includes basic ARIA labels, it could benefit from improved keyboard navigation and ARIA attributes for better accessibility.
Apply this diff to enhance accessibility:
<nav aria-label="Report Scroll Tracker" + role="navigation" className={ctw( 'sticky top-0 h-screen overflow-hidden p-4 transition-all duration-300', isSidebarOpen ? 'w-60' : 'w-16', )} > {/* ... */} - <ul className="space-y-2"> + <ul className="space-y-2" role="menu"> {sections.map(section => ( <li key={section.id} + role="menuitem" + tabIndex={0} className={ctw( 'mb-2 flex cursor-pointer items-center gap-2', activeSection === section.id && 'font-bold text-blue-600', !isSidebarOpen && 'pl-2', )} onClick={() => scrollToSection(section.id)} + onKeyDown={e => { + if (e.key === 'Enter' || e.key === ' ') { + scrollToSection(section.id); + } + }} + aria-current={activeSection === section.id ? 'true' : undefined} >Also applies to: 75-98
108-134: Consider memoizing section rendering for performance.The sections are re-rendered on every state change. Consider memoizing the section rendering to optimize performance.
Apply this diff to optimize rendering:
+import { useMemo } from 'react'; export const BusinessReport = ({ report }: BusinessReportProps) => { const { sections, sectionRefs, parentRef } = useReportSections({ report }); const [isSidebarOpen, setIsSidebarOpen] = useState(true); + const renderedSections = useMemo( + () => + sections.map(section => { + const titleContent = ( + <div className="mb-6 mt-8 flex items-center gap-2 text-lg font-bold"> + {section.Icon && <section.Icon className="d-6" />} + <span>{section.title}</span> + </div> + ); + + return ( + <div + key={section.id} + id={section.id} + ref={el => (sectionRefs.current[section.id] = el)} + > + {section.description ? ( + <ContentTooltip description={section.description}>{titleContent}</ContentTooltip> + ) : ( + <>{titleContent}</> + )} + {section.Component} + </div> + ); + }), + [sections], + ); return ( <div ref={parentRef} className={`flex transition-all duration-300`}> <div className={`flex-1 overflow-y-visible transition-all duration-300`}> - {sections.map(section => { - // ... existing section rendering code - })} + {renderedSections} </div>packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx (1)
91-98: Consider destructuring report.data for cleaner code.The repeated use of optional chaining on
report.datacould be simplified.Apply this diff to improve code readability:
+ const { summary, ongoingMonitoringSummary, riskLevel, homePageScreenshotUrl } = report.data ?? {}; + <BusinessReportSummary - summary={report.data?.summary ?? ''} - ongoingMonitoringSummary={report.data?.ongoingMonitoringSummary ?? ''} - riskLevel={report.data?.riskLevel ?? null} + summary={summary ?? ''} + ongoingMonitoringSummary={ongoingMonitoringSummary ?? ''} + riskLevel={riskLevel ?? null} riskIndicators={sectionsSummary} Link={Link} - homepageScreenshotUrl={report.data?.homePageScreenshotUrl ?? ''} + homepageScreenshotUrl={homePageScreenshotUrl ?? ''} />packages/ui/src/components/templates/report/components/AdsAndSocialMedia/AdsAndSocialMedia.tsx (2)
111-127: Consider extracting the no-profile card to a separate component.The no-profile card rendering logic could be extracted to improve readability and reusability.
Consider creating a separate component:
type NoProfileCardProps = { provider: string; icon: ReactNode; }; const NoProfileCard = ({ provider, icon }: NoProfileCardProps) => ( <Card className={ctw('shadow-l w-full p-4 opacity-60')}> <div className="flex flex-row items-center gap-2 font-semibold"> {icon} <h4 className="text-xl">{capitalize(provider)}</h4> </div> <div className="my-4 flex items-center gap-2 text-gray-400"> <BanIcon className="h-5 w-5" /> <span className="text-sm">No {capitalize(provider)} profile detected.</span> </div> </Card> );
140-192: Consider extracting social media details section to a separate component.The social media details rendering logic is complex and could be extracted to improve maintainability.
Consider creating a separate component:
type SocialMediaDetailsProps = { provider: string; fields: typeof socialMediaMapper[keyof typeof socialMediaMapper]['fields']; rest: Record<string, unknown>; }; const SocialMediaDetails = ({ provider, fields, rest }: SocialMediaDetailsProps) => ( <div className="w-2/3 min-w-0 grow-0"> {/* ... existing fields rendering logic ... */} </div> );packages/ui/src/components/templates/report/hooks/useReportSections/useReportSections.tsx (2)
75-208: Consider extracting section configurations to a separate constant.The sections array configuration is complex and could be moved outside the hook for better maintainability.
Consider creating a separate configuration:
const REPORT_SECTIONS: Omit<BusinessReportSection, 'Component'>[] = [ { id: 'summary', title: 'Summary', description: "Provides a concise overview...", Icon: ListChecksIcon, }, // ... other section configurations ];Then use it in the hook:
const sections = useMemo( () => REPORT_SECTIONS.map(section => ({ ...section, Component: getComponentForSection(section.id, report.data), })), [report.data], );
215-233: Consider extracting intersection observer logic to a separate hook.The intersection observer setup could be extracted to a custom hook for better reusability.
Consider creating a separate hook:
const useIntersectionObserver = ( refs: MutableRefObject<Record<string, HTMLDivElement | null>>, onIntersect: (id: string) => void, options?: IntersectionObserverInit, ) => { useEffect(() => { const observer = new IntersectionObserver( entries => { entries.forEach(entry => { if (entry.isIntersecting) { onIntersect(entry.target.id); } }); }, options, ); Object.values(refs.current).forEach(ref => { if (ref) observer.observe(ref); }); return () => observer.disconnect(); }, [refs, onIntersect, options]); };apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/MerchantMonitoringBusinessReport.page.tsx (1)
289-294: Consider extracting header components.The header section with website information and status badges could be extracted into separate components for better maintainability.
Consider creating separate components:
const WebsiteHeader = ({ websiteWithNoProtocol }: { websiteWithNoProtocol: string }) => ( <TextWithNAFallback as={'h2'} className="pb-4 text-2xl font-bold"> {websiteWithNoProtocol} </TextWithNAFallback> ); const StatusHeader = ({ businessReport, statusToBadgeData, notes }: StatusHeaderProps) => ( <div className={`flex items-center space-x-8 pb-4`}> {/* ... existing status badges and info ... */} </div> );Also applies to: 295-337
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
apps/backoffice-v2/src/domains/business-reports/components/BusinessReport/BusinessReport.tsx(1 hunks)apps/backoffice-v2/src/lib/blocks/variants/WebsiteMonitoringBlocks/hooks/useWebsiteMonitoringReportBlock/hooks/useWebsiteMonitoringBusinessReportTab/useWebsiteMonitoringBusinessReportTab.tsx(1 hunks)apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/MerchantMonitoringBusinessReport.page.tsx(3 hunks)apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/hooks/useMerchantMonitoringBusinessReportLogic/useMerchantMonitoringBusinessReportLogic.tsx(1 hunks)packages/ui/src/components/atoms/Chart/Chart.tsx(1 hunks)packages/ui/src/components/molecules/RiskIndicator/RiskIndicator.tsx(2 hunks)packages/ui/src/components/molecules/RiskIndicatorsSummary/RiskIndicatorsSummary.tsx(2 hunks)packages/ui/src/components/templates/report/components/AdsAndSocialMedia/AdsAndSocialMedia.tsx(1 hunks)packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx(2 hunks)packages/ui/src/components/templates/report/components/Ecosystem/Ecosystem.tsx(1 hunks)packages/ui/src/components/templates/report/components/Transactions/Transactions.tsx(1 hunks)packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx(6 hunks)packages/ui/src/components/templates/report/components/WebsiteLineOfBusiness/WebsiteLineOfBusiness.tsx(1 hunks)packages/ui/src/components/templates/report/components/WebsitesCompany/WebsitesCompany.tsx(1 hunks)packages/ui/src/components/templates/report/hooks/index.ts(1 hunks)packages/ui/src/components/templates/report/hooks/useReportSections/index.ts(1 hunks)packages/ui/src/components/templates/report/hooks/useReportSections/useReportSections.tsx(1 hunks)packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx(4 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/ui/src/components/templates/report/hooks/useReportSections/index.ts
- packages/ui/src/components/atoms/Chart/Chart.tsx
- apps/backoffice-v2/src/lib/blocks/variants/WebsiteMonitoringBlocks/hooks/useWebsiteMonitoringReportBlock/hooks/useWebsiteMonitoringBusinessReportTab/useWebsiteMonitoringBusinessReportTab.tsx
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Analyze (javascript)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: build (windows-latest)
- GitHub Check: lint
- GitHub Check: build (ubuntu-latest)
- GitHub Check: spell_check
🔇 Additional comments (16)
packages/ui/src/components/molecules/RiskIndicator/RiskIndicator.tsx (2)
6-8: LGTM! Import statements are well organized.The restructured imports improve code organization by grouping external and internal imports separately while making the dependencies more explicit.
19-22: LGTM! Props interface is more flexible and type-safe.The changes improve the component's reusability by:
- Making
searchandLinkprops optional- Using a more explicit array type for
riskIndicatorspackages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx (3)
58-62: Type declarations have been standardized using explicit Array syntax.The change from array shorthand syntax (
T[]) to explicitArray<T>syntax improves consistency but doesn't affect functionality.
213-229: Improved tooltip structure for Traffic Analysis section.The tooltip content has been restructured to eliminate unnecessary wrapping divs, making the code cleaner and more maintainable.
230-231: Enhanced responsive layout with better breakpoints.The layout has been improved with:
- Flexible column/row layouts using
flex-coland2xl:!flex-row- Consistent height management across breakpoints
- Better width distribution between chart sections
Also applies to: 338-339, 400-400
packages/ui/src/components/molecules/RiskIndicatorsSummary/RiskIndicatorsSummary.tsx (3)
10-10: LGTM! Making the search property optional improves component flexibility.The change allows the component to be used in contexts where search functionality is not required.
13-13: LGTM! Making the Link property optional improves component flexibility.The change allows the component to be used without requiring a custom Link implementation.
11-11:❓ Verification inconclusive
Verify prop naming consistency.
There's a mismatch between the prop definition and its usage:
- The prop is defined as
indicatorsin the interface (line 11)- But it's passed as
riskIndicatorsto theRiskIndicatorcomponent (line 24)This could indicate that the
RiskIndicatorcomponent hasn't been updated to use the new prop name.Let's verify the prop name in the
RiskIndicatorcomponent:Also applies to: 24-24
🏁 Script executed:
#!/bin/bash # Search for the RiskIndicator component's prop definition ast-grep --pattern 'export const RiskIndicator: FunctionComponent<{ $$$ riskIndicators: $_ $$$ }>'Length of output: 106
Action Required: Verify Prop Naming Consistency
There's an inconsistency between the updated prop name in the interface and its usage in the JSX:
- The interface in
RiskIndicatorsSummary.tsx(line 11) defines the prop asindicators.- However, the component is being passed a prop named
riskIndicators(line 24).The automated search for the
RiskIndicatorcomponent’s prop definition did not return results. Please manually verify theRiskIndicatorcomponent to confirm whether its props are defined with the new name (indicators) or if it still expectsriskIndicators. Adjust accordingly to ensure consistent naming between the parent and child components.packages/ui/src/components/templates/report/hooks/index.ts (1)
1-3: LGTM!The addition of the
useReportSectionsexport aligns with the PR objectives of implementing a scrollable report page.packages/ui/src/components/templates/report/components/Ecosystem/Ecosystem.tsx (1)
12-18: LGTM!The UI changes improve consistency and clarity:
- Adjusted vertical spacing for better visual rhythm
- Updated header text to be more descriptive
packages/ui/src/components/templates/report/components/Transactions/Transactions.tsx (1)
7-37: LGTM!The refactoring to explicit return improves code readability.
packages/ui/src/components/templates/report/components/WebsitesCompany/WebsitesCompany.tsx (1)
10-43: LGTM!The changes improve the component:
- Simplified props by removing unused
companyName- Improved type definition for
riskIndicators- Consistent spacing with other components
- Good error handling for empty risk indicators
packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx (2)
14-32: LGTM! Props refactoring improves type safety and flexibility.The changes to the props interface enhance type safety with more precise types and improve flexibility with optional properties.
34-34: LGTM! Grid layout enhances visual organization.The grid system with gap utilities provides better spacing and organization of the content.
packages/ui/src/components/templates/report/components/WebsiteLineOfBusiness/WebsiteLineOfBusiness.tsx (1)
16-16: LGTM! Spacing adjustment improves visual density.The reduction in vertical spacing from
space-y-8tospace-y-6provides better content density while maintaining readability.packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx (1)
24-64: LGTM! Type improvements enhance code maintainability.The changes to
sectionsSummarytype improve type safety and naming consistency. The use ofReadonlyArrayand explicit type definition is a good practice.
apps/backoffice-v2/src/domains/business-reports/components/BusinessReport/BusinessReport.tsx
Outdated
Show resolved
Hide resolved
...fice-v2/src/pages/MerchantMonitoringBusinessReport/MerchantMonitoringBusinessReport.page.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
packages/ui/src/components/templates/report/components/WebsitesCompany/WebsitesCompany.tsx (2)
19-38: Consider optimizing the conditional checks and list item keys.The implementation is solid but could be improved:
- The double bang operator (!!) is redundant when used with optional chaining.
- Using just the
reasontext as a key might lead to issues if multiple items have the same reason.Consider applying these improvements:
<ol className={ctw({ - 'ps-4': !!riskIndicators?.length, + 'ps-4': riskIndicators?.length > 0, })} > - {!!riskIndicators?.length && + {riskIndicators?.length > 0 && riskIndicators.map(({ reason, sourceUrl }) => ( - <li key={reason} className={'list-decimal'}> + <li key={`${reason}-${sourceUrl ?? ''}`} className={'list-decimal'}> {reason} {!!sourceUrl && ( <span className={'ms-4'}> (<BallerineLink href={sourceUrl}>source</BallerineLink>) </span> )} </li> ))} - {!riskIndicators?.length && ( + {!riskIndicators?.length > 0 && ( <li>No indications of negative company reputation were detected.</li> )} </ol>
29-31: Consider adding i18n and improving accessibility.
- Hardcoded text should be internationalized.
- The source link could benefit from additional context for screen readers.
Consider these improvements:
{!!sourceUrl && ( <span className={'ms-4'}> - (<BallerineLink href={sourceUrl}>source</BallerineLink>) + (<BallerineLink href={sourceUrl} aria-label={`View source for ${reason}`}>source</BallerineLink>) </span> )} - <li>No indications of negative company reputation were detected.</li> + <li>{t('websitesCompany.noNegativeReputation', 'No indications of negative company reputation were detected.')}</li>Also applies to: 36-36
apps/backoffice-v2/src/domains/business-reports/components/BusinessReport/BusinessReport.tsx (2)
51-58: Consider optimizing sidebar width transitions.The sidebar width transitions could cause layout shifts. Consider:
- Using CSS custom properties for width values
- Adding
contain: layoutto prevent layout shiftsApply this diff to improve the implementation:
+const SIDEBAR_WIDTH = { + OPEN: '60px', + CLOSED: '16px', +} as const; <nav aria-label="Report Scroll Tracker" className={ctw( - 'sticky top-0 h-screen overflow-hidden p-4 transition-all duration-300', - isSidebarOpen ? 'w-60' : 'w-16', + 'sticky top-0 h-screen overflow-hidden p-4 transition-all duration-300 contain-layout', + isSidebarOpen ? `w-[${SIDEBAR_WIDTH.OPEN}]` : `w-[${SIDEBAR_WIDTH.CLOSED}]`, )} >
130-130: Consider wrapping dynamic section components with error boundaries.The
section.Componentis rendered directly without error boundaries, which could cause the entire report to crash if a section component fails.Consider wrapping the component with an error boundary:
- {section.Component} + <ErrorBoundary fallback={<div>Failed to load section</div>}> + {section.Component} + </ErrorBoundary>apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/hooks/useMerchantMonitoringBusinessReportLogic/useMerchantMonitoringBusinessReportLogic.tsx (2)
88-150: Consider improving error handling and extracting note creation logic.The monitoring toggle implementation could benefit from:
- More specific error handling instead of generic error messages
- Extracting the note creation logic into a reusable function
Consider refactoring the note creation logic like this:
+ const createMonitoringNote = useCallback( + async ({ + content, + reason, + userReason, + }: { + content: string; + reason?: string; + userReason?: string; + }) => { + if (!businessReport?.business.id || !businessReport?.id) { + throw new Error('Missing required IDs'); + } + + const noteContent = [ + content, + reason ? `with reason: ${capitalize(reason)}` : null, + userReason ? `(${userReason})` : '', + ] + .filter(Boolean) + .join(' '); + + return mutateCreateNote({ + content: noteContent, + entityId: businessReport.business.id, + entityType: 'Business', + noteableId: businessReport.id, + noteableType: 'Report', + parentNoteId: null, + }); + }, + [businessReport?.business.id, businessReport?.id, mutateCreateNote], + ); const turnOnMonitoringMutation = useToggleMonitoringMutation({ state: 'on', onSuccess: () => { - void mutateCreateNote({ - content: 'Monitoring turned on', - entityId: businessReport?.business.id ?? '', - entityType: 'Business', - noteableId: businessReport?.id ?? '', - noteableType: 'Report', - parentNoteId: null, - }); + void createMonitoringNote({ content: 'Monitoring turned on' }); toast.success(t(`toast:business_monitoring_on.success`)); }, onError: error => { + const errorMessage = error instanceof Error ? error.message : 'Unknown error occurred'; toast.error( t(`toast:business_monitoring_on.error`, { - errorMessage: isObject(error) && 'message' in error ? error.message : error, + errorMessage, }), ); }, }); const turnOffMonitoringMutation = useToggleMonitoringMutation({ state: 'off', onSuccess: () => { const { reason, userReason } = form.getValues(); - const content = [ - 'Monitoring turned off', - reason ? `with reason: ${capitalize(reason)}` : null, - userReason ? `(${userReason})` : '', - ] - .filter(Boolean) - .join(' '); - void mutateCreateNote({ - content, - entityId: businessReport?.business.id ?? '', - entityType: 'Business', - noteableId: businessReport?.id ?? '', - noteableType: 'Report', - parentNoteId: null, - }); + void createMonitoringNote({ + content: 'Monitoring turned off', + reason, + userReason, + }); setIsDeboardModalOpen(false); setIsDropdownOpen(false); form.reset(); toast.success(t(`toast:business_monitoring_off.success`)); }, onError: error => { + const errorMessage = error instanceof Error ? error.message : 'Unknown error occurred'; toast.error( t(`toast:business_monitoring_off.error`, { - errorMessage: isObject(error) && 'message' in error ? error.message : error, + errorMessage, }), ); }, });
180-198: Consider adding explicit return type for better maintainability.The return value structure is well-organized, but could benefit from an explicit type definition.
Consider adding a return type like this:
type MerchantMonitoringBusinessReportLogicReturn = { onNavigateBack: () => void; websiteWithNoProtocol: string | undefined; businessReport: typeof businessReport; statusToBadgeData: typeof statusToBadgeData; notes: typeof notes; isNotesOpen: boolean; turnOngoingMonitoringOn: typeof turnOnMonitoringMutation.mutate; isDeboardModalOpen: boolean; setIsDeboardModalOpen: (value: boolean) => void; isDropdownOpen: boolean; setIsDropdownOpen: (value: boolean) => void; form: typeof form; onSubmit: typeof onSubmit; deboardingReasonOptions: typeof deboardingReasonOptions; isFetchingBusinessReport: boolean; locale: string; }; // Then use it in the hook: export const useMerchantMonitoringBusinessReportLogic = (): MerchantMonitoringBusinessReportLogicReturn => { // ... existing implementation };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/backoffice-v2/src/domains/business-reports/components/BusinessReport/BusinessReport.tsx(1 hunks)apps/backoffice-v2/src/lib/blocks/variants/WebsiteMonitoringBlocks/hooks/useWebsiteMonitoringReportBlock/hooks/useWebsiteMonitoringBusinessReportTab/useWebsiteMonitoringBusinessReportTab.tsx(1 hunks)apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/MerchantMonitoringBusinessReport.page.tsx(3 hunks)apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/hooks/useMerchantMonitoringBusinessReportLogic/useMerchantMonitoringBusinessReportLogic.tsx(2 hunks)packages/ui/src/components/templates/report/components/AdsAndSocialMedia/AdsAndSocialMedia.tsx(1 hunks)packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx(2 hunks)packages/ui/src/components/templates/report/components/Ecosystem/Ecosystem.tsx(1 hunks)packages/ui/src/components/templates/report/components/Transactions/Transactions.tsx(1 hunks)packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx(6 hunks)packages/ui/src/components/templates/report/components/WebsiteLineOfBusiness/WebsiteLineOfBusiness.tsx(1 hunks)packages/ui/src/components/templates/report/components/WebsitesCompany/WebsitesCompany.tsx(1 hunks)packages/ui/src/components/templates/report/hooks/useReportSections/useReportSections.tsx(1 hunks)packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/ui/src/components/templates/report/components/Ecosystem/Ecosystem.tsx
- packages/ui/src/components/templates/report/components/Transactions/Transactions.tsx
- packages/ui/src/components/templates/report/components/AdsAndSocialMedia/AdsAndSocialMedia.tsx
- packages/ui/src/components/templates/report/hooks/useReportSections/useReportSections.tsx
- packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx
- apps/backoffice-v2/src/lib/blocks/variants/WebsiteMonitoringBlocks/hooks/useWebsiteMonitoringReportBlock/hooks/useWebsiteMonitoringBusinessReportTab/useWebsiteMonitoringBusinessReportTab.tsx
- packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: build (windows-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Analyze (javascript)
- GitHub Check: lint
🔇 Additional comments (7)
packages/ui/src/components/templates/report/components/WebsitesCompany/WebsitesCompany.tsx (2)
2-2: LGTM! Props interface simplified and imports organized.The removal of the
companyNameprop and the standardization of theriskIndicatorstype usingArray<T>syntax improves type consistency. The imports are well-organized and logically grouped.Also applies to: 5-8, 10-12
14-14: LGTM! Consistent spacing adjustment.The reduced vertical spacing aligns with other components and supports the scrollable report page layout.
packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx (1)
14-32: LGTM! Props refactoring improves type safety and naming consistency.The changes to the props interface enhance type safety by:
- Making
searchoptional- Allowing
riskLevelto be null- Renaming
sectionstoriskIndicatorsandriskIndicatorstoindicatorsfor better domain alignmentapps/backoffice-v2/src/domains/business-reports/components/BusinessReport/BusinessReport.tsx (1)
25-45: Consider optimizing observer cleanup and initialization.The observer is created outside the
useEffecthook, which means it's recreated on every render. Additionally, the cleanup function doesn't remove observers from individual refs.packages/ui/src/components/templates/report/components/WebsiteLineOfBusiness/WebsiteLineOfBusiness.tsx (1)
16-16: LGTM! Improved spacing and tooltip descriptions.The changes enhance the user experience by:
- Reducing vertical spacing for better content density
- Providing more descriptive tooltips
apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/MerchantMonitoringBusinessReport.page.tsx (1)
338-351: LGTM! Improved loading states and navigation.The changes enhance the user experience by:
- Replacing tab-based navigation with the new BusinessReport component
- Adding appropriate skeleton loading states that match the content structure
apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/hooks/useMerchantMonitoringBusinessReportLogic/useMerchantMonitoringBusinessReportLogic.tsx (1)
1-20: LGTM! Well-organized imports and schema definitions.The imports are properly organized and the schema definitions are well-structured with appropriate validation logic.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/backoffice-v2/src/domains/business-reports/components/BusinessReport/BusinessReport.tsx (1)
34-45: 🛠️ Refactor suggestionOptimize observer initialization and cleanup.
The observer is created outside the
useEffecthook and cleanup doesn't unobserve individual refs.Apply this diff to optimize the implementation:
-const observer = new IntersectionObserver(observerCallback, { - threshold: 0.1, -}); useEffect(() => { + const observer = new IntersectionObserver(observerCallback, { + threshold: 0.1, + }); + Object.values(sectionRefs.current).forEach(ref => { if (ref) { observer.observe(ref); } }); - return () => observer.disconnect(); + return () => { + Object.values(sectionRefs.current).forEach(ref => { + if (ref) { + observer.unobserve(ref); + } + }); + observer.disconnect(); + }; }, []);
🧹 Nitpick comments (5)
apps/backoffice-v2/src/domains/business-reports/components/BusinessReport/BusinessReport.tsx (2)
52-58: Enhance accessibility for keyboard navigation.The nav element has an aria-label but lacks keyboard navigation support.
Consider adding keyboard navigation:
<nav aria-label="Report Scroll Tracker" + role="navigation" className={ctw( 'sticky top-0 h-screen overflow-hidden p-4 transition-all duration-300', isSidebarOpen ? 'w-60' : 'w-16', )} + onKeyDown={(e) => { + if (e.key === 'ArrowDown' || e.key === 'ArrowUp') { + e.preventDefault(); + const currentIndex = sections.findIndex(section => section.id === activeSection); + const nextIndex = e.key === 'ArrowDown' + ? Math.min(currentIndex + 1, sections.length - 1) + : Math.max(currentIndex - 1, 0); + scrollToSection(sections[nextIndex].id); + } + }} + tabIndex={0} >
103-144: Consider memoizing section components for performance.The sections array is mapped on every render, which could be optimized.
Consider using
useMemofor the sections:+import { useMemo } from 'react'; export const BusinessReport = ({ report }: BusinessReportProps) => { const { sections, sectionRefs, parentRef } = useReportSections({ report }); const [isSidebarOpen, setIsSidebarOpen] = useState(true); + const renderedSections = useMemo(() => + sections.map(section => { + const titleContent = ( + <div className="mb-6 mt-8 flex items-center gap-2 text-lg font-bold"> + {section.Icon && <section.Icon className="d-6" />} + <span>{section.title}</span> + </div> + ); + + return ( + <div + key={section.id} + id={section.id} + ref={el => (sectionRefs.current[section.id] = el)} + > + {section.description ? ( + <ContentTooltip description={section.description}>{titleContent}</ContentTooltip> + ) : ( + <>{titleContent}</> + )} + {section.Component} + </div> + ); + }), + [sections]); return ( <div ref={parentRef} className={`flex transition-all duration-300`}> <div className={`flex-1 overflow-y-visible transition-all duration-300`}> - {sections.map(section => { - // ... existing section rendering code - })} + {renderedSections} </div> <BusinessReportSectionsObserver sections={sections} sectionRefs={sectionRefs} isSidebarOpen={isSidebarOpen} setIsSidebarOpen={setIsSidebarOpen} /> </div> ); };apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/hooks/useMerchantMonitoringBusinessReportLogic/useMerchantMonitoringBusinessReportLogic.tsx (2)
88-94: Enhance error handling in form submission.The error handling could be improved by providing more specific error messages.
Consider enhancing the error handling:
const onSubmit: SubmitHandler<z.infer<typeof ZodDeboardingSchema>> = async (data, e) => { + try { if (!businessReport?.business.id) { - throw new Error('Business ID is missing'); + throw new Error('Cannot turn off monitoring: Business ID is missing'); } return turnOffMonitoringMutation.mutate(businessReport.business.id); + } catch (error) { + console.error('Form submission failed:', error); + throw error; + } };
152-158: Consider adding default values for search parameters.The search parameters schema could benefit from explicit default values.
Consider adding default values:
const MerchantMonitoringBusinessReportSearchSchema = z.object({ - isNotesOpen: ParsedBooleanSchema.catch(false), + isNotesOpen: ParsedBooleanSchema.default(false).catch(false), }); const [{ isNotesOpen }] = useZodSearchParams(MerchantMonitoringBusinessReportSearchSchema, { replace: true, });apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/MerchantMonitoringBusinessReport.page.tsx (1)
338-351: Consider adding error boundary for report rendering.The loading state is handled, but there's no error handling for the BusinessReport component.
Consider adding error handling:
+import { ErrorBoundary } from 'react-error-boundary'; +const ReportErrorFallback = ({ error }: { error: Error }) => ( + <div className="p-4 text-destructive"> + <h3 className="text-lg font-bold">Error Loading Report</h3> + <p>{error.message}</p> + </div> +); {isFetchingBusinessReport || !businessReport ? ( <> <Skeleton className="h-6 w-72" /> {/* ... other skeleton elements ... */} </> ) : ( + <ErrorBoundary FallbackComponent={ReportErrorFallback}> <BusinessReport report={businessReport} /> + </ErrorBoundary> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/backoffice-v2/src/domains/business-reports/components/BusinessReport/BusinessReport.tsx(1 hunks)apps/backoffice-v2/src/lib/blocks/variants/WebsiteMonitoringBlocks/hooks/useWebsiteMonitoringReportBlock/hooks/useWebsiteMonitoringBusinessReportTab/useWebsiteMonitoringBusinessReportTab.tsx(1 hunks)apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/MerchantMonitoringBusinessReport.page.tsx(3 hunks)apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/hooks/useMerchantMonitoringBusinessReportLogic/useMerchantMonitoringBusinessReportLogic.tsx(2 hunks)packages/ui/src/components/templates/report/components/AdsAndSocialMedia/AdsAndSocialMedia.tsx(1 hunks)packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx(2 hunks)packages/ui/src/components/templates/report/components/Ecosystem/Ecosystem.tsx(1 hunks)packages/ui/src/components/templates/report/components/Transactions/Transactions.tsx(1 hunks)packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx(6 hunks)packages/ui/src/components/templates/report/components/WebsiteLineOfBusiness/WebsiteLineOfBusiness.tsx(1 hunks)packages/ui/src/components/templates/report/components/WebsitesCompany/WebsitesCompany.tsx(1 hunks)packages/ui/src/components/templates/report/hooks/useReportSections/useReportSections.tsx(1 hunks)packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/ui/src/components/templates/report/components/Transactions/Transactions.tsx
- packages/ui/src/components/templates/report/components/Ecosystem/Ecosystem.tsx
- apps/backoffice-v2/src/lib/blocks/variants/WebsiteMonitoringBlocks/hooks/useWebsiteMonitoringReportBlock/hooks/useWebsiteMonitoringBusinessReportTab/useWebsiteMonitoringBusinessReportTab.tsx
- packages/ui/src/components/templates/report/hooks/useReportSections/useReportSections.tsx
- packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx
- packages/ui/src/components/templates/report/components/AdsAndSocialMedia/AdsAndSocialMedia.tsx
- packages/ui/src/components/templates/report/components/WebsiteLineOfBusiness/WebsiteLineOfBusiness.tsx
- packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test_windows
- GitHub Check: build (windows-latest)
- GitHub Check: Analyze (javascript)
- GitHub Check: test_linux
- GitHub Check: build (ubuntu-latest)
- GitHub Check: lint
🔇 Additional comments (7)
packages/ui/src/components/templates/report/components/WebsitesCompany/WebsitesCompany.tsx (3)
1-9: LGTM!The imports are well-organized and all are utilized in the component.
10-12: LGTM!The prop changes simplify the component's interface while maintaining type safety through explicit typing of
riskIndicators.
13-43: LGTM!The implementation maintains core functionality while simplifying the UI through:
- Reduced vertical spacing for better visual consistency
- Removal of redundant UI elements (ContentTooltip and company name header)
packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx (4)
1-13: LGTM!The imports have been cleaned up by removing unused dependencies while maintaining clear organization.
33-73: LGTM!The implementation properly handles the nullable
riskLevelprop while maintaining the existing badge styling and content.
74-104: LGTM!The implementation maintains functionality while adapting to use the new
riskIndicatorsprop structure.
14-32:❓ Verification inconclusive
Verify naming consistency across the codebase.
The prop structure changes improve type safety and flexibility. However, the change from
riskIndicatorstoindicatorsin the internal structure should be verified for consistency with other components.
🏁 Script executed:
#!/bin/bash # Description: Check for consistency in naming between riskIndicators and indicators # Search for both naming patterns to ensure consistency echo "Searching for 'riskIndicators' usage:" rg "riskIndicators" -A 2 echo -e "\nSearching for 'indicators' usage:" rg "indicators(?!.*riskIndicators)" -A 2Length of output: 23770
Verify naming consistency for component props usage
The updated prop structure in
BusinessReportSummary.tsxnow usesriskIndicatorswith an internal property calledindicators. While usage ofriskIndicatorsappears consistent across the codebase, our initial search for standalone occurrences ofindicatorsusing a lookahead regex failed due to regex engine limitations. Please re-run the search with PCRE2 enabled to confirm that the naming change fromriskIndicatorsentries to their internalindicatorsproperty is applied consistently.
Next Action: Run the following shell script to search for standalone occurrences of
'indicators'using the--pcre2flag:#!/bin/bash echo "Searching for 'indicators' usage with PCRE2:" rg --pcre2 'indicators(?!.*riskIndicators)' -A 2Once verified, please confirm that there are no inconsistencies in how the
indicatorsproperty is used across components.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/backoffice-v2/src/domains/business-reports/components/BusinessReport/BusinessReport.tsx (2)
34-36: Consider adjusting the intersection threshold.A threshold of 0.1 (10%) might be too low and could cause sections to be marked as active too early. Consider increasing it to 0.3-0.5 for more accurate section detection.
const observer = new IntersectionObserver(observerCallback, { - threshold: 0.1, // Trigger when 10% of the section is visible + threshold: 0.3, // Trigger when 30% of the section is visible });
54-57: Enhance sidebar transition UX.The current width transition might feel abrupt. Consider adding a transform and opacity transition for a smoother experience.
className={ctw( - 'sticky top-0 h-screen overflow-hidden p-4 transition-all duration-300', + 'sticky top-0 h-screen overflow-hidden p-4 transition-all duration-300 ease-in-out', isSidebarOpen ? 'w-60' : 'w-16', // Adjust width for collapsed state )}packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx (1)
34-34: Enhance grid layout responsiveness.The current grid layout might not adapt well to smaller screens. Consider adding responsive grid columns.
- <div className={'grid grid-cols-5 gap-y-6 gap-x-8'}> + <div className={'grid grid-cols-1 md:grid-cols-3 lg:grid-cols-5 gap-y-6 gap-x-8'}>apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/hooks/useMerchantMonitoringBusinessReportLogic/useMerchantMonitoringBusinessReportLogic.tsx (1)
163-175: Enhance session storage handling.Consider using a constant for the session storage key and add error handling for storage operations.
+const PREVIOUS_PATH_KEY = 'merchant-monitoring:business-report:previous-path'; + const onNavigateBack = useCallback(() => { - const previousPath = sessionStorage.getItem( - 'merchant-monitoring:business-report:previous-path', - ); + try { + const previousPath = sessionStorage.getItem(PREVIOUS_PATH_KEY); - if (!previousPath) { - navigate('../'); + if (!previousPath) { + navigate('../'); + return; + } - return; + navigate(previousPath); + sessionStorage.removeItem(PREVIOUS_PATH_KEY); + } catch (error) { + console.error('Failed to access session storage:', error); + navigate('../'); } - - navigate(previousPath); - sessionStorage.removeItem('merchant-monitoring:business-report:previous-path'); }, [navigate]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
apps/backoffice-v2/src/domains/business-reports/components/BusinessReport/BusinessReport.tsx(1 hunks)apps/backoffice-v2/src/lib/blocks/variants/WebsiteMonitoringBlocks/hooks/useWebsiteMonitoringReportBlock/hooks/useWebsiteMonitoringBusinessReportTab/useWebsiteMonitoringBusinessReportTab.tsx(1 hunks)apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/MerchantMonitoringBusinessReport.page.tsx(3 hunks)apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/hooks/useMerchantMonitoringBusinessReportLogic/useMerchantMonitoringBusinessReportLogic.tsx(2 hunks)packages/ui/src/components/atoms/Chart/Chart.tsx(1 hunks)packages/ui/src/components/molecules/RiskIndicator/RiskIndicator.tsx(2 hunks)packages/ui/src/components/molecules/RiskIndicatorsSummary/RiskIndicatorsSummary.tsx(2 hunks)packages/ui/src/components/templates/report/components/AdsAndSocialMedia/AdsAndSocialMedia.tsx(1 hunks)packages/ui/src/components/templates/report/components/BusinessReportSummary/BusinessReportSummary.tsx(2 hunks)packages/ui/src/components/templates/report/components/Ecosystem/Ecosystem.tsx(1 hunks)packages/ui/src/components/templates/report/components/Transactions/Transactions.tsx(1 hunks)packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx(6 hunks)packages/ui/src/components/templates/report/components/WebsiteLineOfBusiness/WebsiteLineOfBusiness.tsx(1 hunks)packages/ui/src/components/templates/report/components/WebsitesCompany/WebsitesCompany.tsx(1 hunks)packages/ui/src/components/templates/report/hooks/index.ts(1 hunks)packages/ui/src/components/templates/report/hooks/useReportSections/index.ts(1 hunks)packages/ui/src/components/templates/report/hooks/useReportSections/useReportSections.tsx(1 hunks)packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- packages/ui/src/components/templates/report/hooks/useReportSections/index.ts
- packages/ui/src/components/atoms/Chart/Chart.tsx
- packages/ui/src/components/templates/report/components/Transactions/Transactions.tsx
- apps/backoffice-v2/src/lib/blocks/variants/WebsiteMonitoringBlocks/hooks/useWebsiteMonitoringReportBlock/hooks/useWebsiteMonitoringBusinessReportTab/useWebsiteMonitoringBusinessReportTab.tsx
- packages/ui/src/components/templates/report/components/Ecosystem/Ecosystem.tsx
- packages/ui/src/components/templates/report/hooks/index.ts
- packages/ui/src/components/templates/report/components/WebsiteLineOfBusiness/WebsiteLineOfBusiness.tsx
- packages/ui/src/components/molecules/RiskIndicatorsSummary/RiskIndicatorsSummary.tsx
- packages/ui/src/components/templates/report/hooks/useReportTabs/useReportTabs.tsx
- packages/ui/src/components/molecules/RiskIndicator/RiskIndicator.tsx
- packages/ui/src/components/templates/report/components/AdsAndSocialMedia/AdsAndSocialMedia.tsx
- packages/ui/src/components/templates/report/components/WebsiteCredibility/WebsiteCredibility.tsx
- packages/ui/src/components/templates/report/hooks/useReportSections/useReportSections.tsx
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test_windows
- GitHub Check: test_linux
- GitHub Check: build (windows-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Analyze (javascript)
- GitHub Check: lint
🔇 Additional comments (5)
apps/backoffice-v2/src/domains/business-reports/components/BusinessReport/BusinessReport.tsx (1)
34-45: Consider optimizing observer cleanup and initialization.The observer is created outside the
useEffecthook, which means it's recreated on every render. Additionally, the cleanup function doesn't remove observers from individual refs.packages/ui/src/components/templates/report/components/WebsitesCompany/WebsitesCompany.tsx (1)
10-12: LGTM! Clean and well-typed props definition.The component has been simplified to focus on risk indicators with proper type definitions using zod schema inference.
apps/backoffice-v2/src/pages/MerchantMonitoringBusinessReport/MerchantMonitoringBusinessReport.page.tsx (3)
47-47: LGTM!The import of the
BusinessReportcomponent aligns with the new scrollable report page implementation.
155-155: Removal of ScrollArea component.The ScrollArea component has been removed as previously suggested, and replaced with a flex layout section, which is more appropriate for the scrollable report page implementation.
338-351: Well-implemented loading state!The skeleton loading state effectively mirrors the content structure, providing a smooth user experience during data fetching. The conditional rendering is clear and handles both loading and data states appropriately.
apps/backoffice-v2/src/domains/business-reports/components/BusinessReport/BusinessReport.tsx
Outdated
Show resolved
Hide resolved
apps/backoffice-v2/src/domains/business-reports/components/BusinessReport/BusinessReport.tsx
Show resolved
Hide resolved
|
|
||
| export const BusinessReportSummary: FunctionComponent<{ | ||
| summary: string; | ||
| ongoingMonitoringSummary?: string; | ||
| sections: ReadonlyArray<{ | ||
| riskIndicators: ReadonlyArray<{ |
There was a problem hiding this comment.
from the name sections in this component it's very hard to understand the purpose of this prop, so i renamed it to better represent the intent. do you think there could be any issues with it?
| search: string; | ||
| riskIndicators: z.infer<typeof RiskIndicatorSchema>[] | null; | ||
| search?: string; | ||
| indicators: Array<z.infer<typeof RiskIndicatorSchema>> | null; |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/ui/src/components/templates/report/hooks/useReportSections/useReportSections.tsx (4)
11-11: Remove unused React hooks.The
useEffect,useRef, anduseStatehooks are imported but not used in this file.-import { ReactNode, useEffect, useMemo, useRef, useState } from 'react'; +import { ReactNode, useMemo } from 'react';
183-183: Replace type assertion with type validation.Using type assertion (
as BusinessReportSection[]) bypasses TypeScript's type checking. Consider using type validation instead.- ] as BusinessReportSection[], + ].map((section): BusinessReportSection => section),
145-151: Add type safety for traffic data.The traffic data object might contain undefined properties. Consider adding type safety:
- trafficData={{ - trafficSources: trafficSources, - monthlyVisits: monthlyVisits, - pagesPerVisit: pagesPerVisit, - timeOnSite: timeOnSite, - bounceRate: bounceRate, - }} + trafficData={{ + trafficSources: trafficSources ?? null, + monthlyVisits: monthlyVisits ?? null, + pagesPerVisit: pagesPerVisit ?? null, + timeOnSite: timeOnSite ?? null, + bounceRate: bounceRate ?? null, + }}
187-188: Add explicit return type.Consider adding an explicit return type for better maintainability and documentation.
-export const useReportSections = (report: z.infer<typeof ReportSchema>) => { +export const useReportSections = ( + report: z.infer<typeof ReportSchema>, +): { sections: BusinessReportSection[] } => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/backoffice-v2/src/domains/business-reports/components/BusinessReport/BusinessReport.tsx(1 hunks)packages/ui/src/components/templates/report/hooks/useReportSections/useReportSections.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/backoffice-v2/src/domains/business-reports/components/BusinessReport/BusinessReport.tsx
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: build-kyb / build-and-push
- GitHub Check: build-dashboard / build-and-push
- GitHub Check: build-backoffice / build-and-push
- GitHub Check: test_linux
- GitHub Check: test_windows
- GitHub Check: build (windows-latest)
- GitHub Check: Analyze (javascript)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: lint
🔇 Additional comments (2)
packages/ui/src/components/templates/report/hooks/useReportSections/useReportSections.tsx (2)
37-66: LGTM! Clean and safe data extraction.The implementation demonstrates good practices:
- Proper type validation using zod
- Safe destructuring with fallback
- Consistent use of optional chaining
184-184:❓ Verification inconclusive
Review useMemo dependencies.
The dependency array
[report.data]might be insufficient. Consider adding dependencies for imported components and utility functions if they can change.
🏁 Script executed:
#!/bin/bash # Check if the imported components or utilities are defined in the same package fd -e tsx -e ts "^(BusinessReportSummary|WebsitesCompany|WebsiteLineOfBusiness|WebsiteCredibility|Ecosystem|AdsAndSocialMedia|Transactions|getUniqueRiskIndicators)\.tsx?$" packages/ui/src/Length of output: 843
Review useMemo dependencies and related assumptions.
After verifying that the imported components (e.g., BusinessReportSummary, WebsitesCompany, WebsiteLineOfBusiness, WebsiteCredibility, Ecosystem, AdsAndSocialMedia, Transactions) are defined locally in the packages/ui directory, it appears they are stable and not likely to change at runtime. However, note the following:
- The dependency array currently only includes
[report.data]. If any externally imported utility (such asgetUniqueRiskIndicators) or the components are updated (e.g., via hot module replacement or changes in their internal logic), those changes won’t trigger a recalculation.- Double-check that
getUniqueRiskIndicators(which wasn’t found among the TSX files) is stable. If it can change independently, consider adding it to the dependency array.- Although not directly part of the memoization concern, also ensure that any type assertions used in forming the sections array and the handling of traffic data (with potentially undefined properties) are safe.
Please verify that the stability assumptions for these imported values hold in your use case and adjust the dependency array if needed.
Summary by CodeRabbit