fix(reports): add tooltip to the late reports #3254
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Task linked: CU-86c1arg2b [FEAT] Tooltip to the late reports |
WalkthroughThe changes involve adding a tooltip feature to the Changes
Sequence DiagramsequenceDiagram
participant User
participant Checkbox
participant HelpIcon
participant Tooltip
User->>HelpIcon: Hover
HelpIcon->>Checkbox: Update hover state
Checkbox->>Tooltip: Trigger display
Tooltip-->>User: Show contextual help
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 (
|
|
|
@rhahao Do you think we should create a new component, just for these purposes (tooltip with icon and checkbox)? Then we can get rid of code duplication. Or is this okay? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/features/reports/publisher_records_details/report_details/late_report/useLateReport.tsx (1)
74-75: Update TypeScript return type definitionThe hook's return type should be updated to include the new hover state properties.
Add these properties to the return type:
interface UseLateReportReturn { // ... existing properties lateCheckboxHelpIsHovered: boolean; setLateCheckboxHelpIsHovered: (value: boolean) => void; }src/features/reports/field_service/report_details/late_report/index.tsx (1)
35-54: Add aria-label to improve accessibilityThe help icon should have an aria-label for better screen reader support.
<Box onMouseEnter={() => setLateCheckboxHelpIsHovered(true)} onMouseLeave={() => setLateCheckboxHelpIsHovered(false)} + aria-label={t('tr_lateReportTooltip')} + role="button" sx={{ display: 'flex', alignItems: 'center', }} >src/features/reports/publisher_records_details/report_details/late_report/index.tsx (3)
6-6: Remove unused importThe
IconInfoimport appears to be unused in the current implementation.-import { IconHelpFilled, IconInfo } from '@components/icons'; +import { IconHelpFilled } from '@components/icons';
54-73: Add aria-label to improve accessibilityThe help icon should have an aria-label for better screen reader support, similar to the field service component.
<Box onMouseEnter={() => setLateCheckboxHelpIsHovered(true)} onMouseLeave={() => setLateCheckboxHelpIsHovered(false)} + aria-label={t('tr_lateReportTooltip')} + role="button" sx={{ display: 'flex', alignItems: 'center', }} >
Line range hint
1-89: Consider extracting shared tooltip componentBoth LateReport components share identical tooltip implementation. Consider extracting this into a shared component to improve maintainability.
Create a new shared component:
// src/features/reports/shared/LateReportTooltip.tsx interface LateReportTooltipProps { isHovered: boolean; onHoverChange: (value: boolean) => void; } const LateReportTooltip = ({ isHovered, onHoverChange }: LateReportTooltipProps) => { const { t } = useAppTranslation(); return ( <Tooltip title={t('tr_lateReportTooltip')} placement="bottom-start"> <Box onMouseEnter={() => onHoverChange(true)} onMouseLeave={() => onHoverChange(false)} aria-label={t('tr_lateReportTooltip')} role="button" sx={{ display: 'flex', alignItems: 'center', }} > <IconHelpFilled width={16} height={16} color={isHovered ? 'var(--accent-main)' : 'var(--black)'} /> </Box> </Tooltip> ); };src/features/reports/field_service/report_details/late_report/useLateReport.tsx (1)
133-141: Add TypeScript interface and JSDoc documentation.Consider adding type safety and documentation for better maintainability:
+ interface UseLateReportResult { + show_late: boolean; + late_sent: string; + checked: boolean; + handleChecked: (value: boolean) => Promise<void>; + readOnly: boolean; + setLateCheckboxHelpIsHovered: (value: boolean) => void; + lateCheckboxHelpIsHovered: boolean; + } + /** + * Hook to manage late report state and interactions + * @param person - The person associated with the report + * @returns {UseLateReportResult} Object containing late report state and handlers + */ const useLateReport = (person: PersonType): UseLateReportResult => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/locales/en/congregation.jsonis excluded by!**/*.json
📒 Files selected for processing (4)
src/features/reports/field_service/report_details/late_report/index.tsx(1 hunks)src/features/reports/field_service/report_details/late_report/useLateReport.tsx(2 hunks)src/features/reports/publisher_records_details/report_details/late_report/index.tsx(3 hunks)src/features/reports/publisher_records_details/report_details/late_report/useLateReport.tsx(3 hunks)
🔇 Additional comments (4)
src/features/reports/publisher_records_details/report_details/late_report/useLateReport.tsx (1)
16-17: LGTM: Clean state management addition
The hover state management follows React patterns and is appropriately initialized.
src/features/reports/field_service/report_details/late_report/index.tsx (1)
27-55: LGTM: Clean tooltip implementation
The tooltip implementation follows MUI best practices and maintains consistent styling with CSS variables.
src/features/reports/field_service/report_details/late_report/useLateReport.tsx (2)
28-30: LGTM! State variable follows React conventions.
The new hover state is well-named and appropriately initialized.
139-140: Verify consistent tooltip implementation.
The hook is used in multiple components. Let's verify that the tooltip implementation is consistent across all usage locations.
✅ Verification successful
Tooltip implementation is consistent across late report components
The tooltip implementation for the late report feature is consistent in both locations:
src/features/reports/field_service/report_details/late_report/index.tsx:
<Tooltip title={t('tr_lateReportTooltip')} placement="bottom-start">
<IconHelpFilled
lateCheckboxHelpIsHovered
/>
</Tooltip>src/features/reports/publisher_records_details/report_details/late_report/index.tsx:
<Tooltip title={t('tr_lateReportTooltip')} placement="bottom-start">
<IconHelpFilled
lateCheckboxHelpIsHovered
/>
</Tooltip>Both implementations use the same:
- Custom
Tooltipcomponent - Same tooltip text key (
tr_lateReportTooltip) - Same placement (
bottom-start) - Same icon component (
IconHelpFilled) - Same hover state handling (
lateCheckboxHelpIsHovered)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components using this hook and verify tooltip implementation
echo "Searching for components using useLateReport hook..."
rg -l "useLateReport" --type tsx
# Search for tooltip-related code in those components
echo "\nChecking tooltip implementation in components..."
rg "lateCheckboxHelpIsHovered|IconHelpFilled|Tooltip" --type tsx
Length of output: 381
Script:
#!/bin/bash
# Search for components using this hook and verify tooltip implementation
echo "Searching for components using useLateReport hook..."
rg -l "useLateReport"
# Search for tooltip-related code in those components
echo -e "\nChecking tooltip implementation in components..."
rg "lateCheckboxHelpIsHovered|IconHelpFilled|Tooltip"
Length of output: 28490
Description
Add small info icon + tooltip explaining how the mechanics of late reports work in Organized: Mark a report as 'Late' if the monthly branch office report has already been submitted. Late report will automatically be included in next month's submission.
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: