css fixes #1792
css fixes #1792
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request includes updates to various CSS and SCSS files, primarily focusing on modifying styles for components in the employee interface. Key changes involve updating the version of a linked CSS stylesheet in HTML files, adjusting margin properties in SCSS files, and enhancing the structure and styling of React components. The changes aim to improve layout consistency, visual presentation, and user experience across the application. 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: 8
🧹 Outside diff range comments (7)
health/micro-ui/web/micro-ui-internals/example/public/index.html (1)
Line range hint
17-21: Consider cleaning up commented code.There are multiple commented-out CSS imports and script tags that appear to be unused. If these are no longer needed, consider removing them 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"/> --> - <!-- <script src="https://s3.ap-south-1.amazonaws.com/egov-dev-assets/globalConfigs.js"></script> -->health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignType.js (1)
Line range hint
111-154: Establish a consistent styling approach.The component mixes inline styles with CSS classes. Consider adopting a consistent approach using CSS classes throughout the component for better maintainability and readability. This could involve:
- Moving all inline styles to CSS classes
- Using BEM or another CSS naming convention
- Organizing styles in a dedicated SCSS file
Would you like assistance in restructuring the styles using a consistent methodology?
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/SetupCampaign.js (5)
Line range hint
58-70: Simplify nestedif-elsestatements for better readabilityThe deeply nested
if-elseblocks within theuseEffecthook can make the code hard to read and maintain. Consider refactoring to flatten the structure and improve clarity.Here's a refactored version:
-useEffect(() => { - if (isPreview === "true") { - setIsDraftCreated(true); - setCurrentKey(14); - } - else if (isDraft === "true") { - setIsDraftCreated(true); - if (isSkip === "false") { - if (currentKey === 1) setCurrentKey(1); - } else if (isDateRestricted === "true") { - setCurrentKey(3); - } else { - if(draftData?.additionalDetails?.key === 7 || draftData?.additionalDetails?.key === 8){ - setCurrentKey(6); - } - else{ - setCurrentKey(draftData?.additionalDetails?.key); - } - } - } -}, [isPreview, isDraft, draftData]); +useEffect(() => { + if (isPreview === "true") { + setIsDraftCreated(true); + setCurrentKey(14); + return; + } + + if (isDraft === "true") { + setIsDraftCreated(true); + if (isSkip === "false" && currentKey === 1) { + setCurrentKey(1); + } else if (isDateRestricted === "true") { + setCurrentKey(3); + } else if (draftData?.additionalDetails?.key === 7 || draftData?.additionalDetails?.key === 8) { + setCurrentKey(6); + } else { + setCurrentKey(draftData?.additionalDetails?.key); + } + } +}, [isPreview, isDraft, draftData]);
Line range hint
80-80: InitializedisplayMenustate with a boolean valueThe
displayMenustate is used as a boolean but is initialized withnull. Initializing it withfalseenhances type consistency and code clarity.Apply this diff to fix the issue:
-const [displayMenu, setDisplayMenu] = useState(null); +const [displayMenu, setDisplayMenu] = useState(false);
Line range hint
220-220: Avoid usingasyncdirectly withuseEffectDefining an
asyncfunction directly as the callback touseEffectcan lead to unexpected behaviors becauseuseEffectexpects a cleanup function, not a Promise. Instead, define an inner async function and call it.Apply this diff to fix the issue:
-useEffect(async () => { +useEffect(() => { + const updateData = async () => { if (shouldUpdate === true) { // existing code } + }; + updateData(); }, [shouldUpdate]);
Line range hint
254-254: Replaceconsole.logwith proper error loggingUsing
console.logfor error handling in production code is not recommended. Consider using a dedicated logging mechanism or error tracking service to capture and manage errors effectively.Apply this diff to fix the issue:
- console.log(error); + // Replace with appropriate logging mechanism + Digit.Logger.error("Update Campaign Error:", error);
Line range hint
263-318: Refactor to eliminate duplication when constructingpayloadDataThe logic for constructing
payloadDatais repeated in multiple places. Extract this logic into a separate function to improve maintainability and reduce code duplication.Example refactoring:
+const constructPayloadData = () => { + let payloadData = { ...draftData }; + payloadData.hierarchyType = hierarchyType; + // ... rest of the payload construction + return payloadData; +}; // Inside reqCreate functions - let payloadData = { ...draftData }; - payloadData.hierarchyType = hierarchyType; - // ... repeated code + let payloadData = constructPayloadData();Also applies to: 392-453
📜 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 (8)
health/micro-ui/web/micro-ui-internals/example/public/index.html(1 hunks)health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/campaign.scss(1 hunks)health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/coreOverride.scss(2 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/modules/campaign-manager/src/components/CampaignType.js(3 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/DateWithBoundary.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/SetupCampaign.js(1 hunks)health/micro-ui/web/public/index.html(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignType.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/DateWithBoundary.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/SetupCampaign.js (1)
Pattern **/*.js: check
🔇 Additional comments (7)
health/micro-ui/web/public/index.html (1)
13-13: LGTM! CSS version update looks correct.
The version bump from 0.1.28 to 0.1.29 follows semantic versioning for patch updates, suggesting bug fixes or minor improvements in the CSS.
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/campaign.scss (1)
47-48: LGTM! Please verify the visual spacing.
The margin adjustments align with the zero-margin pattern used in related description classes (.name-description, .dates-description), maintaining consistency in the layout.
Please verify that removing these margins doesn't create any undesirable spacing issues in the campaign interface, particularly:
- Between the description and adjacent elements
- In the overall vertical rhythm of the form
✅ Verification successful
Margin adjustments are consistent with the design pattern
After reviewing the complete campaign.scss file, the margin changes (0rem) in the .description-type class are consistent with the established pattern in the codebase, where related description classes (.name-description, .dates-description) also use 0rem margins. The spacing is properly managed through parent containers and other layout classes like .date-field-container and .selecting-boundary-div which provide the necessary spacing through padding and grid gaps.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any related description classes that might need similar margin adjustments
rg -l 'description.*margin' --type scss
Length of output: 70
Script:
#!/bin/bash
# Search for any related description classes that might need similar margin adjustments
# Using ripgrep without file type restriction since .scss extension isn't recognized
rg -l 'description.*margin'
Length of output: 303
Script:
#!/bin/bash
# Get the content of the campaign.scss file to understand the context better
cat "health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/campaign.scss"
Length of output: 2999
Script:
#!/bin/bash
# Search for other scss/css files in the same directory
find health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee -type f -name "*.scss" -o -name "*.css"
Length of output: 992
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/coreOverride.scss (1)
106-106: Verify the removal of bottom margin for headers.
The removal of bottom margin (margin-bottom: 0rem) might affect the vertical rhythm and spacing between elements. Please ensure this change doesn't negatively impact the layout and readability of content below headers.
✅ Verification successful
Setting margin-bottom to 0 is consistent with the codebase's styling patterns
The removal of bottom margin (margin-bottom: 0rem) for headers aligns with the codebase's styling patterns. The search results show multiple instances where similar zero-margin patterns are used for headers and typography elements across different components, particularly in:
- Login components
- Campaign components
- Core layout elements
- Form elements
This change maintains consistency with the existing styling approach in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any other margin-related styles for headers that might conflict
# or complement this change
# Search for header-related margin styles
rg -t scss "header.*margin|margin.*header"
# Search for any typography extensions that might affect headers
rg -t scss "typography.*heading|heading.*typography"
Length of output: 157
Script:
#!/bin/bash
# Let's try without the file type flag since scss isn't recognized
# Search for header-related margin styles in all files
rg "header.*margin|margin.*header" --type-add 'style:*.{scss,css}' -t style
# Look for any typography-related styles that might affect headers
rg "typography.*heading|heading.*typography" --type-add 'style:*.{scss,css}' -t style
# Also search for the specific class or element where this margin is being applied
rg "margin-bottom:\s*0" --type-add 'style:*.{scss,css}' -t style -C 3
Length of output: 51967
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignType.js (1)
2-2: Verify the Card component import change across the codebase.
The Card component import has been moved from digit-ui-react-components to digit-ui-components. This change might need to be consistent across other files using the Card component.
Also applies to: 5-5
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/DateWithBoundary.js (1)
Line range hint 252-294: Verify the search-field-container class exists in SCSS.
Since this appears to be a new className, we should verify it exists in the corresponding SCSS file.
✅ Verification successful
The search-field-container class exists and is properly defined
The class is defined in health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/index.scss with appropriate styling including border, padding, and background color properties. The implementation matches the usage in the component.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the search-field-container class definition
rg -t scss "search-field-container"
Length of output: 64
Script:
#!/bin/bash
# Search for the search-field-container class definition with ripgrep, checking all files
rg "search-field-container" --type-add 'styles:*.{scss,css,less,sass}' -t styles
Length of output: 190
Script:
#!/bin/bash
# Let's check the actual implementation of the search-field-container class
rg -A 5 "search-field-container" health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/index.scss
Length of output: 288
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/SetupCampaign.js (2)
Line range hint 711-714: Ensure displayMenu state is toggled correctly
When toggling displayMenu, ensure that it is of boolean type to prevent unexpected behavior during rendering.
Line range hint 48-51: Verify consistent usage of useCustomMDMS hook
The useCustomMDMS hook is used with different parameters. Ensure that the hook is called consistently and that the options provided match the expected signature to prevent unintended behavior.
Run the following script to check the usage:
Also applies to: 177-180
✅ Verification successful
The search results show that useCustomMDMS hook is used consistently across the codebase with the following pattern:
-
It always takes 4 parameters:
- tenantId
- moduleName
- masterDetails array (containing name and optional filter)
- select options (optional)
- schema options (optional)
-
The hook is used to fetch different types of MDMS configurations like:
- HierarchySchema
- Project Types
- Delivery Config
- Attribute Config
- etc.
Looking at the file in question (SetupCampaign.js), all three usages of useCustomMDMS (lines 61, 71, and 142) follow the same pattern and are consistent with the rest of the codebase:
- Line 61: Fetches HierarchySchema
- Line 71: Fetches DeliveryConfig
- Line 142: Fetches projectType
All calls include the required parameters and follow the same structure as other usages throughout the codebase.
The hook usage is consistent and follows the expected signature
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of useCustomMDMS and list their parameters for verification.
# Search for useCustomMDMS hook usage with context lines.
rg -A 5 -B 2 'useCustomMDMS' health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src --pretty
Length of output: 13493
health/micro-ui/web/micro-ui-internals/example/public/index.html
Outdated
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/coreOverride.scss
Show resolved
Hide resolved
...o-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignType.js
Show resolved
Hide resolved
...o-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignType.js
Show resolved
Hide resolved
.../web/micro-ui-internals/packages/modules/campaign-manager/src/components/DateWithBoundary.js
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/index.scss
Show resolved
Hide resolved
Summary by CodeRabbit
New Features
Bug Fixes
Style