Changes role access #1617
Changes role access #1617
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request primarily involve updates to the HTML and SCSS files within the micro-ui project. The HTML file's stylesheet link for health-related CSS has been updated from version 0.1.6 to 0.1.7. The SCSS file has undergone extensive modifications, including new styles, variable adjustments, and mixin definitions, to enhance the visual consistency and responsiveness of various UI components. Additionally, the JavaScript component Changes
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: 5
🧹 Outside diff range comments (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (1)
Line range hint
434-446: Improve styling approach and fix indentation.The addition of a scrollable container for the DataTable is a good UI improvement. However, let's enhance it further:
Replace inline styling with a CSS class:
<div className="data-table-container">Add this to your CSS file:
.data-table-container { overflow-y: scroll; max-height: [appropriate-value]; /* Consider setting a max-height */ }Fix the indentation of the DataTable component to align with the opening div tag.
These changes will improve code maintainability and readability.
📜 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 (3)
- health/micro-ui/web/micro-ui-internals/example/public/index.html (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss (2 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (1)
Pattern
**/*.js: check
📓 Learnings (1)
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.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/RoleTableComposer.js
[error] 323-323: 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)
🔇 Additional comments (3)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (2)
57-58: Verify handling of undefinedtopBoundaryValuefor national roles.The updated logic for
selectedBoundarieslooks good and aligns with the introduction oftopBoundaryValue. However, ensure that the logic handles cases wheretopBoundaryValuemight be undefined for national roles.Consider adding a fallback or validation:
selectedBoundaries: nationalRoles?.includes(category) ? (topBoundaryValue ? [topBoundaryValue] : []) : data?.planData?.find((i) => i.employeeId === item?.user?.userServiceUuid)?.jurisdiction,This ensures that even if
topBoundaryValueis undefined, an empty array is used instead of potentially causing issues downstream.
Line range hint
1-456: Overall, good improvements with minor suggestions for refinement.The changes in this file effectively implement the preselecting boundary for the national level, as mentioned in the PR objectives. The modifications consistently handle national roles across different parts of the component, improving its functionality. The UI enhancements, such as the scrollable DataTable container, align with the goal of modifying the CSS of the role-table.
To further refine the code:
- Ensure robust null checking where nested objects are accessed.
- Consider performance optimizations, especially in data filtering and mapping operations.
- Maintain consistent code style, particularly in the use of arrow functions and boolean assignments.
- Prefer CSS classes over inline styles for better maintainability.
These minor adjustments will enhance the overall quality and maintainability of the component.
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss (1)
2593-2593: Consider the impact ofmin-width: 90vwon smaller screensSetting
min-width: 90vwmay cause layout issues on devices with smaller screens, potentially leading to horizontal scrolling. Please verify that this change maintains responsive design across all viewports.
...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
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range comments (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (1)
Line range hint
435-447: Consider moving inline styles to a CSS classThe inline style
style={{overflowY:"auto"}}can be moved to a CSS class for better maintainability and consistency.Example:
- <div style={{overflowY:"auto"}}> + <div className="dataTableContainer">And in your CSS file, add:
.dataTableContainer { overflow-y: auto; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (2)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (8 hunks)
- health/micro-ui/web/public/index.html (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (1)
Pattern
**/*.js: check
🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
[error] 324-324: 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)
🔇 Additional comments (4)
health/micro-ui/web/public/index.html (1)
14-14: CSS version update looks good, but verify consistency across the project.The update of the health CSS version from 0.1.6 to 0.1.7 is in line with the PR objectives of modifying the CSS of the role-table. This change should bring in the latest styles for the health module.
To ensure consistency across the project, please run the following script to check if this CSS version is used in other files:
This will help ensure that the CSS version is updated consistently across all relevant files in the project.
✅ Verification successful
CSS version update is consistent across relevant HTML files.
Updated to version
0.1.7in:
health/micro-ui/web/public/index.htmlhealth/micro-ui/web/micro-ui-internals/example/public/index.htmlNo occurrences of the old version
0.1.6found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for other occurrences of the health CSS version # Search for both old and new versions echo "Occurrences of old version (0.1.6):" rg -i "[email protected]" --type html echo "Occurrences of new version (0.1.7):" rg -i "[email protected]" --type html # Check package.json for health CSS dependency echo "Health CSS dependency in package.json:" jq '.dependencies["@egovernments/digit-ui-health-css"]' package.jsonLength of output: 801
Script:
#!/bin/bash # Description: Locate all package.json files and check for the health CSS dependency # Find all package.json files in the repository echo "Searching for all package.json files..." fd package.json # Assuming package.json files are found, check each for the health CSS dependency echo "Checking for '@egovernments/digit-ui-health-css' dependency in found package.json files:" fd package.json --exec sh -c 'echo "File: {}"; jq ".dependencies[\"@egovernments/digit-ui-health-css\"] // .devDependencies[\"@egovernments/digit-ui-health-css\"]" {} || echo "Dependency not found."' \;Length of output: 4640
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (3)
31-33: Approved: Null checks have been appropriately implementedThe introduction of
topBoundaryValuewith null checks enhances robustness and prevents potential runtime errors.
57-58: Approved: Conditional assignment ofselectedBoundariesis appropriateThe logic for assigning
selectedBoundariesbased onnationalRolesandcategorycorrectly handles different scenarios.
87-90: Approved: Row data initialization logic is soundThe updated initialization of
rowDataeffectively managesselectedBoundariesfor national roles and other categories.
...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
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range comments (2)
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/index.scss (1)
Line range hint
419-423: Consider revising the submit bar styling for better maintainability and responsiveness.The current implementation has a few potential issues:
- The use of
!importantfor the background color may lead to specificity issues and make future style overrides difficult.- Setting
width: max-contentcould cause layout problems on smaller screens if the content is too wide.- The hardcoded color
#c84c0emight not be consistent with the application's color scheme.Consider the following improvements:
- Remove
!importantand increase the specificity of the selector if needed.- Use a more responsive width setting, such as
width: 100%withmax-width.- Use a color variable for consistency with the overall theme.
.addProductActionClass { .submit-bar { - background-color: #c84c0e !important; - width: max-content; + background-color: var(--primary-color); + width: 100%; + max-width: 300px; padding-left: 1.5rem; padding-right: 1.5rem; } }health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (1)
Line range hint
434-446: Avoid inline styles for better maintainability.Consider moving the inline style
style={{overflowY:"auto"}}to a CSS class. This promotes separation of concerns and improves maintainability.Apply this change:
- <div style={{overflowY:"auto"}}> + <div className="dataTableContainer">Then, in your CSS file, add:
.dataTableContainer { overflow-y: auto; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (2)
- health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/index.scss (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (1)
Pattern
**/*.js: check
🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js
[error] 323-323: 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)
🔇 Additional comments (4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/RoleTableComposer.js (4)
31-33: Consider adding null checks for better robustness.The previous review comment about adding null checks is still applicable to ensure code robustness.
87-90: Refactoring suggestion remains valid.The previous review comment about improving readability and performance for the
selectedBoundarieslogic is still applicable.
308-308: Simplify the filter condition for better readability.The previous suggestion to simplify the condition is still valid.
323-323: Simplify thedisabledprop assignment.The previous review comment about simplifying the
disabledprop assignment is still applicable.🧰 Tools
🪛 Biome
[error] 323-323: 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)
Made preseleceting boundary for national level, also changed css of role-table
Summary by CodeRabbit
New Features
RoleTableComposercomponent with improved logic for boundary selection and user interaction.Bug Fixes
DataTableby adding a vertical scroll capability.Style