Localizations and Actions being cut #1874
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request involve updates to various components of the health micro-UI. Notably, the stylesheet version in two HTML files has been updated from 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range comments (11)
health/micro-ui/web/micro-ui-internals/example/public/index.html (1)
Line range hint
17-20: Clean up commented codeThere are multiple commented-out CSS inclusions that appear to be outdated or unused. Consider removing these comments to improve code maintainability.
- <!-- added below css for hcm-workbench module inclusion--> - <!-- <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> --> - - <!-- <link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-css/dist/index.css"/> -->health/micro-ui/web/public/index.html (1)
Let me analyze the results and generate a final response.
Remove the commented globalConfigs.js reference
The commented
globalConfigs.jsscript appears in multipleindex.htmlfiles across the codebase, all in the same commented state. Given that:
- The repository has numerous dedicated configuration files for different modules
- There's a robust configuration system in place (e.g.,
configUtils.js,mergeConfig.js)- The S3-hosted configuration approach appears to be deprecated across all instances
This commented code should be removed as it represents technical debt and may cause confusion about the current configuration approach.
🔗 Analysis chain
Line range hint
18-18: Review the commented globalConfigs.js implementationThere's a commented reference to
globalConfigs.jsfrom S3. Consider:
- If this configuration is still needed, implement it properly
- If not needed, remove the commented code
- If needed but implemented differently, update the comment to explain why
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if globalConfigs.js is referenced elsewhere # Search for other references to globalConfigs.js echo "Checking for other references to globalConfigs.js" rg "globalConfigs\.js" # Check if there are alternative config implementations echo "Checking for alternative config implementations" fd -t f "config" | grep -i "config"Length of output: 10375
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/MicroplanSearch.js (1)
Line range hint
1-46: Consider implementing defensive programming practicesThe component could benefit from several robustness improvements:
- Add validation for the URL tab parameter
- Implement error boundaries
- Add null checks for TabSearchconfig access
- Consider adding loading states
Here's a suggested implementation:
const MicroplanSearch = () => { const { t } = useTranslation(); const [defaultValues, setDefaultValues] = useState(defaultSearchValues); - const [config, setConfig] = useState(TabSearchconfig?.TabSearchconfig?.[0]); + const [config, setConfig] = useState(() => { + if (!TabSearchconfig?.TabSearchconfig?.length) { + console.error('Invalid or empty TabSearchconfig'); + return {}; + } + return TabSearchconfig.TabSearchconfig[0]; + }); const [tabData, setTabData] = useState( - TabSearchconfig?.TabSearchconfig?.map((configItem, index) => ({ + TabSearchconfig?.TabSearchconfig?.length ? TabSearchconfig.TabSearchconfig.map((configItem, index) => ({ key: index, label: configItem.label, active: index === 0 ? true : false - })) + })) : [] ); const onTabChange = (n) => { + // Validate tab index + if (n < 0 || n >= (TabSearchconfig?.TabSearchconfig?.length || 0)) { + console.error('Invalid tab index:', n); + return; + } setTabData((prev) => prev.map((i, c) => ({ ...i, active: c === n ? true : false }))); const url = new URL(window.location.href); url.searchParams.set("tabId", `${n}`); window.history.replaceState({}, "", url); };health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (2)
Line range hint
117-125: Refactor navigation to use React Router's historyIt's recommended to use React Router's
historyobject for navigation instead of directly manipulatingwindow.location.href. This approach maintains SPA behavior and ensures proper history management within your React application.To implement this, you can modify your code as follows:
First, obtain the
historyobject using theuseHistoryhook:import { useHistory } from "react-router-dom"; const history = useHistory();Then, update your
selectfunction:select={(e) => { - if (e.name == "MP_ACTIONS_EDIT_SETUP") { - window.location.href = `/${window.contextPath}/employee/microplan/setup-microplan?key=${1}µplanId=${row.id}&campaignId=${row.campaignDetails.id}`; - } - if (e.name == "MP_ACTIONS_VIEW_SUMMARY") { - window.location.href = `/${window.contextPath}/employee/microplan/setup-microplan?key=${10}µplanId=${row.id}&campaignId=${row.campaignDetails.id}&setup-completed=true`; - } + if (e.name === "MP_ACTIONS_EDIT_SETUP") { + history.push(`/${window.contextPath}/employee/microplan/setup-microplan?key=1µplanId=${row.id}&campaignId=${row.campaignDetails.id}`); + } + if (e.name === "MP_ACTIONS_VIEW_SUMMARY") { + history.push(`/${window.contextPath}/employee/microplan/setup-microplan?key=10µplanId=${row.id}&campaignId=${row.campaignDetails.id}&setup-completed=true`); + } }}This change leverages React Router's navigation capabilities and keeps the navigation consistent within your application's routing context.
Note: Ensure that
useHistoryis called within a functional component. IfadditionalCustomizationsis not a component, you might need to pass thehistoryobject as a prop or adjust your code structure accordingly.
Line range hint
117-125: Ensure consistency between selected value and optionsThe
selectedprop in yourDropdowncomponent is set to{ code: "1", name: "MP_ACTIONS_FOR_MICROPLAN_SEARCH" }, which does not match any of the entries in youroptionsarray. This could lead to unexpected behavior or the dropdown displaying an incorrect default value.Consider updating the
selectedprop to match one of the entries in theoptionsarray or adjust youroptionsarray to include the selected value. For example:Option 1: Update the
selectedprop to match an option:selected={ - { code: "1", name: "MP_ACTIONS_FOR_MICROPLAN_SEARCH" } + options[0] }Option 2: Include the selected value in your
optionsarray:if (row?.status == "DRAFT") { - options = [{ code: "1", name: "MP_ACTIONS_EDIT_SETUP" }]; + options = [ + { code: "1", name: "MP_ACTIONS_FOR_MICROPLAN_SEARCH" }, + { code: "2", name: "MP_ACTIONS_EDIT_SETUP" } + ]; }Ensure that the
selectedvalue corresponds to an option to prevent any inconsistencies in the dropdown's behavior.health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js (6)
Line range hint
834-844: Use Strict Equality Operators (===) Instead of Loose Equality Operators (==)To prevent unintended type coercion, it's recommended to use strict equality operators (
===) when comparing values in JavaScript.Suggested changes:
- if (e.name == "MP_ACTIONS_EDIT_SETUP") { + if (e.name === "MP_ACTIONS_EDIT_SETUP") { - if (e.name == "MP_ACTIONS_VIEW_SUMMARY") { + if (e.name === "MP_ACTIONS_VIEW_SUMMARY") {
Line range hint
542-544: Avoid Using React Hooks Inside Non-Component FunctionsReact hooks like
useLocationanduseParamsshould only be used within React function components or custom hooks, not inside regular functions.To fix this, consider passing
locationandmasterNameas parameters to thepreProcessfunction or refactoring the function into a custom hook.
Line range hint
878-882: Avoid Using React Hooks Inside Non-Component FunctionsThe
Digit.Hooks.useQueryParams()hook is used inside a regular functionpreProcess. Hooks should only be called within React function components or custom hooks.To resolve this, pass
tabIdas a parameter to thepreProcessfunction or restructure the code to comply with React hooks rules.
Line range hint
890-908: Use React Router'suseHistoryHook for NavigationFor consistent navigation handling in React applications, it's recommended to use the
useHistoryhook fromreact-router-dominstead of directly manipulatingwindow.history.Suggested refactor:
+ import { useHistory } from "react-router-dom"; + const history = useHistory(); const onActionSelect = (key, row) => { switch (key) { case "START": - window.history.pushState( - { - microplanId: row?.id, - campaignId: row?.campaignId, - }, - "", - `/${window.contextPath}/employee/microplan/select-activity?microplanId=${row?.id}&campaignId=${row?.campaignId}` - ); - const navEvent = new PopStateEvent("popstate"); - window.dispatchEvent(navEvent); + history.push(`/${window.contextPath}/employee/microplan/select-activity?microplanId=${row?.id}&campaignId=${row?.campaignId}`); break; case "EDIT": // Similar changes for the EDIT case break; case "DOWNLOAD": handleFileDownload(); break; default: console.log(value); break; } };
Line range hint
910-912: EnsurerolesCodesis Defined Before MappingThere's a potential for
rolesCodesto beundefined, which would cause a runtime error when callingmap. It's important to handle this case to prevent crashes.Suggested fix:
- const rolesCodes = Digit.Hooks.useSessionStorage("User", {})[0]?.info?.roles; + const rolesCodes = Digit.Hooks.useSessionStorage("User", {})[0]?.info?.roles || []; const roles = rolesCodes.map((item) => item.code);
Line range hint
1305-1316: Avoid Using React Hooks Inside Non-Component FunctionsThe
Digit.Hooks.useQueryParams()hook is used inside thegetFacilitySearchRequestfunction, which is not a React component or custom hook. Hooks should only be used within React function components or custom hooks.To fix this, pass
campaignIdas a parameter to thegetFacilitySearchRequestfunction or refactor the function into a custom hook.
📜 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.jsonis excluded by!**/*.json
📒 Files selected for processing (7)
health/micro-ui/web/micro-ui-internals/example/public/index.html(1 hunks)health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FacilityPopup.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/pages/employee/MicroplanSearch.js(1 hunks)health/micro-ui/web/public/index.html(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FacilityPopup.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/pages/employee/MicroplanSearch.js (1)
Pattern **/*.js: check
🔇 Additional comments (3)
health/micro-ui/web/public/index.html (1)
13-13: Verify the CSS package version and potential breaking changes
The health CSS package version has been updated from 0.1.38 to 0.1.39. Let's ensure this is the latest stable version and check for any breaking changes.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/MicroplanSearch.js (2)
36-36: Improved translation handling approach
The modification to wrap the entire template string in the translation function is a better approach. It allows for more natural translations where the word order might differ across languages.
Line range hint 4-4: Verify TabSearchconfig structure
Let's ensure the configuration file maintains the expected structure for proper component functionality.
✅ Verification successful
The search results provide clear evidence of the TabSearchconfig structure and its usage. Let me generate the final response:
TabSearchconfig structure is properly defined and used consistently
The configuration follows a well-defined structure across the codebase:
- Defined in
MicroplanSearchConfig.jswith expected properties:showTabandTabSearchconfigarray - Each tab config item contains required fields:
label,type, andapiDetails - Used consistently in both
MicroplanSearch.jsandMyMicroplans.jswith proper null checks
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check TabSearchconfig structure in the codebase
# Search for TabSearchconfig definition
ast-grep --pattern 'const TabSearchconfig = {
TabSearchconfig: $_
}'
# Look for any other usage of this config
rg "TabSearchconfig" -A 5
Length of output: 8555
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FacilityPopup.js
Show resolved
Hide resolved
Localizations and Actions being cut
Summary by CodeRabbit
New Features
FacilityPopupcomponent with localization for village information and improved assignment logic.Bug Fixes
FacilityPopupcomponent.Style
Documentation
MicroplanSearchcomponent.