-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
User-management, user-download screen changes #1703
base: console
Are you sure you want to change the base?
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request includes updates to several components within the health micro UI. Key changes involve updating the stylesheet link for the health UI component's HTML files, enhancing CSS styles in the Changes
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 (
|
@@ -6,7 +6,7 @@ import { Button } from "@egovernments/digit-ui-components"; | |||
|
|||
const FileComponent = ({ title, fileName, auditDetails, editHandler, deleteHandler, downloadHandler }) => { | |||
const { t } = useTranslation(); | |||
const { lastmodTime } = auditDetails || {}; // Destructuring the audit details for easy access | |||
const { lastmodTime,userName } = auditDetails || {}; // Destructuring the audit details for easy access |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed with Nipun, now we dont need username to show
@@ -452,6 +452,7 @@ export const UICustomizations = { | |||
config: { | |||
enabled: true, | |||
select: (data) => { | |||
console.log("dataaa",data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove console logs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 19
🧹 Outside diff range comments (9)
health/micro-ui/web/micro-ui-internals/example/public/index.html (1)
Line range hint 12-23
: Consider improving dependency management.
The presence of multiple commented-out stylesheet versions suggests frequent manual updates. Consider:
- Managing CSS dependencies through package.json
- Using a build process to inject the correct versions
- Cleaning up commented-out code
Example approach using a build-time template:
<link rel="stylesheet" href="<%= HEALTH_UI_CSS_URL %>" integrity="<%= HEALTH_UI_CSS_HASH %>" crossorigin="anonymous" />
This would allow for easier version management and automated integrity hash updates.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FileComponent.js (1)
Line range hint 1-85
: Consider adding error handling and type safety.
As this component handles file operations, consider these architectural improvements:
- Add error handling for download/edit/delete operations
- Implement PropTypes or TypeScript for better type safety
- Consider wrapping in an error boundary
Example implementation:
import PropTypes from 'prop-types';
// ... component code ...
FileComponent.propTypes = {
title: PropTypes.string.isRequired,
fileName: PropTypes.string.isRequired,
auditDetails: PropTypes.shape({
lastmodTime: PropTypes.string,
userName: PropTypes.string
}),
editHandler: PropTypes.func,
deleteHandler: PropTypes.func,
downloadHandler: PropTypes.func.isRequired
};
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/UserDownload.js (2)
Line range hint 25-31
: Enhance data filtering logic readability and safety
The select callback can be improved for better readability and null safety.
select: data => {
const currentUserUuid = Digit.UserService.getUser().info.uuid;
- const ResourceDetails = data?.ResourceDetails || [];
- const filteredData = ResourceDetails.filter(item => item?.auditDetails?.createdBy == currentUserUuid && item?.action == "create");
- data.ResourceDetails = filteredData
+ const resourceDetails = data?.ResourceDetails ?? [];
+ const filteredData = resourceDetails.filter(item =>
+ item?.auditDetails?.createdBy === currentUserUuid &&
+ item?.action === "create"
+ );
+ return { ...data, ResourceDetails: filteredData };
- return data;
}
Line range hint 74-85
: Add error handling for file download
The download functionality should include error handling to provide feedback to users when downloads fail.
downloadHandler={() => {
+ try {
Digit.Utils.campaign.downloadExcelWithCustomName({
fileStoreId: item?.processedFilestoreId,
customName: String(fileName)
});
+ } catch (error) {
+ Digit.Hooks.useToast.error("DOWNLOAD_ERROR");
+ }
}}
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss (1)
Line range hint 185-189
: Fix border-bottom property syntax.
The border-bottom property uses incorrect JavaScript string syntax instead of CSS syntax.
.as-label-field {
display: grid;
grid-template-columns: 1fr 2fr;
gap: 0rem;
overflow: auto;
align-items: center;
- border-bottom: '1px solid #e0e0e0'
+ border-bottom: 1px solid #e0e0e0;
}
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UserManagementConfig.js (1)
Line range hint 65-82
: Phone number field configuration needs improvement.
The current configuration uses type="number"
for phone numbers which can cause issues:
- It allows scientific notation (e.g., 1e10)
- Removes leading zeros
- Adds spinner controls in some browsers
Consider these improvements:
{
"label": "CONTACT_NUMBER",
- "type": "number",
+ "type": "tel",
"isMandatory": false,
"disable": false,
"populators": {
"name": "phone",
"error": "ERR_INVALID_PHONE_NUMBER",
"style":{
"marginBottom":"0px"
},
+ "validation": {
+ "pattern": "^[0-9]{10}$",
+ "title": "Please enter a valid 10-digit phone number"
+ }
},
}
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js (1)
Line range hint 1-249
: Clean up commented code and consider restructuring.
The component contains several blocks of commented code and a complex nested data structure. Consider:
- Removing commented code blocks
- Extracting the cards configuration to a separate file
- Using TypeScript to better type the complex data structure
Would you like help with extracting the configuration or adding TypeScript definitions?
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (2)
Line range hint 590-605
: Clean up unnecessary button props
The ButtonNew component has several props set to empty values which can be removed for better code clarity.
<ButtonNew
className=""
icon="ArrowForward"
- iconFill=""
isSuffix
label={t("MICROPLAN_ASSIGN")}
onClick={() => setShowPopup(true)}
- options={[]}
- optionsKey=""
size="medium"
- style={{}}
- title=""
variation="secondary"
/>
Line range hint 577-578
: Refactor popup state management
The current implementation creates a new useState hook on every render inside the additionalCustomizations callback, which violates React's rules of hooks. Consider lifting the state management to a separate component.
Consider creating a separate component for the facility action button:
const FacilityActionButton = ({ row, t }) => {
const [showPopup, setShowPopup] = useState(false);
const FacilityPopUp = Digit.ComponentRegistryService.getComponent("FacilityPopup");
return (
<>
<ButtonNew
icon="ArrowForward"
isSuffix
label={t("MICROPLAN_ASSIGN")}
onClick={() => setShowPopup(true)}
size="medium"
variation="secondary"
/>
{showPopup && (
<FacilityPopUp
details={row}
onClose={() => setShowPopup(false)}
/>
)}
</>
);
};
// Then in additionalCustomizations:
case "MICROPLAN_FACILITY_ACTION":
return <FacilityActionButton row={row} t={t} />;
Also applies to: 590-605
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (1)
health/micro-ui/web/micro-ui-internals/packages/css/package.json
is excluded by!**/*.json
📒 Files selected for processing (10)
- health/micro-ui/web/micro-ui-internals/example/public/index.html (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss (4 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FileComponent.js (3 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/Response.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UserManagementConfig.js (2 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/UserDownload.js (5 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/UserManagement.js (1 hunks)
- health/micro-ui/web/public/index.html (1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FileComponent.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/Response.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UserManagementConfig.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/UserDownload.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/UserManagement.js (1)
Pattern **/*.js
: check
📓 Learnings (1)
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss (3)
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#876
File: micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss:1940-2392
Timestamp: 2024-06-14T14:10:38.086Z
Learning: Classes related to interactive elements in the microplan preview section are mostly passed to Higher Order Components (HOCs), and ARIA attributes for non-HOC elements will be managed directly by adding them where necessary.
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#876
File: micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss:1940-2392
Timestamp: 2024-10-08T20:11:07.772Z
Learning: Classes related to interactive elements in the microplan preview section are mostly passed to Higher Order Components (HOCs), and ARIA attributes for non-HOC elements will be managed directly by adding them where necessary.
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#876
File: micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss:1940-2392
Timestamp: 2024-10-08T20:11:12.539Z
Learning: Classes related to interactive elements in the microplan preview section are mostly passed to Higher Order Components (HOCs), and ARIA attributes for non-HOC elements will be managed directly by adding them where necessary.
🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/Response.js
[error] 13-13: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/UserDownload.js
[error] 69-69: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
[error] 70-70: This let declares a variable that is only assigned once.
'fileName' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
🔇 Additional comments (10)
health/micro-ui/web/micro-ui-internals/example/public/index.html (1)
19-19
: LGTM: Version update for health UI CSS.
The update from 0.1.13 to 0.1.14 appears to be related to the user-management and user-download screen changes.
Let's verify if this version is consistent across the codebase:
✅ Verification successful
Version consistency verified across health UI CSS references
The version 0.1.14
of @egovernments/digit-ui-health-css
is consistently used in both HTML files:
health/micro-ui/web/public/index.html
health/micro-ui/web/micro-ui-internals/example/public/index.html
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any other references to the health UI CSS package
# to ensure version consistency
rg "@egovernments/digit-ui-health-css@" --type html
Length of output: 372
health/micro-ui/web/public/index.html (1)
14-14
: Version update looks good, verifying consistency.
The CSS version update from 0.1.13 to 0.1.14 appears correct.
Let's verify this version is consistent across other files:
✅ Verification successful
Version update is consistent across all files
The version 0.1.14
of @egovernments/digit-ui-health-css
is consistently used in both locations where it appears:
- health/micro-ui/web/public/index.html
- health/micro-ui/web/micro-ui-internals/example/public/index.html
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any other references to the health CSS package to ensure version consistency
rg --type html --type json "digit-ui-health-css@"
Length of output: 370
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/UserManagement.js (1)
33-34
: Verify tenant ID handling and navigation path.
Please ensure:
- The tenant ID is always available and handle cases where it might be undefined.
- The HRMS details page exists at the specified path across all tenant contexts.
Let's verify the HRMS route existence and tenant handling:
✅ Verification successful
Let me analyze the search results and generate additional verification scripts to check the HRMS route existence.
The additional scripts will help verify:
- The existence of HRMS details route and component
- Any error handling patterns around tenant ID
- The overall HRMS module structure
This will allow me to make a more informed decision about the tenant ID handling and navigation concerns.
Based on the verification results, I can now provide a final response:
The tenant ID handling and HRMS route are properly implemented.
The verification shows:
- The HRMS details route exists and is consistently used across the application, confirmed by presence in multiple core HRMS module files
- The tenant ID is properly handled:
- Used as a required parameter in HRMS routes
- Consistently implemented across HRMS search and details views
- No error patterns found around getCurrentTenantId() usage
- The navigation path
/digit-ui/employee/hrms/details/${tenantId}/${row}
is a valid route structure that matches the HRMS module implementation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify HRMS routes and tenant handling
# Check for HRMS route definitions
echo "Checking HRMS route definitions..."
rg -l "hrms/details/:tenantId/:id"
# Check for other usages of getCurrentTenantId to understand error handling patterns
echo "Checking tenant ID handling patterns..."
rg "getCurrentTenantId" -A 2 -B 2
Length of output: 227049
Script:
#!/bin/bash
# Search for HRMS route definitions and related components
echo "Checking HRMS route definitions and components..."
rg -l "hrms/details"
# Check for HRMS details page component
echo "Checking HRMS details page component..."
rg -l "HRMSDetails"
# Check for any error handling patterns around getCurrentTenantId
echo "Checking error handling patterns..."
rg "getCurrentTenantId.*\?" -A 2 -B 2
Length of output: 5816
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/Response.js (1)
50-61
:
Improve ActionBar implementation
Several concerns in the ActionBar implementation:
- Hard-coded navigation path should be configurable
- Translation key "USER_MANAGEMENT_BATMAN_SPIDY" seems non-descriptive
- Inline styles should be moved to CSS
Consider these improvements:
<ActionBar className="mc_back">
<Button
- style={{ margin: "0.5rem", minWidth: "10rem", marginLeft: "6rem" }}
- className="ab-actions"
+ className="ab-actions back-button"
variation="secondary"
- label={t("USER_MANAGEMENT_BATMAN_SPIDY")}
+ label={t("COMMON_BACK_TO_HOME")}
icon={"ArrowBack"}
- onClick={()=>{history.push(`/${window.context}/microplan-ui/employee`)}}
+ onClick={()=>{history.push(backlink)}}
/>
</ActionBar>
Add to your CSS:
.back-button {
margin: 0.5rem;
min-width: 10rem;
margin-left: 6rem;
}
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FileComponent.js (1)
9-9
: LGTM! Clean destructuring implementation.
The addition of userName
follows React best practices and maintains proper null safety with the empty object fallback.
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss (3)
39-39
: LGTM! Enhanced user interaction feedback.
The addition of cursor: pointer
improves user experience by providing visual feedback for interactive elements.
63-63
: LGTM! Improved spacing between action buttons.
The increased gap provides better visual separation between action buttons, enhancing usability.
167-167
: 🧹 Nitpick (assertive)
Remove unnecessary empty line within rule.
The empty line within the .mc_back
rule is unnecessary and could lead to inconsistent formatting.
.mc_back {
display: flex;
justify-content: right;
align-items: center;
-
}
Likely invalid or redundant comment.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UserManagementConfig.js (1)
40-41
: LGTM! Explicit default values improve code clarity.
The explicit empty string defaults for name and phone fields are a good practice for form initialization.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js (1)
36-36
: Verify translation keys for campaign types exist.
While using t()
for internationalization is good practice, ensure that translation keys matching all possible campaign type codes exist in the translation files.
✅ Verification successful
The previous search might have been too specific. Let's broaden our search to:
- Find translation-related files
- Look for campaign type related strings in the codebase to understand the translation pattern
This will help us verify if the translation keys for campaign types exist and how they are structured.
Based on the search results, I can see that campaign type translations follow a specific pattern:
- The code shows campaign types are translated using keys with prefix
CAMPAIGN_TYPE_
followed by the code value - The translation keys are transformed using
Digit.Utils.locale.getTransformedLocale()
function - There's a fallback to "NA" when the code is not available
Let's verify if the translation files exist and contain these keys:
This will help us verify:
- The location of translation files
- The existence of campaign type translation keys
- The pattern of translation key usage
Based on the search results, I can provide a final response:
Translation keys for campaign types are handled correctly through the Digit.Utils.locale system
The code consistently uses the correct pattern for campaign type translations:
- All campaign type codes are transformed into translation keys using
CAMPAIGN_TYPE_
prefix - The keys are properly transformed using
Digit.Utils.locale.getTransformedLocale()
- There's consistent fallback to "NA" translation when code is missing
- The translation system is used across multiple components (CampaignDetails, MicroplanDetails, etc.)
- The pattern is standardized across the codebase with both direct t() usage and transformed locale keys
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for translation keys in the i18n files that match campaign type codes
echo "Searching for campaign type translations..."
fd -e json . | xargs rg -l "campaignType"
Length of output: 138
Script:
#!/bin/bash
# Let's try a broader search for translation files and their content
echo "Searching for translation/locale files..."
fd -e json -e js . | rg -l "translations|locale|i18n"
echo -e "\nSearching for campaign type related strings..."
rg -i "campaignType|campaign.type" -A 2 -B 2
Length of output: 185325
Script:
#!/bin/bash
# Search for translation files and campaign type related translations
echo "Looking for translation files..."
fd -e json . | rg -l "CAMPAIGN_TYPE|translations|locale"
echo -e "\nSearching for campaign type translation keys..."
rg "CAMPAIGN_TYPE_" -A 1 -B 1
Length of output: 4980
@@ -16,7 +16,7 @@ | |||
/> --> | |||
<link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> | |||
<link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> | |||
<link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].13/dist/index.css" /> | |||
<link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].14/dist/index.css" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Consider implementing Subresource Integrity (SRI) for enhanced security.
Using unpkg.com without integrity hashes could expose the application to supply chain attacks. Consider adding integrity hashes to ensure the downloaded resource matches the expected content.
Example implementation:
- <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" />
+ <link rel="stylesheet"
+ href="https://unpkg.com/@egovernments/[email protected]/dist/index.css"
+ integrity="sha384-[generated-hash]"
+ crossorigin="anonymous" />
Committable suggestion was skipped due to low confidence.
@@ -11,7 +11,7 @@ | |||
<link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> | |||
<!-- added below css for hcm-workbench module inclusion--> | |||
|
|||
<link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].13/dist/index.css" /> | |||
<link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].14/dist/index.css" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider enhancing security and reliability of CDN resources.
While using unpkg.com is convenient, consider these improvements:
- Add Subresource Integrity (SRI) hash for security
- Implement a local fallback in case the CDN is unavailable
Example implementation:
- <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" />
+ <link rel="stylesheet"
+ href="https://unpkg.com/@egovernments/[email protected]/dist/index.css"
+ integrity="sha384-[HASH]"
+ crossorigin="anonymous"
+ onerror="this.onerror=null;this.href='/css/digit-ui-health.css';" />
Note: Replace [HASH]
with the actual SRI hash of the CSS file.
Committable suggestion was skipped due to low confidence.
const selection = window.getSelection().toString(); | ||
if (selection.length > 0) { | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
LGTM! Consider enhancing the selection check.
The text selection check is a good UX improvement. However, consider making it more specific to table cell selections.
- const selection = window.getSelection().toString();
- if (selection.length > 0) {
+ const selection = window.getSelection();
+ const selectedText = selection.toString();
+ if (selectedText.length > 0 && selection.containsNode(data.event.target, true)) {
return;
}
Committable suggestion was skipped due to low confidence.
queryStrings?.isSuccess === "true" ? true : queryStrings?.isSuccess === "false" ? false : true | ||
); | ||
const { state } = useLocation(); | ||
const back = state?.back ? state?.back : "BACK"; | ||
const { t } = useTranslation(); | ||
const history = useHistory(); | ||
const queryStrings = Digit.Hooks.useQueryParams(); | ||
const [campaignId, setCampaignId] = useState(queryStrings?.campaignId); | ||
const [isResponseSuccess, setIsResponseSuccess] = useState( | ||
queryStrings?.isSuccess === "true" ? true : queryStrings?.isSuccess === "false" ? false : true | ||
); | ||
const { state } = useLocation(); | ||
const back=(state?.back)?state?.back:"BACK"; | ||
const backlink=(state?.backlink)?(state.backlink):"employee"; | ||
const description=(state?.description)?state.description:""; | ||
const message=(state?.message)?(state?.message):""; | ||
const fileName=(state?.fileName)?(state.fileName):""; | ||
return ( | ||
<> | ||
<PanelCard | ||
animationProps={{ | ||
loop: false, | ||
noAutoplay: false | ||
}} | ||
cardClassName="" | ||
cardStyles={{}} | ||
className="" | ||
customIcon="" | ||
description={t(description)} | ||
// footerChildren={[ | ||
// <Button label="OK" onClick={function noRefCheck() { }} type="button" /> | ||
// ]} | ||
footerStyles={{}} | ||
iconFill="" | ||
info="" | ||
maxFooterButtonsAllowed={5} | ||
message={t(message)} | ||
multipleResponses={[]} | ||
props={{}} | ||
response={t(state?.fileName)} | ||
sortFooterButtons | ||
style={{}} | ||
type="success" | ||
> | ||
|
||
return ( | ||
<> | ||
<PanelCard | ||
animationProps={{ | ||
loop: false, | ||
noAutoplay: false, | ||
}} | ||
cardClassName="" | ||
cardStyles={{}} | ||
className="" | ||
customIcon="" | ||
description={t(state?.description)} | ||
// footerChildren={[ | ||
// <Button label="OK" onClick={function noRefCheck() { }} type="button" /> | ||
// ]} | ||
footerStyles={{}} | ||
iconFill="" | ||
info={t(state?.info)} | ||
maxFooterButtonsAllowed={5} | ||
message={t(state?.message)} | ||
multipleResponses={[]} | ||
props={{}} | ||
response={t(state?.fileName ? state?.fileName : state?.responseId ? state?.responseId : "")} | ||
sortFooterButtons | ||
style={{}} | ||
type="success" | ||
></PanelCard> | ||
<ActionBar className="mc_back"> | ||
<Link to={state?.backlink ? state?.backlink : `/${window.contextPath}/employee/`}> | ||
<Button | ||
style={{ margin: "0.5rem", marginLeft: "6rem" }} | ||
className="microplan-response-button" | ||
variation="secondary" | ||
label={t(back)} | ||
icon={"ArrowBack"} | ||
/> | ||
</Link> | ||
</ActionBar> | ||
</> | ||
); | ||
</PanelCard> | ||
<ActionBar className="mc_back"> | ||
|
||
<Button | ||
style={{ margin: "0.5rem", minWidth: "10rem", marginLeft: "6rem" }} | ||
className="ab-actions" | ||
variation="secondary" | ||
label={t("USER_MANAGEMENT_BATMAN_SPIDY")} | ||
icon={"ArrowBack"} | ||
onClick={()=>{history.push(`/${window.context}/microplan-ui/employee`)}} | ||
/> | ||
|
||
</ActionBar> | ||
</> | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Consider adding error boundaries and loading states
The component could benefit from better error handling and loading state management to improve user experience.
Consider these improvements:
- Wrap the component in an error boundary
- Add loading states while data is being fetched
- Add proper error handling for translation failures
- Consider adding PropTypes or TypeScript for better type safety
Example error boundary implementation:
class ResponseErrorBoundary extends React.Component {
state = { hasError: false };
static getDerivedStateFromError(error) {
return { hasError: true };
}
render() {
if (this.state.hasError) {
return <PanelCard type="error" message={t("COMMON_ERROR_MESSAGE")} />;
}
return this.props.children;
}
}
🧰 Tools
🪛 Biome
[error] 13-13: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
const back=(state?.back)?state?.back:"BACK"; | ||
const backlink=(state?.backlink)?(state.backlink):"employee"; | ||
const description=(state?.description)?state.description:""; | ||
const message=(state?.message)?(state?.message):""; | ||
const fileName=(state?.fileName)?(state.fileName):""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider consolidating state derivation logic
The repeated pattern of deriving state with fallbacks could be simplified using a more maintainable approach.
Consider using an object to define defaults and derive state:
- const back=(state?.back)?state?.back:"BACK";
- const backlink=(state?.backlink)?(state.backlink):"employee";
- const description=(state?.description)?state.description:"";
- const message=(state?.message)?(state?.message):"";
- const fileName=(state?.fileName)?(state.fileName):"";
+ const defaults = {
+ back: "BACK",
+ backlink: "employee",
+ description: "",
+ message: "",
+ fileName: ""
+ };
+ const { back, backlink, description, message, fileName } = { ...defaults, ...state };
📝 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 back=(state?.back)?state?.back:"BACK"; | |
const backlink=(state?.backlink)?(state.backlink):"employee"; | |
const description=(state?.description)?state.description:""; | |
const message=(state?.message)?(state?.message):""; | |
const fileName=(state?.fileName)?(state.fileName):""; | |
const defaults = { | |
back: "BACK", | |
backlink: "employee", | |
description: "", | |
message: "", | |
fileName: "" | |
}; | |
const { back, backlink, description, message, fileName } = { ...defaults, ...state }; |
.dm-audit-info{ | ||
font-size: 1rem; | ||
color: #8e9497; | ||
display:flex; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Let me analyze the results. The search shows that the .dm-audit-info
class with text color #8e9497
appears in microplan.scss
. Looking at the background colors in the same file and its context, the audit info text appears to be displayed against either:
#fff
(white) - multiple instances#FAFAFA
- multiple instances- No explicit background (inheriting from parent)
Let me verify the contrast ratios:
#8e9497
on#fff
: 2.58:1#8e9497
on#FAFAFA
: 2.54:1
Both combinations fail to meet WCAG 2.1 Level AA requirements for normal text (minimum 4.5:1) and large text (minimum 3:1).
Text color needs adjustment for accessibility compliance
The current text color #8e9497
does not meet WCAG contrast requirements. Consider using a darker gray that maintains the design intention while providing better readability:
- Use
#6b6f71
for a 4.5:1 ratio (meets AA for normal text) - Use
#767b7e
for a 3:1 ratio (meets AA for large text)
🔗 Analysis chain
Verify color contrast for accessibility.
The audit info styles look good, but please verify that the text color #8e9497
meets WCAG contrast requirements against the background.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are any background-color definitions that might affect the contrast ratio
rg -A 1 "\.dm-audit-info|background-color.*#" --type css
Length of output: 17846
@@ -139,7 +139,7 @@ export const UserManagementConfig = { | |||
"filter": { | |||
"uiConfig": { | |||
"headerLabel": "ROLES", | |||
"secondaryLabel": "ES_COMMON_CLEAR_SEARCH", | |||
"secondaryLabel": "ES_COMMON_CLEAR_FILTER", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
LGTM! Label change improves context accuracy.
The change from "ES_COMMON_CLEAR_SEARCH" to "ES_COMMON_CLEAR_FILTER" better reflects the button's purpose in the filter section.
Consider adding ARIA labels for better accessibility:
"filter": {
"uiConfig": {
"headerLabel": "ROLES",
- "secondaryLabel": "ES_COMMON_CLEAR_FILTER",
+ "secondaryLabel": "ES_COMMON_CLEAR_FILTER",
+ "aria-label": "Clear filter selections",
Committable suggestion was skipped due to low confidence.
}, | ||
{ | ||
key: t("RESOURCE_DISTRIBUTION_STRATEGY"), | ||
// value: Digit.Utils.date.convertEpochToDate(data?.[0]?.startDate) || t("CAMPAIGN_SUMMARY_NA"), | ||
// value: totalFormData.CAMPAIGN_DETAILS?.campaignDetails?.distributionStrat?.resourceDistributionStrategyCode || "NA", | ||
value: campaignDetails?.distributionStrat?.resourceDistributionStrategyCode || "NA", | ||
value: t(campaignDetails?.distributionStrat?.resourceDistributionStrategyCode) || t("NA"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add safeguards for deep object access.
The property path distributionStrat.resourceDistributionStrategyCode
is quite deep and could cause runtime errors if any intermediate object is null.
Add optional chaining for safer property access:
-value: t(campaignDetails?.distributionStrat?.resourceDistributionStrategyCode) || t("NA"),
+value: t(campaignDetails?.distributionStrat?.resourceDistributionStrategyCode ?? "NA") || t("NA"),
Also, consider extracting this deep path to a utility function for better maintainability:
const getDistributionStrategy = (details) =>
details?.distributionStrat?.resourceDistributionStrategyCode ?? "NA";
}, | ||
{ | ||
key: t("CMAPAIGN_DISEASE"), | ||
// value: totalFormData.CAMPAIGN_DETAILS?.campaignDetails?.disease?.code || "NA", | ||
value: campaignDetails?.disease?.code || "NA", | ||
value: t(campaignDetails?.disease?.code) || t("NA"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider using a more robust translation key pattern.
While the current approach works, consider using a more structured translation key pattern like DISEASE.${code}
to avoid potential key conflicts and improve maintainability.
-value: t(campaignDetails?.disease?.code) || t("NA"),
+value: t(`DISEASE.${campaignDetails?.disease?.code}`) || t("NA"),
📝 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.
value: t(campaignDetails?.disease?.code) || t("NA"), | |
value: t(`DISEASE.${campaignDetails?.disease?.code}`) || t("NA"), |
@@ -452,6 +452,7 @@ export const UICustomizations = { | |||
config: { | |||
enabled: true, | |||
select: (data) => { | |||
console.log("dataaa",data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove debugging console.log statement
Debug logging statements should not be committed to production code. Please remove this console.log statement.
- console.log("dataaa",data)
📝 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.
console.log("dataaa",data) |
variation="teritiary" | ||
|
||
/> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some line gaps are there, in random ways, from next time try to make it structured
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (2)
- health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss (4 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/UserDownload.js (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/UserDownload.js (1)
Pattern **/*.js
: check
🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/UserDownload.js
[error] 37-37: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (4)
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss (4)
39-39
: LGTM! Enhanced user interaction feedback.
The addition of cursor: pointer
improves user experience by providing visual feedback for interactive elements.
63-63
: LGTM! Improved spacing between action buttons.
The increased gap provides better visual separation and touch targets.
191-195
: Reusing existing accessibility feedback.
Referring to the previous review comment about WCAG contrast requirements for the text color #8e9497
.
197-200
: Reusing existing responsive design feedback.
Referring to the previous review comment about making the action buttons responsive.
.pagination-controls { | ||
display: flex; | ||
justify-content: space-between; | ||
align-items: center; | ||
padding: 1rem; | ||
border-top: 1px solid #ccc; | ||
margin-top: 1rem; | ||
font-size: 0.9rem; | ||
} | ||
|
||
.pagination-controls label { | ||
display: flex; | ||
align-items: center; | ||
gap: 0.5rem; | ||
} | ||
|
||
.pagination-controls select { | ||
padding: 0.3rem; | ||
font-size: 0.9rem; | ||
border-radius: 4px; | ||
border: 1px solid #ccc; | ||
} | ||
|
||
.pagination-controls div { | ||
display: flex; | ||
align-items: center; | ||
gap: 0.5rem; | ||
} | ||
|
||
.pagination-controls button { | ||
background: none; | ||
border: none; | ||
color: #007bff; | ||
cursor: pointer; | ||
font-size: 0.9rem; | ||
} | ||
|
||
.pagination-controls button:disabled { | ||
color: #ccc; | ||
} | ||
|
||
.pagination-controls button:hover:not(:disabled) { | ||
text-decoration: underline; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance pagination controls accessibility and user experience.
While the pagination implementation is good, consider these improvements:
- Add focus styles for keyboard navigation
- Ensure sufficient color contrast for disabled state
- Add smooth transitions for hover effects
.pagination-controls button {
background: none;
border: none;
color: #007bff;
cursor: pointer;
font-size: 0.9rem;
+ transition: all 0.2s ease;
+}
+
+.pagination-controls button:focus {
+ outline: 2px solid #007bff;
+ outline-offset: 2px;
}
.pagination-controls button:disabled {
- color: #ccc;
+ color: #767676; /* WCAG AA compliant */
}
.pagination-controls button:hover:not(:disabled) {
text-decoration: underline;
+ opacity: 0.8;
}
📝 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.
.pagination-controls { | |
display: flex; | |
justify-content: space-between; | |
align-items: center; | |
padding: 1rem; | |
border-top: 1px solid #ccc; | |
margin-top: 1rem; | |
font-size: 0.9rem; | |
} | |
.pagination-controls label { | |
display: flex; | |
align-items: center; | |
gap: 0.5rem; | |
} | |
.pagination-controls select { | |
padding: 0.3rem; | |
font-size: 0.9rem; | |
border-radius: 4px; | |
border: 1px solid #ccc; | |
} | |
.pagination-controls div { | |
display: flex; | |
align-items: center; | |
gap: 0.5rem; | |
} | |
.pagination-controls button { | |
background: none; | |
border: none; | |
color: #007bff; | |
cursor: pointer; | |
font-size: 0.9rem; | |
} | |
.pagination-controls button:disabled { | |
color: #ccc; | |
} | |
.pagination-controls button:hover:not(:disabled) { | |
text-decoration: underline; | |
} | |
.pagination-controls { | |
display: flex; | |
justify-content: space-between; | |
align-items: center; | |
padding: 1rem; | |
border-top: 1px solid #ccc; | |
margin-top: 1rem; | |
font-size: 0.9rem; | |
} | |
.pagination-controls label { | |
display: flex; | |
align-items: center; | |
gap: 0.5rem; | |
} | |
.pagination-controls select { | |
padding: 0.3rem; | |
font-size: 0.9rem; | |
border-radius: 4px; | |
border: 1px solid #ccc; | |
} | |
.pagination-controls div { | |
display: flex; | |
align-items: center; | |
gap: 0.5rem; | |
} | |
.pagination-controls button { | |
background: none; | |
border: none; | |
color: #007bff; | |
cursor: pointer; | |
font-size: 0.9rem; | |
transition: all 0.2s ease; | |
} | |
.pagination-controls button:focus { | |
outline: 2px solid #007bff; | |
outline-offset: 2px; | |
} | |
.pagination-controls button:disabled { | |
color: #767676; /* WCAG AA compliant */ | |
} | |
.pagination-controls button:hover:not(:disabled) { | |
text-decoration: underline; | |
opacity: 0.8; | |
} |
onClick={() => handlePageChange(currentPage - 1)} | ||
isDisabled={currentPage === 1} | ||
icon={"ArrowBackIos"} | ||
variation="teritiary" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct the spelling of 'tertiary' in Button component 'variation' prop
The variation
prop is misspelled as 'teritiary'
; please correct it to 'tertiary'
to ensure the prop is recognized correctly.
Apply this diff to fix the typo:
// At lines 100 and 108
- variation="teritiary"
+ variation="tertiary"
Also applies to: 108-108
{/* Pagination Controls */} | ||
<div className="pagination-controls"> | ||
<label> | ||
{t("Rows per page")}: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use consistent translation keys for proper localization
The translation keys 'Rows per page'
and 'Page'
should follow the project's convention of using uppercase keys with underscores, like 'ROWS_PER_PAGE'
and 'PAGE'
, to ensure consistency and proper localization.
Apply this diff to update the translation keys:
- {t("Rows per page")}:
+ {t("ROWS_PER_PAGE")}:
- {t("Page")}: {currentPage} / {totalPages}
+ {t("PAGE")}: {currentPage} / {totalPages}
Also applies to: 95-95
// Calculate total pages | ||
const totalPages = Math.ceil(files.length / rowsPerPage); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle zero files scenario to prevent incorrect pagination display
When there are no files, totalPages
becomes 0
, which can lead to issues like displaying '1 / 0'
and enabling pagination buttons incorrectly. Consider adding logic to handle cases when there are no files.
Apply this diff to adjust the pagination logic:
// Calculate total pages
-const totalPages = Math.ceil(files.length / rowsPerPage);
+const totalPages = Math.ceil(Math.max(files.length, 1) / rowsPerPage);
Alternatively, conditionally render the pagination controls:
{files.length > 0 ? (
// Pagination Controls
) : (
<TextBlock body={t("NO_FILES_AVAILABLE")} />
)}
Ensure you have a translation key for "NO_FILES_AVAILABLE"
in your localization files.
Also applies to: 95-95
} | ||
{/* Display paginated files */} | ||
{currentData.map((item, index) => { | ||
const fileName = item?.additionalDetails?.fileName || `FileNo${item?.processedFilestoreId?.slice(0, 4) || ''}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure safe usage of 'slice' on 'processedFilestoreId'
If item.processedFilestoreId
is null
or undefined
, calling slice
on it will throw an error. Please ensure that processedFilestoreId
is a string before calling slice
.
Apply this diff to safely handle processedFilestoreId
:
- const fileName = item?.additionalDetails?.fileName || `FileNo${item?.processedFilestoreId?.slice(0, 4) || ''}`;
+ const fileName = item?.additionalDetails?.fileName || (item?.processedFilestoreId ? `FileNo${item.processedFilestoreId.slice(0, 4)}` : '');
📝 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 fileName = item?.additionalDetails?.fileName || `FileNo${item?.processedFilestoreId?.slice(0, 4) || ''}`; | |
const fileName = item?.additionalDetails?.fileName || (item?.processedFilestoreId ? `FileNo${item.processedFilestoreId.slice(0, 4)}` : ''); |
select: data => { | ||
const currentUserUuid = Digit.UserService.getUser().info.uuid; | ||
const ResourceDetails = data?.ResourceDetails || []; | ||
const filteredData = ResourceDetails.filter(item => item?.auditDetails?.createdBy == currentUserUuid && item?.action == "create"); | ||
data.ResourceDetails = filteredData | ||
const filteredData = ResourceDetails.filter(item => item?.auditDetails?.createdBy === currentUserUuid && item?.action === "create"); | ||
data.ResourceDetails = filteredData; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid mutating 'data' inside the 'select' function
Directly mutating data.ResourceDetails
can lead to unintended side effects. It's better to return a new object with the updated ResourceDetails
.
Apply this diff to return a new data object:
select: data => {
const currentUserUuid = Digit.UserService.getUser().info.uuid;
const ResourceDetails = data?.ResourceDetails || [];
const filteredData = ResourceDetails.filter(
item => item?.auditDetails?.createdBy === currentUserUuid && item?.action === "create"
);
- data.ResourceDetails = filteredData;
- return data;
+ return {
+ ...data,
+ ResourceDetails: filteredData,
+ };
},
📝 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.
select: data => { | |
const currentUserUuid = Digit.UserService.getUser().info.uuid; | |
const ResourceDetails = data?.ResourceDetails || []; | |
const filteredData = ResourceDetails.filter(item => item?.auditDetails?.createdBy == currentUserUuid && item?.action == "create"); | |
data.ResourceDetails = filteredData | |
const filteredData = ResourceDetails.filter(item => item?.auditDetails?.createdBy === currentUserUuid && item?.action === "create"); | |
data.ResourceDetails = filteredData; | |
select: data => { | |
const currentUserUuid = Digit.UserService.getUser().info.uuid; | |
const ResourceDetails = data?.ResourceDetails || []; | |
const filteredData = ResourceDetails.filter(item => item?.auditDetails?.createdBy === currentUserUuid && item?.action === "create"); | |
return { | |
...data, | |
ResourceDetails: filteredData, | |
}; | |
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FileComponent.js (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FileComponent.js (1)
Pattern **/*.js
: check
🔇 Additional comments (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FileComponent.js (1)
18-23
:
Improve click handler implementation.
The current implementation has several issues:
- Missing event handling safeguards
- Redundant download triggers (both Card and Download button)
- No loading state feedback
<Card type={"secondary"}
className="card-color"
onClick={(e) => {
+ e.preventDefault();
+ e.stopPropagation();
+ setIsDownloading?.(true);
downloadHandler();
}}
>
Also consider:
- Remove the Download button since the entire card is now clickable
- Add loading state feedback during download
- Add error handling for failed downloads
Likely invalid or redundant comment.
{lastmodTime && userName && ( | ||
<div className="dm-audit-info" style={{ fontSize: "1rem", color: "#8e9497" }}> | ||
{/* Uploaded by {userName} | {lastmodTime} */} | ||
Last Modified time {lastmodTime} | ||
</div> | ||
)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve audit info implementation.
Several issues need to be addressed:
- Username check should be removed as per previous discussion
- Inline styles should be moved to CSS
- Text should be translatable
- {lastmodTime && userName && (
+ {lastmodTime && (
<div className="dm-audit-info" style={{ fontSize: "1rem", color: "#8e9497" }}>
- Last Modified time {lastmodTime}
+ {t("LAST_MODIFIED_TIME", { time: lastmodTime })}
</div>
)}
Add to your CSS:
.dm-audit-info {
font-size: 1rem;
color: #8e9497;
}
Add to your translations:
{
"LAST_MODIFIED_TIME": "Last Modified time {{time}}"
}
@@ -6,7 +6,7 @@ import { Button } from "@egovernments/digit-ui-components"; | |||
|
|||
const FileComponent = ({ title, fileName, auditDetails, editHandler, deleteHandler, downloadHandler }) => { | |||
const { t } = useTranslation(); | |||
const { lastmodTime } = auditDetails || {}; // Destructuring the audit details for easy access | |||
const { lastmodTime,userName } = auditDetails || {}; // Destructuring the audit details for easy access |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove userName destructuring as per previous discussion.
Based on the previous comment from ashish-egov: "Discussed with Nipun, now we dont need username to show", the userName destructuring should be removed.
- const { lastmodTime,userName } = auditDetails || {}; // Destructuring the audit details for easy access
+ const { lastmodTime } = auditDetails || {}; // Destructuring the audit details for easy access
📝 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 { lastmodTime,userName } = auditDetails || {}; // Destructuring the audit details for easy access | |
const { lastmodTime } = auditDetails || {}; // Destructuring the audit details for easy access |
User-management, user-download screen changes
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
Chores