fix(web): deadline detail #17427
fix(web): deadline detail #17427
Conversation
WalkthroughThe pull request introduces several modifications to the Grants section of the web application, focusing on improving the rendering and display of grant-related information. Changes include conditional rendering of the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial 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
Documentation and Community
|
Datadog ReportAll test runs ✅ 10 Total Test Services: 0 Failed, 10 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (2) |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17427 +/- ##
=======================================
Coverage 35.67% 35.67%
=======================================
Files 6941 6941
Lines 149131 149178 +47
Branches 42640 42658 +18
=======================================
+ Hits 53196 53217 +21
- Misses 95935 95961 +26
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 13 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/screens/Grants/Grant/GrantSidebar/DetailPanel.tsx (1)
71-73: Consider adding date format localizationThe deadline period display could benefit from localized date formatting to match user preferences.
- formatMessage(m.single.deadlinePeriod), + formatMessage(m.single.deadlinePeriod), + status.deadlinePeriod ? ( + <Text variant="medium"> + {formatDate(status.deadlinePeriod, locale)} + </Text> + ) : undefined,
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/screens/Grants/Grant/Grant.tsx(1 hunks)apps/web/screens/Grants/Grant/GrantSidebar/DetailPanel.tsx(2 hunks)apps/web/screens/Grants/Grant/GrantSidebar/ExtraPanel.tsx(1 hunks)apps/web/screens/Grants/Grant/GrantSidebar/GrantSidebar.tsx(1 hunks)apps/web/screens/Grants/messages.ts(1 hunks)apps/web/screens/Grants/utils.ts(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/screens/Grants/Grant/GrantSidebar/ExtraPanel.tsx
🧰 Additional context used
📓 Path-based instructions (5)
apps/web/screens/Grants/Grant/Grant.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/screens/Grants/Grant/GrantSidebar/GrantSidebar.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/screens/Grants/messages.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/screens/Grants/Grant/GrantSidebar/DetailPanel.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/screens/Grants/utils.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (5)
apps/web/screens/Grants/Grant/GrantSidebar/GrantSidebar.tsx (1)
58-69: Verify spacing consistency between panelsThe removal of the Stack component affects the vertical spacing between panels. Ensure that each panel has consistent spacing through individual margin controls.
✅ Verification successful
Spacing consistency is maintained ✓
The removal of the outer Stack component is safe as each panel (DetailPanel, ExtraPanel) maintains consistent internal spacing with
space={2}, while the containing Box provides adequate padding withpadding={3}.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if other sidebar components use Stack for spacing rg -l "Stack.*space.*[23]" apps/web/screens/GrantsLength of output: 229
Script:
#!/bin/bash # Check Stack implementations in sidebar components echo "=== GrantSidebar.tsx ===" rg "Stack.*space" apps/web/screens/Grants/Grant/GrantSidebar/GrantSidebar.tsx -B 1 -A 1 echo -e "\n=== ExtraPanel.tsx ===" rg "Stack.*space" apps/web/screens/Grants/Grant/GrantSidebar/ExtraPanel.tsx -B 1 -A 1 echo -e "\n=== DetailPanel.tsx ===" rg "Stack.*space" apps/web/screens/Grants/Grant/GrantSidebar/DetailPanel.tsx -B 1 -A 1Length of output: 704
apps/web/screens/Grants/Grant/GrantSidebar/DetailPanel.tsx (1)
59-67: LGTM! Improved status display clarityThe status display now properly combines application status with deadline status using consistent Text components.
apps/web/screens/Grants/Grant/Grant.tsx (1)
193-197: LGTM! Improved conditional renderingThe ExtraPanel is now only rendered when there's actual content (files or support links) to display, which is more efficient and provides better UX.
apps/web/screens/Grants/messages.ts (2)
149-152: LGTM: New deadline period message added correctly.The new
deadlinePeriodmessage follows the established pattern and naming conventions.
146-148: Verify thedeadlinemessage's default text.The
deadlinemessage uses the same default text ('Umsóknir') as theapplicationsmessage. This could cause confusion as they appear to serve different purposes. Consider using a more specific text for the deadline message.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/screens/Grants/utils.ts (1)
58-64: Consider extracting date formatting logicThe date formatting logic is repeated across multiple status cases. Consider extracting it into a helper function for better maintainability.
+const formatDeadlinePeriod = (dateFrom: string, dateTo: string): string => { + return `${format(new Date(dateFrom), 'dd.MM.yyyy')} - ${format( + new Date(dateTo), + 'dd.MM.yyyy', + )}`; +}; case GrantStatus.Closed: { // ... return { // ... - deadlinePeriod: - grant.dateFrom && grant.dateTo - ? `${format(new Date(grant.dateFrom), 'dd.MM.yyyy')} - ${format( - new Date(grant.dateTo), - 'dd.MM.yyyy', - )}` - : undefined, + deadlinePeriod: grant.dateFrom && grant.dateTo + ? formatDeadlinePeriod(grant.dateFrom, grant.dateTo) + : undefined, }; }Also applies to: 79-85, 133-139
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/screens/Grants/Grant/Grant.tsx(1 hunks)apps/web/screens/Grants/Grant/GrantSidebar/DetailPanel.tsx(2 hunks)apps/web/screens/Grants/Grant/GrantSidebar/ExtraPanel.tsx(1 hunks)apps/web/screens/Grants/Grant/GrantSidebar/GrantSidebar.tsx(1 hunks)apps/web/screens/Grants/messages.ts(1 hunks)apps/web/screens/Grants/utils.ts(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/screens/Grants/Grant/GrantSidebar/ExtraPanel.tsx
🧰 Additional context used
📓 Path-based instructions (5)
apps/web/screens/Grants/messages.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/screens/Grants/utils.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/screens/Grants/Grant/Grant.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/screens/Grants/Grant/GrantSidebar/DetailPanel.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/screens/Grants/Grant/GrantSidebar/GrantSidebar.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (7)
apps/web/screens/Grants/Grant/GrantSidebar/GrantSidebar.tsx (1)
58-69: Verify spacing between panelsThe removal of the Stack component affects the spacing between panels. Ensure that each panel component (DetailPanel, ExtraPanel, InstitutionPanel) handles its own margin/spacing to maintain consistent layout.
✅ Verification successful
Spacing is properly handled by individual components ✅
Each panel component already implements its own spacing:
- DetailPanel: marginBottom={3}
- ExtraPanel: marginBottom={3}
- InstitutionPanel: No bottom margin (appropriate as last component)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for margin or spacing properties in panel components ast-grep --pattern 'marginBottom={$_}'Length of output: 40
Script:
#!/bin/bash # Search for panel component implementations and their styles echo "=== Panel Component Implementations ===" fd "DetailPanel|ExtraPanel|InstitutionPanel" --type f echo -e "\n=== Styled Components Usage ===" rg -l "styled\." --type ts --type tsx echo -e "\n=== CSS Properties in Components ===" rg "margin|padding" --type ts --type tsx -g "*Panel*"Length of output: 892
Script:
#!/bin/bash # Check content of panel components echo "=== DetailPanel Content ===" cat "apps/web/screens/Grants/Grant/GrantSidebar/DetailPanel.tsx" echo -e "\n=== ExtraPanel Content ===" cat "apps/web/screens/Grants/Grant/GrantSidebar/ExtraPanel.tsx" echo -e "\n=== InstitutionPanel Content ===" cat "apps/web/components/InstitutionPanel/InstitutionPanel.tsx" echo -e "\n=== Styled Components and CSS Properties ===" rg "styled\.|margin|padding" -t tsx -t ts -g "*Panel.tsx"Length of output: 7594
apps/web/screens/Grants/Grant/GrantSidebar/DetailPanel.tsx (3)
59-67: LGTM! Status text combination improves readabilityThe fragment combining application status and deadline status enhances information density while maintaining clarity.
71-73: LGTM! Deadline period display enhancementThe addition of a dedicated deadline period display improves the clarity of grant timing information.
97-102: LGTM! Proper spacing maintainedThe addition of marginBottom={3} ensures consistent spacing after removing the Stack component from the parent.
apps/web/screens/Grants/utils.ts (1)
14-14: LGTM! Type definition enhancementThe addition of deadlinePeriod to the Status interface properly types the new feature.
apps/web/screens/Grants/Grant/Grant.tsx (1)
193-197: LGTM! Improved conditional renderingThe addition of the condition
(grant.files || grant.supportLinks)prevents rendering empty ExtraPanel components, improving the UI.apps/web/screens/Grants/messages.ts (1)
149-152: Clarify the distinction between 'deadline' and 'deadlinePeriod' messages.The addition follows the established naming conventions and integrates well with the PR objectives. However, there's an existing
deadlinemessage (line 124) with a similardefaultMessage. Could you clarify the intended difference between these two messages to ensure we're not introducing redundancy?
Grant
What
Detail panel
Display date period
Combine status texts
Remove extra spacing
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
These updates provide a more informative and streamlined user experience when viewing grant details.