Feature/hcmpre 1632 #2114
Feature/hcmpre 1632 #2114
Conversation
📝 WalkthroughWalkthroughThis pull request introduces several enhancements to the campaign management module in the health micro-UI, focusing on attendance tracking and user management. The changes include adding new components for attendance tracking, user tables, and progress bars, updating proxy configurations, and implementing new hooks and services for fetching and displaying attendance-related data. The modifications aim to improve the functionality and user experience of the campaign management interface. Changes
Sequence DiagramsequenceDiagram
participant User
participant AttendancePage
participant AttendanceService
participant IndividualService
User->>AttendancePage: Navigate to Attendance
AttendancePage->>AttendanceService: Fetch Attendance Register
AttendanceService-->>AttendancePage: Return Attendance Data
AttendancePage->>IndividualService: Fetch User Details
IndividualService-->>AttendancePage: Return User Information
AttendancePage->>User: Display Attendance Table
Possibly Related PRs
Suggested Reviewers
Poem
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: 17
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
health/micro-ui/web/micro-ui-internals/example/package.jsonis excluded by!**/*.jsonhealth/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/package.jsonis excluded by!**/*.json
📒 Files selected for processing (12)
health/micro-ui/web/micro-ui-internals/example/src/setupProxy.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/campaign.scss(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/Module.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/IndividualUserTable.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/ProgressBar.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/tableCustomStyle.js(4 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/UICustomizations.js(4 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/index.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/services/searchAttendanceWithUserDetails.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/useAttendanceWithUserDetails.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/Attendance.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/index.js(2 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
health/micro-ui/web/micro-ui-internals/example/src/setupProxy.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/Module.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/ProgressBar.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/index.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/useAttendanceWithUserDetails.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/index.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/UICustomizations.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/services/searchAttendanceWithUserDetails.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/tableCustomStyle.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/IndividualUserTable.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/Attendance.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/UICustomizations.js
[error] 477-477: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 826-826: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/Attendance.js
[error] 138-138: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
[error] 139-139: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
[error] 140-140: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
[error] 141-141: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
[error] 311-311: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 312-316: 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)
[error] 320-327: 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)
🔇 Additional comments (10)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/services/searchAttendanceWithUserDetails.js (3)
61-92: Optimize API calls by checking if ID sets are non-emptyTo prevent unnecessary API calls, add checks to ensure that requests are made only when there are IDs to fetch.
[performance]
Modify the code as follows:
// Get staff user details + if (staffUserIds.size > 0) { const staffResponse = await Digit.CustomService.getResponse({ // existing code... }); + } else { + const staffResponse = { Individual: [] }; + } // Get attendee details + if (individualIds.size > 0) { const individualResponse = await Digit.CustomService.getResponse({ // existing code... }); + } else { + const individualResponse = { Individual: [] }; + }
148-157: Avoid exposing internal error details to the clientReturning
error.messagemight reveal sensitive internal information. It's safer to return a generic error message.[security]
Apply this diff:
return { registers: [], staffAttendeeMap: {}, attendeeDetailsMap: {}, userDetails: [], isError: true, - error: error.message || "An unknown error occurred", + error: "An error occurred while fetching attendance data", };
51-59:⚠️ Potential issueCorrect the condition to handle cases with available IDs
Currently, the function returns early if both
individualIdsandstaffUserIdsare empty. However, if either of them contains IDs, we should proceed to fetch the corresponding details.Apply this diff to adjust the condition:
- if (individualIds.size === 0 && staffUserIds.size === 0) { + if (individualIds.size === 0 && staffUserIds.size === 0) { return { registers: registerResponse.attendanceRegister, staffAttendeeMap, attendeeDetailsMap: {}, userDetails: [], isError: false }; + } else { + // Proceed to fetch details if either set has IDs + }Likely invalid or redundant comment.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/index.js (1)
22-22: LGTM! Clean integration of the attendance hook.The addition of
useAttendanceWithUserDetailsto the campaign hooks is well-organized and maintains the existing modular structure.Also applies to: 52-52
health/micro-ui/web/micro-ui-internals/example/src/setupProxy.js (1)
100-102: LGTM! Verify the proxy endpoints are accessible.The new health-related endpoints are properly configured. Let's verify their accessibility:
✅ Verification successful
Proxy endpoints are properly configured and actively used
The endpoints are verified to be correctly configured and actively used across the codebase:
/census-servicein microplan module for census operations/health-attendancein campaign manager for attendance tracking/health-individualfor user details management🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the health endpoints are properly configured and accessible # Test: Check if the new endpoints are referenced in the codebase rg -l "health-attendance.*/_search|health-individual.*/_search" --type jsLength of output: 313
Script:
#!/bin/bash # Check census-service endpoint usage rg "census-service" --type js -A 2 # Examine implementation in the found files cat health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/services/searchAttendanceWithUserDetails.js cat health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/IndividualUserTable.jsLength of output: 19316
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/tableCustomStyle.js (2)
Line range hint
72-159: Well-structured table style generator with frozen column support.The new
getTableCustomStylefunction provides good flexibility with thefreezeFirstColumnparameter. The implementation follows best practices for conditional styling.
3-3: Consider the implications of removing overflow styles.The removal of overflow styles from wrapper components might affect table behavior in constrained spaces. Consider adding a comment explaining why these were removed or if they should be configurable.
Also applies to: 6-6, 9-9, 12-12
✅ Verification successful
Overflow styles were intentionally removed as they are handled at the component level
The removal of overflow styles from the tableCustomStyle.js is correct since:
- Individual components manage their own overflow behavior through container styles and specific classes
- Tables that need fixed headers explicitly set scroll behavior using props like fixedHeader and fixedHeaderScrollHeight
- Parent containers provide the necessary overflow constraints based on their specific requirements
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential overflow issues in table implementations # Test: Look for table implementations that might be affected by the style changes rg -A 5 "tableCustomStyle" --type jsLength of output: 19507
Script:
#!/bin/bash # Check for table-related styles and size constraints rg -g '*.{js,css}' -A 3 'table.*{|\.table' # Look for specific style properties that might affect overflow rg -g '*.{js,css}' -A 3 'height:|width:|overflow:|scroll'Length of output: 69340
Script:
#!/bin/bash # Search for table-related styles and containers rg -g '*.{js,css}' -A 3 'table|Table' --type js | rg -A 3 'style|Style|styled' # Look for scroll/overflow related styles rg -g '*.{js,css}' -A 3 'overflow|scroll|height|maxHeight' --type jsLength of output: 73377
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/index.js (1)
19-19: LGTM! Clean import of the Attendance component.The import follows the established pattern of the codebase.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/IndividualUserTable.js (1)
9-38: Well-structured popup component!The SupervisorPopup component is well-implemented with proper internationalization, styling, and event handling.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/Module.js (1)
60-62: LGTM! Clean component registrationThe new components are properly registered following the existing pattern.
Also applies to: 181-183
| limit: 1000, | ||
| offset: 0, | ||
| tenantId, |
There was a problem hiding this comment.
Handle dynamic limits to ensure complete data retrieval
Hardcoding the limit to 1000 may lead to incomplete data if the number of staffUserIds or individualIds exceeds 1000. To ensure all records are fetched, consider setting the limit based on the size of the IDs set.
Apply the following changes:
// For staffResponse
params: {
- limit: 1000,
+ limit: staffUserIds.size || 1000,
offset: 0,
tenantId,
},
// For individualResponse
params: {
- limit: 1000,
+ limit: individualIds.size || 1000,
offset: 0,
tenantId,
},Also applies to: 83-85
|
|
||
| // Get staff user details | ||
| const staffResponse = await Digit.CustomService.getResponse({ | ||
| url: "/health-individual/v1/_search", | ||
| method: "POST", | ||
| params: { | ||
| limit: 1000, | ||
| offset: 0, | ||
| tenantId, | ||
| }, | ||
| body: { | ||
| Individual: { | ||
| id: Array.from(staffUserIds) | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
|
|
||
| // Get attendee details | ||
| const individualResponse = await Digit.CustomService.getResponse({ | ||
| url: "/health-individual/v1/_search", | ||
| method: "POST", | ||
| params: { | ||
| limit: 1000, | ||
| offset: 0, | ||
| tenantId, | ||
| }, | ||
| body: { | ||
| Individual: { | ||
| id: Array.from(individualIds) | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| // Create map of staff details | ||
| const userDetails = staffResponse?.Individual.map(user => { | ||
| // Find the register that contains this staff member | ||
| const registerWithStaff = registerResponse.attendanceRegister.find( | ||
| register => register.staff?.[0]?.userId === user.id | ||
| ); | ||
|
|
||
| return { | ||
| id: user.id, | ||
| name: user?.name?.givenName, | ||
| dob: user.dateOfBirth, | ||
| email: user?.email, | ||
| mobileNumber: user?.mobileNumber, | ||
| registerId: registerWithStaff?.staff?.[0]?.registerId || null // Add registerId here | ||
| }; | ||
| }); | ||
|
|
||
| // Create map of individual details | ||
| const attendeeDetailsMap = {}; | ||
| individualResponse?.Individual.forEach(individual => { | ||
| attendeeDetailsMap[individual.id] = { | ||
| name: individual?.name?.givenName, | ||
| dob: individual.dateOfBirth, | ||
| email: individual?.email, | ||
| mobileNumber: individual?.mobileNumber, | ||
| }; | ||
| }); | ||
|
|
||
| // Create a map for quick lookup of staff details | ||
| const staffDetailsMap = {}; | ||
| userDetails.forEach(user => { | ||
| staffDetailsMap[user.id] = user; | ||
| }); | ||
|
|
||
| // Enhance staffAttendeeMap with both staff and individual details | ||
| Object.keys(staffAttendeeMap).forEach(staffId => { | ||
| const enhancedStaffData = { | ||
| staffDetails: staffDetailsMap[staffId] || null, | ||
| attendees: staffAttendeeMap[staffId].map(attendee => ({ | ||
| ...attendee, | ||
| individualDetails: attendeeDetailsMap[attendee.individualId] || null | ||
| })) | ||
| }; | ||
| staffAttendeeMap[staffId] = enhancedStaffData; | ||
| }); | ||
|
|
||
| return { | ||
| registers: registerResponse.attendanceRegister, | ||
| staffAttendeeMap, | ||
| attendeeDetailsMap, | ||
| userDetails, | ||
| isError: false, | ||
| }; | ||
|
|
||
| } catch (error) { | ||
| console.error("Error in searchAttendanceWithUserDetails:", error); | ||
| return { | ||
| registers: [], | ||
| staffAttendeeMap: {}, | ||
| attendeeDetailsMap: {}, | ||
| userDetails: [], | ||
| isError: true, | ||
| error: error.message || "An unknown error occurred", | ||
| }; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Refactor the function for better readability and maintainability
The searchAttendanceWithUserDetails function is lengthy and handles multiple responsibilities. Breaking it into smaller functions will enhance readability and maintainability.
Consider refactoring as follows:
- Create a function to fetch the attendance register.
- Create functions to process and collect IDs.
- Create functions to fetch staff and attendee details.
- Create functions to map and enhance data.
| const userDetails = staffResponse?.Individual.map(user => { | ||
| // Find the register that contains this staff member | ||
| const registerWithStaff = registerResponse.attendanceRegister.find( | ||
| register => register.staff?.[0]?.userId === user.id | ||
| ); | ||
|
|
||
| return { | ||
| id: user.id, | ||
| name: user?.name?.givenName, | ||
| dob: user.dateOfBirth, | ||
| email: user?.email, | ||
| mobileNumber: user?.mobileNumber, | ||
| registerId: registerWithStaff?.staff?.[0]?.registerId || null // Add registerId here | ||
| }; | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Handle potential undefined responses gracefully
When mapping over staffResponse?.Individual, if Individual is undefined, it could cause errors. Ensure that you provide a default empty array.
Apply the following change:
- const userDetails = staffResponse?.Individual.map(user => {
+ const userDetails = (staffResponse?.Individual || []).map(user => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const userDetails = staffResponse?.Individual.map(user => { | |
| // Find the register that contains this staff member | |
| const registerWithStaff = registerResponse.attendanceRegister.find( | |
| register => register.staff?.[0]?.userId === user.id | |
| ); | |
| return { | |
| id: user.id, | |
| name: user?.name?.givenName, | |
| dob: user.dateOfBirth, | |
| email: user?.email, | |
| mobileNumber: user?.mobileNumber, | |
| registerId: registerWithStaff?.staff?.[0]?.registerId || null // Add registerId here | |
| }; | |
| }); | |
| const userDetails = (staffResponse?.Individual || []).map(user => { | |
| // Find the register that contains this staff member | |
| const registerWithStaff = registerResponse.attendanceRegister.find( | |
| register => register.staff?.[0]?.userId === user.id | |
| ); | |
| return { | |
| id: user.id, | |
| name: user?.name?.givenName, | |
| dob: user.dateOfBirth, | |
| email: user?.email, | |
| mobileNumber: user?.mobileNumber, | |
| registerId: registerWithStaff?.staff?.[0]?.registerId || null // Add registerId here | |
| }; | |
| }); |
| }; | ||
|
|
||
| } catch (error) { | ||
| console.error("Error in searchAttendanceWithUserDetails:", error); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Replace console.error with a proper logging mechanism
Using console.error in production can expose errors in the browser console. Consider using a logging service or removing it.
- console.error("Error in searchAttendanceWithUserDetails:", error);
+ // Replace or remove this line in production environmentCommittable suggestion skipped: line range outside the PR's diff.
| import React from 'react'; | ||
|
|
||
| const ProgressBar = ({ amount, total }) => { | ||
| const percentage = Math.min((amount / total) * 100, 100); |
There was a problem hiding this comment.
Handle division by zero when total is zero
If total is zero, dividing by zero will result in Infinity or NaN, causing rendering issues. Add a check to handle this case.
Apply this diff:
const ProgressBar = ({ amount, total }) => {
- const percentage = Math.min((amount / total) * 100, 100);
+ const percentage = total > 0 ? Math.min((amount / total) * 100, 100) : 0;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const percentage = Math.min((amount / total) * 100, 100); | |
| const ProgressBar = ({ amount, total }) => { | |
| const percentage = total > 0 ? Math.min((amount / total) * 100, 100) : 0; |
| {assignUsersPopup && ( | ||
| <PopUp | ||
| className="roleComposer" | ||
| type="default" | ||
| heading={t(`ASSIGN_USERS_TO_SUPERVISOR ${curSupervisor}`)} | ||
| children={[ | ||
| <IndividualUserTable | ||
| tenantId={tenantId} | ||
| staffAttendeeIds={curAttendeesList} | ||
| supervisorName={curSupervisor} | ||
| /> | ||
| ]} | ||
| onOverlayClick={() => setAssignUsersPopup(false)} | ||
| footerChildren={[ | ||
| <Button | ||
| type="button" | ||
| size="large" | ||
| variation="secondary" | ||
| label={t("CLOSE")} | ||
| onClick={() => setAssignUsersPopup(false)} | ||
| style={{ minWidth: "200px" }} | ||
| /> | ||
| ]} | ||
| sortFooterChildren={true} | ||
| onClose={() => setAssignUsersPopup(false)} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add missing keys to PopUp children
React elements in arrays or iterables should have unique keys.
children={[
<IndividualUserTable
+ key="individual-user-table"
tenantId={tenantId}
staffAttendeeIds={curAttendeesList}
supervisorName={curSupervisor}
/>
]}
footerChildren={[
<Button
+ key="close-button"
type="button"
size="large"
variation="secondary"
label={t("CLOSE")}
onClick={() => setAssignUsersPopup(false)}
style={{ minWidth: "200px" }}
/>
]}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {assignUsersPopup && ( | |
| <PopUp | |
| className="roleComposer" | |
| type="default" | |
| heading={t(`ASSIGN_USERS_TO_SUPERVISOR ${curSupervisor}`)} | |
| children={[ | |
| <IndividualUserTable | |
| tenantId={tenantId} | |
| staffAttendeeIds={curAttendeesList} | |
| supervisorName={curSupervisor} | |
| /> | |
| ]} | |
| onOverlayClick={() => setAssignUsersPopup(false)} | |
| footerChildren={[ | |
| <Button | |
| type="button" | |
| size="large" | |
| variation="secondary" | |
| label={t("CLOSE")} | |
| onClick={() => setAssignUsersPopup(false)} | |
| style={{ minWidth: "200px" }} | |
| /> | |
| ]} | |
| sortFooterChildren={true} | |
| onClose={() => setAssignUsersPopup(false)} | |
| /> | |
| )} | |
| {assignUsersPopup && ( | |
| <PopUp | |
| className="roleComposer" | |
| type="default" | |
| heading={t(`ASSIGN_USERS_TO_SUPERVISOR ${curSupervisor}`)} | |
| children={[ | |
| <IndividualUserTable | |
| key="individual-user-table" | |
| tenantId={tenantId} | |
| staffAttendeeIds={curAttendeesList} | |
| supervisorName={curSupervisor} | |
| /> | |
| ]} | |
| onOverlayClick={() => setAssignUsersPopup(false)} | |
| footerChildren={[ | |
| <Button | |
| key="close-button" | |
| type="button" | |
| size="large" | |
| variation="secondary" | |
| label={t("CLOSE")} | |
| onClick={() => setAssignUsersPopup(false)} | |
| style={{ minWidth: "200px" }} | |
| /> | |
| ]} | |
| sortFooterChildren={true} | |
| onClose={() => setAssignUsersPopup(false)} | |
| /> | |
| )} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 311-311: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 312-316: 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)
[error] 320-327: 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)
| case "ACTION_LABEL_CONFIGURE_REGISTER": | ||
| window.history.pushState( | ||
| { | ||
| name: row?.campaignName, | ||
| data: row, | ||
| projectId: row?.projectId, | ||
| }, | ||
| "", | ||
| `/${window.contextPath}/employee/campaign/attendance` | ||
| ); | ||
| const navEvent2 = new PopStateEvent("popstate"); | ||
| window.dispatchEvent(navEvent2); | ||
| break; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Wrap switch declarations in blocks and consider reducing duplication
- Switch declarations should be wrapped in blocks to prevent variable hoisting issues.
- The register configuration code is duplicated between MyCampaignConfigOngoing and MyCampaignConfigUpcoming.
case "ACTION_LABEL_CONFIGURE_REGISTER": {
+ {
window.history.pushState(
{
name: row?.campaignName,
data: row,
projectId: row?.projectId,
},
"",
`/${window.contextPath}/employee/campaign/attendance`
);
const navEvent2 = new PopStateEvent("popstate");
window.dispatchEvent(navEvent2);
break;
+ }
}Consider extracting the common navigation logic into a shared function to reduce code duplication.
Also applies to: 816-828
🧰 Tools
🪛 Biome (1.9.4)
[error] 477-477: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
| display: flex; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider enhancing the attendance cell layout.
The flex container could benefit from additional properties for better layout control and responsiveness.
.attendanceCell {
display: flex;
+ align-items: center;
+ gap: 1rem;
+ flex-wrap: wrap;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .attendanceCell { | |
| display: flex; | |
| } | |
| .attendanceCell { | |
| display: flex; | |
| align-items: center; | |
| gap: 1rem; | |
| flex-wrap: wrap; | |
| } |
| .chip-container { | ||
| display: flex; | ||
| align-items: center; | ||
| gap: 0.5rem; /* Adjust the spacing between chips and the button */ | ||
| } | ||
|
|
||
| .chip-wrapper { | ||
| display: inline-flex; | ||
| } | ||
|
|
||
| .more-button { | ||
| margin-left: 0.5rem; /* Adjust the spacing between chips and the + button */ | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM! Consider adding interactive states.
The chip container implementation is well-structured with appropriate spacing and alignment.
Consider adding hover and focus states for better interactivity:
.chip-wrapper {
display: inline-flex;
+ transition: transform 0.2s ease;
+ &:hover {
+ transform: translateY(-1px);
+ }
}
.more-button {
margin-left: 0.5rem;
+ &:hover {
+ opacity: 0.8;
+ }
+ &:focus {
+ outline: 2px solid #1969ff;
+ outline-offset: 2px;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .chip-container { | |
| display: flex; | |
| align-items: center; | |
| gap: 0.5rem; /* Adjust the spacing between chips and the button */ | |
| } | |
| .chip-wrapper { | |
| display: inline-flex; | |
| } | |
| .more-button { | |
| margin-left: 0.5rem; /* Adjust the spacing between chips and the + button */ | |
| } | |
| .chip-container { | |
| display: flex; | |
| align-items: center; | |
| gap: 0.5rem; /* Adjust the spacing between chips and the button */ | |
| } | |
| .chip-wrapper { | |
| display: inline-flex; | |
| transition: transform 0.2s ease; | |
| &:hover { | |
| transform: translateY(-1px); | |
| } | |
| } | |
| .more-button { | |
| margin-left: 0.5rem; /* Adjust the spacing between chips and the + button */ | |
| &:hover { | |
| opacity: 0.8; | |
| } | |
| &:focus { | |
| outline: 2px solid #1969ff; | |
| outline-offset: 2px; | |
| } | |
| } |
| /* Container for the progress bar */ | ||
| .progress-bar-container { | ||
| position: relative; | ||
| width: 20rem; | ||
| max-width: 20rem; | ||
| height: 20px; | ||
| background-color: #f3f3f3; | ||
| border-radius: 10px; | ||
| overflow: hidden; | ||
| border: 1px solid #d6d5d4; | ||
| } | ||
|
|
||
| /* The fill of the progress bar */ | ||
| .progress-bar-fill { | ||
| height: 100%; | ||
| background-color: #2BD27F; /* Use your PRIMARY_COLOR */ | ||
| transition: width 0.4s ease-in-out; | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| } | ||
|
|
||
| /* Text inside the progress bar */ | ||
| .progress-bar-text { | ||
| position: absolute; | ||
| width: 100%; | ||
| text-align: center; | ||
| font-size: 14px; | ||
| color: #fff; | ||
| font-weight: bold; | ||
| pointer-events: none; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance progress bar theming and accessibility.
The progress bar implementation is well-structured, but could benefit from using theme variables and improved accessibility.
.progress-bar-container {
position: relative;
width: 20rem;
max-width: 20rem;
height: 20px;
- background-color: #f3f3f3;
+ background-color: var(--progress-bar-bg, #f3f3f3);
border-radius: 10px;
overflow: hidden;
- border: 1px solid #d6d5d4;
+ border: 1px solid var(--border-color, #d6d5d4);
}
.progress-bar-fill {
height: 100%;
- background-color: #2BD27F; /* Use your PRIMARY_COLOR */
+ background-color: var(--primary-color, #2BD27F);
transition: width 0.4s ease-in-out;
display: flex;
align-items: center;
justify-content: center;
}
.progress-bar-text {
position: absolute;
width: 100%;
text-align: center;
font-size: 14px;
- color: #fff;
+ color: var(--text-on-primary, #fff);
font-weight: bold;
pointer-events: none;
+ /* Ensure text remains readable on any background */
+ text-shadow: 0 0 2px rgba(0, 0, 0, 0.5);
+ /* Add aria attributes in the component */
+ &[aria-valuenow] {
+ /* Styles when progress value is available */
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /* Container for the progress bar */ | |
| .progress-bar-container { | |
| position: relative; | |
| width: 20rem; | |
| max-width: 20rem; | |
| height: 20px; | |
| background-color: #f3f3f3; | |
| border-radius: 10px; | |
| overflow: hidden; | |
| border: 1px solid #d6d5d4; | |
| } | |
| /* The fill of the progress bar */ | |
| .progress-bar-fill { | |
| height: 100%; | |
| background-color: #2BD27F; /* Use your PRIMARY_COLOR */ | |
| transition: width 0.4s ease-in-out; | |
| display: flex; | |
| align-items: center; | |
| justify-content: center; | |
| } | |
| /* Text inside the progress bar */ | |
| .progress-bar-text { | |
| position: absolute; | |
| width: 100%; | |
| text-align: center; | |
| font-size: 14px; | |
| color: #fff; | |
| font-weight: bold; | |
| pointer-events: none; | |
| } | |
| /* Container for the progress bar */ | |
| .progress-bar-container { | |
| position: relative; | |
| width: 20rem; | |
| max-width: 20rem; | |
| height: 20px; | |
| background-color: var(--progress-bar-bg, #f3f3f3); | |
| border-radius: 10px; | |
| overflow: hidden; | |
| border: 1px solid var(--border-color, #d6d5d4); | |
| } | |
| /* The fill of the progress bar */ | |
| .progress-bar-fill { | |
| height: 100%; | |
| background-color: var(--primary-color, #2BD27F); | |
| transition: width 0.4s ease-in-out; | |
| display: flex; | |
| align-items: center; | |
| justify-content: center; | |
| } | |
| /* Text inside the progress bar */ | |
| .progress-bar-text { | |
| position: absolute; | |
| width: 100%; | |
| text-align: center; | |
| font-size: 14px; | |
| color: var(--text-on-primary, #fff); | |
| font-weight: bold; | |
| pointer-events: none; | |
| /* Ensure text remains readable on any background */ | |
| text-shadow: 0 0 2px rgba(0, 0, 0, 0.5); | |
| /* Add aria attributes in the component */ | |
| &[aria-valuenow] { | |
| /* Styles when progress value is available */ | |
| } | |
| } |
Choose the appropriate template for your PR:
Feature PR
Feature Request
JIRA ID
Module
Description
Related Issues
Bugfix PR
Bugfix Request
JIRA ID
Module
Description
Root Cause
Related Issues
Release PR
Summary by CodeRabbit
New Features
Styling
Performance