css fixes #1715
css fixes #1715
Conversation
📝 WalkthroughWalkthroughThe pull request introduces a series of modifications across multiple files in the micro UI framework. Key updates include enhancements to user management configurations, styling adjustments in SCSS files, and refinements in component rendering logic. Notable changes involve the addition of validation methods, improved handling of user roles, and updates to UI components to enhance user experience. The changes also streamline certain functionalities, such as removing redundant elements and optimizing layout properties, while maintaining core logic and functionality throughout the application. Changes
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 (
|
Co-authored-by: rachana-egov <rachana.singh@egovernment.org>
...th/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
🧹 Outside diff range comments (10)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/securityPopUp.js (1)
Line range hint 94-123: Consider adding validation for required questions before save.
While the component handles API errors well, it might benefit from client-side validation before attempting to save, especially for questions marked as required.
Consider adding validation logic before the save operation:
const validateAnswers = () => {
const requiredQuestions = questions.filter(q => q.required);
const unansweredRequired = requiredQuestions.filter(q => !answers[q.id]);
return unansweredRequired.length === 0;
};
const handleSave = async () => {
if (!validateAnswers()) {
setShowToast({
key: "error",
label: t("HCM_REQUIRED_QUESTIONS_ERROR")
});
return;
}
// ... existing save logic
};health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/editVillagePopulationPopUP.js (3)
Line range hint 9-10: Remove unused state for confirmedTargetPopulation
The UI elements for confirmedTargetPopulation have been commented out, but the state initialization is still present. This creates inconsistency and potential maintenance issues.
Clean up the unused state by applying this diff:
// State to manage confirmed population and target population
const [confirmedTotalPopulation, setConfirmedTotalPopulation] = useState("");
- const [confirmedTargetPopulation, setConfirmedTargetPopulation] = useState("");
const [showToast, setShowToast] = useState(null);Also applies to: 77-86
Line range hint 13-19: Update useEffect to remove confirmedTargetPopulation initialization
The effect hook still attempts to initialize the removed confirmedTargetPopulation state.
Update the effect hook to match the current component state:
useEffect(() => {
if (census?.additionalDetails) {
setConfirmedTotalPopulation(census.additionalDetails.confirmedTotalPopulation || "");
- setConfirmedTargetPopulation(census.additionalDetails.confirmedTargetPopulation || "");
}
}, [census]);Line range hint 44-60: Clean up commented properties in mutation payload
The mutation payload contains commented properties that should be removed for better code maintainability.
Update the mutation payload:
body: {
Census: {
...census,
totalPopulation: confirmedTotalPopulation,
additionalDetails: {
...census.additionalDetails,
- // confirmedTotalPopulation,
- //confirmedTargetPopulation,
},
workflow: {
...census.workflow,
action: workflowAction,
},
},
}health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/accessbilityPopUP.js (1)
Line range hint 1-176: File naming convention needs to be consistent.
The filename accessbilityPopUP.js has inconsistent casing. Consider renaming it to AccessibilityPopUp.js to match the component name and follow proper React component naming conventions.
🧰 Tools
🪛 Biome
[error] 155-164: 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)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccessWrapper.js (1)
Line range hint 65-73: Consider enhancing error handling in handleNext.
The current error handling when data is null for national roles could be more informative. Consider adding more context to help users understand why they can't proceed.
Apply this diff to improve the error message:
if (data === null && nationalRoles.includes(String(rolesArray?.[prevKey - 1]))) {
+ console.debug(`Cannot proceed: data is null for national role ${rolesArray?.[prevKey - 1]}`);
setShowErrorToast(true);
return prevKey; // Keep the same value if condition is true
} else {health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FormulaConfiguration.js (2)
Line range hint 31-98: Consider refactoring the deletion logic for better maintainability.
The handleConfirmDelete function contains complex nested logic and multiple state updates. Consider breaking it down into smaller, focused functions for better maintainability and testing.
+ const clearDependentFormulas = (deletedOutput, formula) => {
+ const shouldClearInput = formula.input === deletedOutput;
+ const shouldClearAssumption = formula.assumptionValue === deletedOutput;
+
+ return {
+ ...formula,
+ input: shouldClearInput ? "" : formula.input,
+ assumptionValue: shouldClearAssumption ? "" : formula.assumptionValue
+ };
+ };
const handleConfirmDelete = () => {
if (formulaToDelete !== null) {
const deletedFormula = formulas.find(operation => operation.output === formulaToDelete.output);
- // Current complex implementation
+ // Update formulas
+ const updatedFormulas = formulas.filter(operation => operation.output !== formulaToDelete.output);
+ setFormulas(updatedFormulas);
+
+ // Update deleted formulas tracking
+ if (!deletedFormulaCategories.current[category]) {
+ deletedFormulaCategories.current[category] = [];
+ }
+ deletedFormulaCategories.current[category].push(deletedFormula.output);
+ setDeletedFormulas(prev => [...prev, deletedFormula.output]);
+
+ // Handle cascade effects
+ setFormulaConfigValues(prevValues =>
+ prevValues
+ .filter(value => value.output !== deletedFormula.output)
+ .map(formula => clearDependentFormulas(deletedFormula.output, formula))
+ );
}
setShowPopUp(false);
};Line range hint 246-333: Extract common PopUp logic and add missing key props.
The delete and add formula PopUps share similar structure. Consider extracting common logic and adding required key props.
footerChildren={[
<Button
+ key="confirm"
type={"button"}
size={"large"}
variation={"secondary"}
label={t("YES")}
onClick={handleConfirmDelete}
/>,
<Button
+ key="cancel"
type={"button"}
size={"large"}
variation={"primary"}
label={t("NO")}
onClick={handleCancelDelete}
/>,
]}Consider creating a reusable ConfirmationPopUp component:
const ConfirmationPopUp = ({
isOpen,
onClose,
onConfirm,
title,
children
}) => (
<PopUp
className="popUpClass"
type="default"
heading={title}
equalWidthButtons={true}
children={children}
onOverlayClick={onClose}
footerChildren={[/* ... */]}
sortFooterChildren={true}
onClose={onClose}
/>
);🧰 Tools
🪛 Biome
[error] 269-269: 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)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js (2)
Line range hint 295-309: Remove commented code blocks to improve maintainability.
Instead of keeping commented code in the file:
- If the code will be needed later, create a ticket to track the TODO items.
- If the code is obsolete, remove it entirely.
- Consider using Git history to track previous implementations if needed.
This will improve code readability and reduce maintenance overhead.
Would you like me to help create tickets for tracking these TODO items?
Line range hint 13-24: Consider refactoring to improve component maintainability.
The component currently manages multiple responsibilities and state variables. Consider:
- Breaking down the component into smaller, focused components
- Using a reducer pattern to manage popup states instead of multiple useState calls
- Creating a custom hook to centralize the data fetching logic
This would improve code organization, testability, and maintainability.
Example reducer pattern for popup states:
const popupReducer = (state, action) => {
switch (action.type) {
case 'SHOW_POPUP':
return { ...state, [action.popup]: true };
case 'HIDE_POPUP':
return { ...state, [action.popup]: false };
default:
return state;
}
};
// Usage in component
const [popupState, dispatch] = useReducer(popupReducer, {
accessibility: false,
security: false,
editPopulation: false,
commentLog: false
});Also applies to: 37-40
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (16)
- health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js (2 hunks)
- health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/facility.scss (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/index.scss (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/villageView.scss (3 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FormulaConfigWrapper.js (2 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FormulaConfiguration.js (9 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (9 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/TableSearchBar.js (3 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (2 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccessWrapper.js (4 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/accessbilityPopUP.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/editVillagePopulationPopUP.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/securityPopUp.js (3 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (2 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js (4 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js (3 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
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/FormulaConfigWrapper.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FormulaConfiguration.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/TableSearchBar.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccessWrapper.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/accessbilityPopUP.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/editVillagePopulationPopUP.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/securityPopUp.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/PopInbox.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js (1)
Pattern **/*.js: check
📓 Learnings (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccessWrapper.js (1)
Learnt from: Ramkrishna-egov
PR: egovernments/DIGIT-Frontend#1670
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccessMgmtTableWrapper.js:156-160
Timestamp: 2024-10-25T09:24:29.878Z
Learning: In the `UserAccessMgmtTableWrapper` component, since the `ShowMoreWrapper` is rendered within the cell, additional checks or row-specific state management are not required.
🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FormulaConfiguration.js
[error] 228-228: 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)
[error] 164-164: 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] 269-269: 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)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js
[error] 287-287: 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)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccessWrapper.js
[error] 20-20: This let declares a variable that is only assigned once.
'mpRolesArray' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/securityPopUp.js
[error] 94-94: 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 (18)
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/villageView.scss (3)
8-8: LGTM! Margin adjustment improves vertical rhythm.
The reduction in margin-bottom creates better spacing between the header and subsequent content.
21-21: LGTM! Explicit margin reset improves layout consistency.
Setting margin to 0rem helps prevent any unwanted spacing inheritance.
73-76: LGTM! Consistent margin spacing with other headings.
The increased margin-bottom aligns with the spacing pattern used throughout the file.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/TableSearchBar.js (1)
5-5: LGTM! Component structure follows React best practices.
The component maintains proper prop validation and follows React patterns correctly.
Also applies to: 69-70
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/facility.scss (1)
185-185: LGTM! Good use of gap property for consistent spacing.
The gap property is a clean solution for spacing between tags.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/securityPopUp.js (2)
3-3: LGTM: Import changes are well-organized.
The addition of the Divider component import is properly grouped with other component imports from the same package.
116-116: LGTM: UI improvements enhance readability.
The changes improve the visual hierarchy and spacing:
- Increased gap between radio options provides better visual separation
- Added divider between questions makes the structure clearer
Also applies to: 122-123
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/editVillagePopulationPopUP.js (1)
144-144: LGTM! Good UI enhancement
The addition of equalWidthButtons={true} improves the visual consistency of the popup buttons, aligning with the CSS fixes objective of this PR.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/accessbilityPopUP.js (1)
Line range hint 1-176: Well-structured component with proper implementation patterns.
The component demonstrates good practices:
- Effective use of React hooks for state management
- Proper error handling with Toast notifications
- Clean mutation handling with proper loading states
- Well-structured form with proper validation
- Good implementation of translations
🧰 Tools
🪛 Biome
[error] 155-164: 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)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccessWrapper.js (2)
128-128: LGTM! Styling improvements enhance visual consistency.
The changes improve the component's visual presentation:
- Reduced gap spacing for better element alignment
- More descriptive class name that better reflects the component's purpose
Also applies to: 133-133
139-142: LGTM! Props structure follows best practices.
The UserAccess component props are well-organized and properly typed:
- Clear prop naming
- Logical prop values derived from parent state and constants
- Multi-line format improves readability
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (1)
233-234: LGTM! Verify translation key existence.
The styling changes look good. However, ensure that the translation key ${category}_DESCRIPTION exists in the translation files for all possible category values to avoid missing translations.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js (1)
242-242: LGTM: Button size standardization improves UI consistency.
The button size changes appropriately differentiate between primary actions (medium) and secondary actions (small), creating a clear visual hierarchy.
Also applies to: 261-261, 288-288
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FormulaConfigWrapper.js (1)
Line range hint 361-374: LGTM! Clean class-based styling implementation.
The refactoring to use class-based styling with semantic class names improves maintainability and follows CSS best practices.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js (1)
9-9: Verify CSS styles for the new header.
The Header component is properly added with the className "pop-inbox-header". However, ensure that the corresponding CSS styles exist in your stylesheet.
Also applies to: 384-384
✅ Verification successful
CSS styles exist but could be enhanced
The CSS class pop-inbox-header exists in facility.scss but only sets margin: 0rem. Consider enhancing the styles to better match the header's visual requirements if needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the pop-inbox-header class definition
rg -g "*.scss" -g "*.css" "pop-inbox-header"
Length of output: 148
Script:
#!/bin/bash
# Let's check the actual CSS definition for pop-inbox-header
rg -g "*.scss" -g "*.css" -A 5 "pop-inbox-header"
Length of output: 639
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (2)
541-541: 🧹 Nitpick (assertive)
Remove commented loader code
Clean up the commented LoaderOverlay code since it's been replaced with the standard Loader component.
- {/* {isLoading || isHrmsLoading ? <LoaderOverlay /> : null} */}
<DataTable
columns={columns}
data={HrmsData?.data}
pagination
progressPending={isLoading || isHrmsLoading}
progressComponent={<Loader />}Likely invalid or redundant comment.
165-169: 🛠️ Refactor suggestion
Consider adding confirmation before resetting boundaries
When changing hierarchy, all selected boundaries are reset without warning. This could lead to accidental data loss if users change the hierarchy by mistake.
Consider adding a confirmation dialog:
...i,
selectedHierarchy: value,
boundaryOptions,
-selectedBoundaries: [], // Keep existing selected boundaries
+selectedBoundaries: existingRow.selectedBoundaries?.length > 0
+ ? (window.confirm(t("CONFIRM_RESET_BOUNDARIES")) ? [] : existingRow.selectedBoundaries)
+ : [],health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js (1)
987-987:
Add 'DOWNLOAD' case to handle 'DOWNLOAD' action in onActionSelect
The onClick handler calls onActionSelect("DOWNLOAD", row), but the onActionSelect function does not handle the "DOWNLOAD" action. This will result in the default case being executed, which may lead to unexpected behavior.
Apply this diff to add the 'DOWNLOAD' case:
const onActionSelect = (key, row) => {
switch (key) {
case "START":
// existing code
break;
case "EDIT":
// existing code
break;
+ case "DOWNLOAD":
+ // Add logic to handle DOWNLOAD action
+ window.location.href = `/your/download/path?microplanId=${row?.id}&campaignId=${row?.campaignId}`;
+ break;
default:
- console.log(value);
+ console.log(`Unhandled action: ${key}`);
break;
}
};Likely invalid or redundant comment.
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/villageView.scss
Show resolved
Hide resolved
.../micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/TableSearchBar.js
Show resolved
Hide resolved
.../micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/TableSearchBar.js
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/facility.scss
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/facility.scss
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/index.scss
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js
Show resolved
Hide resolved
Choose the appropriate template for your PR:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Style
Documentation