-
Notifications
You must be signed in to change notification settings - Fork 20
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
Feature/hcmpre 1634 #2027
base: master
Are you sure you want to change the base?
Feature/hcmpre 1634 #2027
Conversation
Co-authored-by: Nipun Arora <aroranipun1@gmail.com>
* Update MicroplanSearch.js * Pagination issues --------- Co-authored-by: Nipun Arora <aroranipun1@gmail.com>
FEATURE/HCMPRE-1634: Fixed all the version updates
📝 WalkthroughWalkthroughThis pull request involves updates to stylesheet links and React component state management across multiple files. The changes include incrementing versions of CSS stylesheets for Changes
Possibly related PRs
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (
|
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: 4
🔭 Outside diff range comments (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/MicroplanSearch.js (1)
Line range hint
27-31
: Consider using React Router for URL managementBoth components manually manage URL parameters. Consider using React Router's built-in URL management capabilities for a more maintainable solution.
Example implementation:
import { useSearchParams } from 'react-router-dom'; // Inside component const [searchParams, setSearchParams] = useSearchParams(); const onTabChange = (n) => { setSearchParams({ tabId: n }); // ... rest of the implementation };health/micro-ui/web/micro-ui-internals/example/public/index.html (1)
Line range hint
18-21
: Clean up commented stylesheet referencesRemove commented-out stylesheet links to improve code maintainability. If these are kept for reference, consider documenting version requirements in a separate configuration file or documentation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (9)
health/micro-ui/web/core/package.json
is excluded by!**/*.json
health/micro-ui/web/micro-ui-internals/example/package.json
is excluded by!**/*.json
health/micro-ui/web/micro-ui-internals/package.json
is excluded by!**/*.json
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/package.json
is excluded by!**/*.json
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/package.json
is excluded by!**/*.json
health/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/package.json
is excluded by!**/*.json
health/micro-ui/web/microplan/package.json
is excluded by!**/*.json
health/micro-ui/web/package.json
is excluded by!**/*.json
health/micro-ui/web/workbench/package.json
is excluded by!**/*.json
📒 Files selected for processing (4)
health/micro-ui/web/micro-ui-internals/example/public/index.html
(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/MicroplanSearch.js
(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/MyMicroplans.js
(1 hunks)health/micro-ui/web/public/index.html
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/MicroplanSearch.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/MyMicroplans.js (1)
Pattern **/*.js
: check
🪛 Biome (1.9.4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/MyMicroplans.js
[error] 26-26: 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 (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/MyMicroplans.js (1)
26-28
: Remove or implement commented code
The commented line setConfig(TabSearchconfig?.TabSearchconfig?.[n])
appears to be important for tab functionality. Either remove it if it's no longer needed or implement it properly.
🧰 Tools
🪛 Biome (1.9.4)
[error] 26-26: 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)
@@ -20,18 +20,18 @@ const MyMicroplans = () => { | |||
// Set default values when component mounts | |||
setDefaultValues(defaultSearchValues); | |||
}, []); | |||
|
|||
const [key,setKey] = useState(0); |
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 alternative approaches to force re-rendering
The addition of a key
state to force component re-rendering might indicate underlying issues with component lifecycle or state management. While this approach works, it's generally considered an anti-pattern as it can lead to unnecessary re-renders.
Consider these alternatives:
- Use proper state management to trigger specific component updates
- Implement
useCallback
for the tab change handler - Move the tab-specific content to a separate component
Would you like assistance in implementing any of these alternatives?
Also applies to: 31-31, 34-34
@@ -16,6 +16,7 @@ const MicroplanSearch = () => { | |||
const [tabData, setTabData] = useState( | |||
TabSearchconfig?.TabSearchconfig?.map((configItem, index) => ({ key: index, label: configItem.label, active: index === 0 ? true : false })) | |||
); // setting number of tab component and making first index enable as default | |||
const [key,setKey] = useState(0) |
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
Extract shared tab management logic
This component implements the same tab management and re-rendering strategy as MyMicroplans.js
. Consider extracting this shared functionality into a custom hook or higher-order component.
Here's a suggested implementation using a custom hook:
// useTabManagement.js
export const useTabManagement = (TabSearchconfig) => {
const [key, setKey] = useState(0);
const [tabData, setTabData] = useState(
TabSearchconfig?.TabSearchconfig?.map((configItem, index) => ({
key: index,
label: configItem.label,
active: index === 0
}))
);
const onTabChange = useCallback((n) => {
setTabData((prev) => prev.map((i, c) => ({ ...i, active: c === n })));
const url = new URL(window.location.href);
url.searchParams.set("tabId", `${n}`);
window.history.replaceState({}, "", url);
setKey((prev) => prev + 1);
}, []);
return { key, tabData, onTabChange };
};
This would reduce duplication and improve maintainability. Would you like assistance in implementing this refactor?
Also applies to: 32-32, 35-35
<link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-components-css@0.0.2-beta.43/dist/index.css" /> | ||
<link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-health-css@0.2.3/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 adding integrity hashes for CDN resources
While updating to newer versions, it's recommended to add integrity hashes for the CDN resources to prevent potential security risks from CDN tampering:
- <link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-components-css@0.0.2-beta.43/dist/index.css" />
- <link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-health-css@0.2.3/dist/index.css" />
+ <link rel="stylesheet"
+ href="https://unpkg.com/@egovernments/digit-ui-components-css@0.0.2-beta.43/dist/index.css"
+ integrity="sha384-[computed-hash]"
+ crossorigin="anonymous" />
+ <link rel="stylesheet"
+ href="https://unpkg.com/@egovernments/digit-ui-health-css@0.2.3/dist/index.css"
+ integrity="sha384-[computed-hash]"
+ crossorigin="anonymous" />
Committable suggestion skipped: line range outside the PR's diff.
@@ -8,7 +8,7 @@ | |||
href="https://fonts.googleapis.com/css2?family=Roboto+Condensed:wght@400;500;700&family=Roboto:wght@400;500;700&display=swap" | |||
rel="stylesheet" type="text/css" /> | |||
<link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-css@1.8.3/dist/index.css" /> | |||
<link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-components-css@0.0.2-beta.42/dist/index.css" /> | |||
<link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-components-css@0.0.2-beta.43/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.
💡 Codebase verification
Version inconsistency confirmed - Update required
The version mismatch is confirmed between the files:
health/micro-ui/web/public/index.html
usesdigit-ui-health-css@0.2.2
- Example file uses
digit-ui-health-css@0.2.3
Additionally, the package source in packages/css/package.json
is at version 0.2.2
, indicating that the example file is ahead of both the main implementation and the package source. This inconsistency should be resolved by:
- Either updating the main implementation and package source to
0.2.3
- Or downgrading the example file to
0.2.2
to match the package source
🔗 Analysis chain
Version mismatch detected between HTML files
There's an inconsistency in the digit-ui-health-css
versions:
- This file uses version
0.2.2
- The file in
example/public/index.html
uses version0.2.3
This could lead to styling inconsistencies across different parts of the application.
Let's verify if this version mismatch is intentional by checking for any version constraints in the project:
Also applies to: 13-13
Let me gather more information to verify the version inconsistency by checking the actual content of both HTML files:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for digit-ui-health-css version references in package.json files
find . -name "package.json" -type f -exec sh -c 'echo "=== $1 ==="; cat "$1" | grep -A 5 -B 5 "digit-ui-health-css"' sh {} \;
Length of output: 2410
Script:
#!/bin/bash
# Check digit-ui-health-css version in both HTML files
echo "=== health/micro-ui/web/public/index.html ==="
grep "digit-ui-health-css" health/micro-ui/web/public/index.html
echo -e "\n=== example/public/index.html ==="
find . -name "index.html" -path "*/example/public/*" -type f -exec grep "digit-ui-health-css" {} \;
Length of output: 565
Choose the appropriate template for your PR:
Feature PR
Feature Request
JIRA ID
Module
Description
Related Issues
Bugfix PR
Bugfix Request
JIRA ID
Module
Description
Root Cause
Related Issues
Release PR
Summary by CodeRabbit
New Features
Bug Fixes