feat: Enhance Status Column with Colors for Better Readability #219
Conversation
WalkthroughThe pull request introduces significant modifications to the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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: 5
🧹 Outside diff range and nitpick comments (9)
shared/utils/capitalize.ts (1)
1-2: Add JSDoc documentation and unit testsSince this is a utility function used for UI elements (status labels), proper documentation and testing are essential.
Add JSDoc documentation:
+/** + * Capitalizes the first character of a string. + * @param str - The input string to capitalize + * @returns The capitalized string, or empty string for invalid inputs + * @example + * capitalize('pending') // returns 'Pending' + * capitalize('') // returns '' + * capitalize(null) // returns '' + */ export const capitalize = (str: string): string => {Would you like me to help generate comprehensive unit tests for this utility function?
shared/utils/checkStatus.ts (2)
16-24: Consider optimizing event checking logic.The current implementation could be improved for better type safety and performance.
Consider this optimization:
- for (const [event, status] of Object.entries(eventStatus)) { - if ( - request?.events?.some( - (e: { name?: string }) => e?.name?.toLowerCase() === event.toLowerCase() - ) - ) { - return capitalize(status); - } - } + const event = request?.events?.find( + (e: Types.IRequestEvent) => + e.name?.toLowerCase() in eventStatus + ); + if (event?.name) { + return capitalize(eventStatus[event.name.toLowerCase() as keyof typeof eventStatus]); + }
4-37: Consider timezone handling in date comparisons.The current implementation uses
new Date()for comparisons, which might lead to inconsistencies across different timezones. Consider using UTC dates or a library likedate-fnsfor more reliable date handling.Example implementation with UTC:
const today = new Date(); const todayUTC = Date.UTC(today.getUTCFullYear(), today.getUTCMonth(), today.getUTCDate()); const dueDateUTC = request?.contentData?.paymentTerms?.dueDate ? new Date(request.contentData.paymentTerms.dueDate).getTime() : null;shared/components/status-label.svelte (2)
45-52: Remove duplicate text-align property.The
text-align: centerproperty is defined twice in the.status-indicatorclass..status-indicator { text-align: center; padding: 6px 12px; - text-align: center; width: fit-content; border-radius: 8px; font-weight: 500; }
44-103: Enhance style maintainability and consistency.Several improvements could be made to the styles:
- Colors should be defined as CSS variables for better maintainability
- The rejected status color (currently using orange-red) should be orange according to PR objectives
- Add styles for the suggested "status-unknown" class
+:root { + --color-green: #328965; + --color-green-bg: rgba(88, 225, 165, 0.15); + --color-blue: #006ebe; + --color-blue-bg: rgba(90, 186, 255, 0.15); + --color-yellow: #c99101; + --color-yellow-bg: rgba(255, 197, 49, 0.15); + --color-dark-gray: #212121; + --color-dark-gray-bg: rgba(37, 37, 37, 0.15); + --color-orange: #ff8c00; + --color-orange-bg: rgba(255, 140, 0, 0.15); + --color-red: #d91c1c; + --color-red-bg: rgba(255, 49, 49, 0.15); + --color-light-gray: #7b7b7b; + --color-light-gray-bg: rgba(224, 224, 224, 0.15); +} .status-paid { - color: #328965; - background-color: rgba(88, 225, 165, 0.15); + color: var(--color-green); + background-color: var(--color-green-bg); } /* Update other status classes similarly */ .status-rejected { - color: #d9601c; - background-color: rgba(255, 135, 67, 0.15); + color: var(--color-orange); + background-color: var(--color-orange-bg); } +.status-unknown { + color: var(--color-light-gray); + background-color: var(--color-light-gray-bg); +}packages/invoice-dashboard/src/lib/dashboard/invoice-view.svelte (3)
372-372: Consider improving the status label placementWhile the StatusLabel implementation is correct, its current placement next to the invoice number might make it less prominent. Consider moving it to a more visible location, such as:
- Above the invoice details
- In a dedicated status section below the invoice number
- <StatusLabel status={checkStatus(request)} /> + </h2> + <div class="status-section"> + <StatusLabel status={checkStatus(request)} /> + </div> + <h2>
Line range hint
147-167: Enhance error handling with specific error messagesThe error handling in
checkInvoicecould be more informative. Consider categorizing errors and providing specific user feedback for different error types.} catch (err: any) { console.error("Error while checking invoice: ", err); if (String(err).includes("Unsupported payment")) { unsupportedNetwork = true; + toast.error("Unsupported payment network", { + description: "This payment network is not supported by the system.", + }); + } else if (err instanceof NetworkError) { + toast.error("Network Error", { + description: "Please check your connection and try again.", + }); + } else { + toast.error("Error checking invoice", { + description: "An unexpected error occurred. Please try again later.", + }); } } finally { loading = false; }
Line range hint
590-591: Add ARIA labels for better accessibilityThe loading and status indicators should have proper ARIA labels for screen readers.
- <div class="loading">Loading...</div> + <div class="loading" role="status" aria-live="polite">Loading...</div> - <div class="unsupported-network">Unsupported payment network!</div> + <div class="unsupported-network" role="alert" aria-live="assertive">Unsupported payment network!</div>Also applies to: 596-597
packages/invoice-dashboard/src/lib/view-requests.svelte (1)
647-647: Consider adding type safety to the checkStatus call.The implementation correctly uses the
StatusLabelcomponent for better readability, which aligns with the PR objectives. However, there's an opportunity to improve type safety.Consider explicitly typing the request parameter:
-<td><StatusLabel status={checkStatus(request)} /></td> +<td><StatusLabel status={checkStatus(request as Types.IRequestDataWithEvents)} /></td>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
packages/invoice-dashboard/src/lib/dashboard/invoice-view.svelte(2 hunks)packages/invoice-dashboard/src/lib/view-requests.svelte(3 hunks)shared/components/status-label.svelte(1 hunks)shared/utils/capitalize.ts(1 hunks)shared/utils/checkStatus.ts(1 hunks)
🧰 Additional context used
📓 Learnings (1)
packages/invoice-dashboard/src/lib/dashboard/invoice-view.svelte (2)
Learnt from: MantisClone
PR: RequestNetwork/web-components#141
File: packages/invoice-dashboard/src/lib/dashboard/invoice-view.svelte:0-0
Timestamp: 2024-11-12T14:52:33.204Z
Learning: In `packages/invoice-dashboard/src/lib/dashboard/invoice-view.svelte`, mapping payment network IDs to their respective approval functions in the `checkApproval` function is acceptable and can improve readability and maintainability.
Learnt from: MantisClone
PR: RequestNetwork/web-components#141
File: packages/invoice-dashboard/src/lib/dashboard/invoice-view.svelte:0-0
Timestamp: 2024-11-12T14:52:33.204Z
Learning: In `packages/invoice-dashboard/src/lib/dashboard/invoice-view.svelte`, prefer using `if`/`else if` structures over mapping in the `checkApproval` function for better readability.
🔇 Additional comments (8)
shared/utils/capitalize.ts (1)
1-2: Verify usage in status labels
Let's ensure this function handles all possible status values correctly.
✅ Verification successful
The capitalize function handles all status values correctly
The implementation is safe for all possible status values because:
- The status-label component handles all possible invoice statuses: "paid", "partially paid", "accepted", "awaiting payment", "canceled", "rejected", "overdue", and "pending"
- The checkStatus utility returns capitalized status values: "Paid", "Partially Paid", "Rejected", "Canceled", "Overdue", "Awaiting Payment"
- The APP_STATUS enum values are not used with this capitalize function
- The capitalize function safely handles edge cases by returning an empty string for null/undefined inputs
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all status values that will be capitalized
# Look for status-related constants or enums
rg -t typescript -t javascript "status.*=.*['\"]"
# Look for direct usage of the capitalize function
rg -t typescript -t javascript "capitalize\([^)]*\)"
Length of output: 178
Script:
#!/bin/bash
# Search for status-related constants or enums using ripgrep without file type filters
rg "status.*=.*['\"]"
# Look for direct usage of the capitalize function
rg "capitalize\("
# Find all TypeScript/JavaScript files that might contain status-related code
fd "\.(ts|js|tsx|jsx)$"
Length of output: 4568
Script:
#!/bin/bash
# Let's check the content of the status-label component and checkStatus utility
# to understand how the capitalize function is used with status values
cat shared/components/status-label.svelte
cat shared/utils/checkStatus.ts
# Also check if there are any status-related enums or types
cat shared/types/enums.ts
Length of output: 3597
shared/utils/checkStatus.ts (2)
1-3: LGTM! Imports are clean and necessary.
11-14: Verify status implementation against requirements.
The current implementation handles "Rejected" and "Canceled" statuses, but according to the requirements:
- "Rejected" status should be orange (to be implemented in future)
- Some statuses from requirements are not handled: "Accepted", "Created"
- "Pending" status exists but should not appear
Let's check if these statuses are used elsewhere in the codebase:
shared/components/status-label.svelte (2)
40-42: LGTM!
The template markup is clean and follows best practices.
1-103: Verify scope alignment with PR objectives.
The implementation includes styles for "rejected" and "overdue" statuses, which were marked as "to be implemented in a future issue" in the PR objectives. While the implementation is correct, it might be exceeding the intended scope of this PR.
packages/invoice-dashboard/src/lib/dashboard/invoice-view.svelte (2)
18-18: LGTM! Imports align with PR objectives
The addition of StatusLabel component and checkStatus utility imports supports the enhancement of the status column with better visual representation.
Also applies to: 26-26
372-372: Verify status color implementation
Please ensure that the StatusLabel component implements all the required status colors from issue #161:
- Paid (Green)
- Partially Paid (Green with white diagonal stripes)
- Accepted (Blue)
- Created (Yellow)
- Canceled (Dark Gray)
packages/invoice-dashboard/src/lib/view-requests.svelte (1)
12-12: LGTM! Import statements are well-organized.
The new imports for StatusLabel component and checkStatus utility are correctly placed in their respective sections (Components and Utils).
Also applies to: 38-38
|
@sstefdev could please resolve the coderabitai reviews and the merging conflicts? |
…161-colorize-the-status-column-in-the-invoice-dashboard
…161-colorize-the-status-column-in-the-invoice-dashboard
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
packages/invoice-dashboard/src/lib/dashboard/invoice-view.svelte (3)
375-375: Optimize status checking logicThe status is being checked redundantly. Consider using a reactive statement to compute and store the status:
-<StatusLabel status={checkStatus(request)} /> +<StatusLabel status={status} />This would leverage the existing
statusvariable that's already being updated when the request changes.Also applies to: 375-375
Line range hint
266-273: Enhance error handling in payTheRequestThe current error handling could be more informative. Consider categorizing errors and providing specific error messages:
- } catch (err) { - console.error("Something went wrong while paying : ", err); - loading = false; - statuses = []; + } catch (err: any) { + console.error("Payment failed:", err); + loading = false; + statuses = []; + const errorMessage = err.code === 4001 + ? "Transaction rejected by user" + : err.message || "Payment failed"; + toast.error("Payment Failed", { + description: errorMessage, + action: { + label: "X", + onClick: () => console.info("Close"), + }, + }); }
Line range hint
644-646: Enhance error state visibilityThe unsupported network message could be more informative and visually prominent:
- .unsupported-network { - font-size: 12px; - color: #e89e14ee; + .unsupported-network { + font-size: 14px; + color: #e89e14; + padding: 8px 12px; + background-color: #fff3cd; + border: 1px solid #ffeeba; + border-radius: 4px; + margin-top: 8px; }packages/invoice-dashboard/src/lib/view-requests.svelte (1)
651-651: Consider adding error handling for status checkThe status check could potentially fail if the request object is malformed. Consider adding error handling:
-<td><StatusLabel status={checkStatus(request)} /></td> +<td> + {#if request} + <StatusLabel status={checkStatus(request)} /> + {:else} + <span class="text-gray-400">-</span> + {/if} +</td>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/invoice-dashboard/src/lib/dashboard/invoice-view.svelte(2 hunks)packages/invoice-dashboard/src/lib/view-requests.svelte(2 hunks)
🧰 Additional context used
📓 Learnings (2)
packages/invoice-dashboard/src/lib/dashboard/invoice-view.svelte (1)
Learnt from: MantisClone
PR: RequestNetwork/web-components#141
File: packages/invoice-dashboard/src/lib/dashboard/invoice-view.svelte:0-0
Timestamp: 2024-11-12T14:52:33.204Z
Learning: In `packages/invoice-dashboard/src/lib/dashboard/invoice-view.svelte`, mapping payment network IDs to their respective approval functions in the `checkApproval` function is acceptable and can improve readability and maintainability.
packages/invoice-dashboard/src/lib/view-requests.svelte (1)
Learnt from: sstefdev
PR: RequestNetwork/web-components#218
File: packages/invoice-dashboard/src/lib/view-requests.svelte:24-24
Timestamp: 2024-11-26T12:56:45.612Z
Learning: In `packages/invoice-dashboard/src/lib/view-requests.svelte`, the import path `@requestnetwork/shared-icons/network/network-icon.svelte` is correct for importing the `Network` component.
🔇 Additional comments (5)
packages/invoice-dashboard/src/lib/dashboard/invoice-view.svelte (2)
18-18: LGTM! Clean integration of StatusLabel component
The StatusLabel component is properly imported and integrated, aligning with the PR objective to enhance status visibility.
Also applies to: 375-375
375-375: Verify status color implementation
The StatusLabel component has been integrated successfully. Let's verify that all status colors match the requirements from issue #161.
✅ Verification successful
Status color implementation matches requirements
The StatusLabel component has been correctly implemented with a comprehensive set of status colors that align with standard invoice states:
- Paid (green)
- Partially Paid (striped green)
- Accepted (blue)
- Awaiting Payment (yellow)
- Canceled (gray)
- Rejected (orange)
- Overdue (red)
- Pending (light gray)
The checkStatus utility properly determines the status based on payment state, due dates, and events, which is then correctly mapped to the appropriate color-coded label.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if StatusLabel component implements all required status colors
ast-grep --pattern 'export const statusColors = {
$$$
}'
# Check for any hardcoded status colors that should be using the StatusLabel
rg -n 'status.*color' --type svelte
Length of output: 129
Script:
#!/bin/bash
# Let's try a different approach to find the StatusLabel component and its color implementation
fd -e js -e jsx -e ts -e tsx -e svelte "StatusLabel" --exec cat {}
# Also search for any color-related constants or configurations
rg -i "status.*color|color.*status" --type-add 'web:*.{js,jsx,ts,tsx,svelte}' --type web
Length of output: 326
Script:
#!/bin/bash
# Let's examine the full content of the status component to see all status implementations
cat shared/components/status.svelte
# Also check for any status-related constants or enums that might define the possible statuses
rg -i "export.*status|status.*=" --type-add 'web:*.{js,jsx,ts,tsx,svelte}' --type web
Length of output: 7031
Script:
#!/bin/bash
# Let's check the StatusLabel component implementation and its color definitions
cat shared/components/status-label.svelte
# Also check the checkStatus utility to understand how statuses are determined
cat shared/utils/checkStatus.ts
Length of output: 3522
packages/invoice-dashboard/src/lib/view-requests.svelte (3)
12-12: LGTM! Clean import of StatusLabel component
The StatusLabel component import follows the established pattern and is correctly grouped with other shared components.
651-651: LGTM! Clean integration of StatusLabel component
The StatusLabel component is correctly integrated with the existing status check logic, which aligns with the PR objective of enhancing status visibility.
651-651: Verify StatusLabel component supports all status types
Ensure that the StatusLabel component supports all current status types and is prepared for future additions (Rejected and Overdue statuses).
✅ Verification successful
Let me gather more information about the status types and StatusLabel component implementation.
Let me try a different approach to find the status-related code.
Let me try one more time without specifying file types.
Based on the search results, I can now generate a final response. The code reveals that:
-
The
checkStatusfunction is implemented inshared/utils/checkStatus.tsand handles multiple statuses:- "Paid"
- "Partially Paid"
- "Rejected"
- "Canceled"
-
The
StatusLabelcomponent is imported from@requestnetwork/shared-components/status-label.svelteand is used consistently across the codebase. -
The status is determined by the
checkStatusfunction which evaluates:- Payment balance vs expected amount
- Event history (for rejected/canceled states)
The StatusLabel implementation appears to handle all current status types
The implementation properly supports all current status types (Paid, Partially Paid, Canceled, Rejected) through the checkStatus utility. The component is consistently used across the codebase and the status handling is centralized in the shared utilities, making it maintainable for future additions like Overdue status.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify StatusLabel component implementation supports all status types
# Check StatusLabel component implementation
echo "Checking StatusLabel component implementation..."
ast-grep --pattern 'export let status: $_'
# Check all possible status values used in the codebase
echo "Checking all status values used..."
rg -A 1 'checkStatus\('
Length of output: 1073
Script:
#!/bin/bash
# Search for StatusLabel component definition
echo "Searching for StatusLabel component definition..."
fd -e svelte -e ts -e js "StatusLabel" --exec cat {}
# Search for checkStatus function implementation
echo -e "\nSearching for checkStatus function implementation..."
ast-grep --pattern 'function checkStatus($_) {
$$$
}'
# Search for status type definitions
echo -e "\nSearching for status type definitions..."
fd -e ts -e js "types" --exec rg -l "type.*Status" {} || true
Length of output: 495
Script:
#!/bin/bash
# Search for status-related code with more context
echo "Searching for status-related code..."
rg -A 5 -B 5 "StatusLabel|checkStatus|type.*Status" --type svelte --type typescript --type javascript
# Search for enum or constant definitions related to status
echo -e "\nSearching for status enums or constants..."
rg -A 5 "enum.*Status|const.*Status" --type typescript --type javascript
# Find files containing both StatusLabel and status-related code
echo -e "\nSearching for files with both StatusLabel and status..."
rg --files-with-matches "StatusLabel.*status|status.*StatusLabel" --type svelte
Length of output: 668
Script:
#!/bin/bash
# Search for status-related code with more context
echo "Searching for status-related code..."
rg -A 5 -B 5 "StatusLabel|checkStatus|type.*Status"
# Search for enum or constant definitions related to status
echo -e "\nSearching for status enums or constants..."
rg -A 5 "enum.*Status|const.*Status"
# Find StatusLabel component implementation
echo -e "\nSearching for StatusLabel component..."
rg -A 10 "export.*StatusLabel"
Length of output: 12582
sstefdev
deleted the
161-colorize-the-status-column-in-the-invoice-dashboard
branch
Fixes #161
Problem
The lack of color coding in the Status column makes it difficult for users to quickly identify the status of invoices at a glance, reducing the efficiency of the Invoice Dashboard.
Changes
Summary by CodeRabbit
New Features
StatusLabelcomponent for improved status display across the invoice dashboard.Bug Fixes
Documentation