BUGFIX/HCMPRE-1634: fix for versions #149 #2029
Conversation
Co-authored-by: Nipun Arora <[email protected]>
* Update MicroplanSearch.js * Pagination issues --------- Co-authored-by: Nipun Arora <[email protected]>
* Revert "Bugfix/hcmpre 1583 (#2021)" This reverts commit 201a0ce. * BUGFIX/HCMPRE-1634 Revert ing"updated core version (#2020)" * BUGFIX/HCMPRE-1583 fixes --------- Co-authored-by: Swathi-eGov <[email protected]>
FEATURE/HCMPRE-1634: Fixed all the version updates
📝 WalkthroughWalkthroughThis pull request involves updates to stylesheet links in two 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/MyMicroplans.js (1)
Line range hint
34-47: Consider a more targeted re-rendering approachWhile using a key on Fragment works for forcing re-renders, it might be more efficient to identify and re-render only the specific components that need updating when tabs change. This could improve performance by avoiding unnecessary re-renders of the entire tree.
Consider these alternatives:
- Move the key to the specific component that needs re-rendering
- Use React's
useEffectto handle tab-specific updates- Consider using React Query or similar solutions for data fetching and cache invalidation
🧰 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)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (9)
health/micro-ui/web/core/package.jsonis excluded by!**/*.jsonhealth/micro-ui/web/micro-ui-internals/example/package.jsonis excluded by!**/*.jsonhealth/micro-ui/web/micro-ui-internals/package.jsonis excluded by!**/*.jsonhealth/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/package.jsonis excluded by!**/*.jsonhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/package.jsonis excluded by!**/*.jsonhealth/micro-ui/web/micro-ui-internals/packages/modules/workbench-hcm/package.jsonis excluded by!**/*.jsonhealth/micro-ui/web/microplan/package.jsonis excluded by!**/*.jsonhealth/micro-ui/web/package.jsonis excluded by!**/*.jsonhealth/micro-ui/web/workbench/package.jsonis 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/MyMicroplans.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/MicroplanSearch.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 (4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/MyMicroplans.js (1)
23-23: LGTM: Clean state implementation
The new state variable follows React best practices and serves its purpose of managing component re-renders.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/MicroplanSearch.js (2)
Line range hint 27-33: Same issues as in MyMicroplans.js
This code segment has the same concerns regarding error handling and commented code as discussed in MyMicroplans.js.
Line range hint 35-47: Same re-rendering concerns as in MyMicroplans.js
This implementation has the same concerns regarding unnecessary re-renders as discussed in MyMicroplans.js.
health/micro-ui/web/micro-ui-internals/example/public/index.html (1)
14-15: Verify version compatibility and check for breaking changes.
Please ensure that:
- The version updates don't introduce breaking changes
- The versions are consistent with other dependencies
| const onTabChange = (n) => { | ||
|
|
||
| setTabData((prev) => prev.map((i, c) => ({ ...i, active: c === n ? true : false }))); //setting tab enable which is being clicked | ||
| // setConfig(TabSearchconfig?.TabSearchconfig?.[n]);// as per tab number filtering the config | ||
| const url = new URL(window.location.href); | ||
| url.searchParams.set("tabId", `${n}`); | ||
| window.history.replaceState({}, "", url); | ||
|
|
||
| setKey((prev)=>prev+1); | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider addressing the commented code and adding error handling
- The commented line
setConfig(TabSearchconfig?.TabSearchconfig?.[n])suggests incomplete functionality. Either implement it or remove if not needed. - The URL manipulation could benefit from error handling for cases where
window.locationmight be undefined (e.g., in tests).
Consider this improvement:
const onTabChange = (n) => {
setTabData((prev) => prev.map((i, c) => ({ ...i, active: c === n ? true : false })));
- // setConfig(TabSearchconfig?.TabSearchconfig?.[n]);
- const url = new URL(window.location.href);
- url.searchParams.set("tabId", `${n}`);
- window.history.replaceState({}, "", url);
+ try {
+ const url = new URL(window.location.href);
+ url.searchParams.set("tabId", `${n}`);
+ window.history.replaceState({}, "", url);
+ } catch (error) {
+ console.error("Error updating URL:", error);
+ }
setKey((prev)=>prev+1);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const onTabChange = (n) => { | |
| setTabData((prev) => prev.map((i, c) => ({ ...i, active: c === n ? true : false }))); //setting tab enable which is being clicked | |
| // setConfig(TabSearchconfig?.TabSearchconfig?.[n]);// as per tab number filtering the config | |
| const url = new URL(window.location.href); | |
| url.searchParams.set("tabId", `${n}`); | |
| window.history.replaceState({}, "", url); | |
| setKey((prev)=>prev+1); | |
| }; | |
| const onTabChange = (n) => { | |
| setTabData((prev) => prev.map((i, c) => ({ ...i, active: c === n ? true : false }))); | |
| try { | |
| const url = new URL(window.location.href); | |
| url.searchParams.set("tabId", `${n}`); | |
| window.history.replaceState({}, "", url); | |
| } catch (error) { | |
| console.error("Error updating URL:", error); | |
| } | |
| setKey((prev)=>prev+1); | |
| }; |
🧰 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)
| @@ -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.
🛠️ Refactor suggestion
Consider extracting tab logic into a custom hook
The key state and tab handling logic is duplicated between MyMicroplans.js and MicroplanSearch.js. This could be abstracted into a custom hook for better maintainability.
Consider creating a custom hook like this:
// useTabNavigation.js
export const useTabNavigation = (initialConfig) => {
const [key, setKey] = useState(0);
const [tabData, setTabData] = useState(initialConfig);
const onTabChange = useCallback((n) => {
setTabData((prev) => prev.map((i, c) => ({ ...i, active: c === n ? true : false })));
try {
const url = new URL(window.location.href);
url.searchParams.set("tabId", `${n}`);
window.history.replaceState({}, "", url);
} catch (error) {
console.error("Error updating URL:", error);
}
setKey((prev) => prev + 1);
}, []);
return { key, tabData, onTabChange };
};| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].43/dist/index.css" /> | ||
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].3/dist/index.css" /> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding integrity hashes and fallback sources for CDN resources.
While updating the CSS versions, consider implementing these security and reliability improvements:
- Add integrity hashes to verify the content hasn't been tampered with
- Include fallback sources in case the CDN is unavailable
Example implementation:
- <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" />
- <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" />
+ <link rel="stylesheet"
+ href="https://unpkg.com/@egovernments/[email protected]/dist/index.css"
+ integrity="sha384-[computed-hash]"
+ crossorigin="anonymous"
+ onerror="this.onerror=null;this.href='[fallback-url]';" />
+ <link rel="stylesheet"
+ href="https://unpkg.com/@egovernments/[email protected]/dist/index.css"
+ integrity="sha384-[computed-hash]"
+ crossorigin="anonymous"
+ onerror="this.onerror=null;this.href='[fallback-url]';" />Committable suggestion skipped: line range outside the PR's diff.
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].43/dist/index.css" /> | ||
| <!-- added below css for hcm-workbench module inclusion--> | ||
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> |
There was a problem hiding this comment.
Fix version inconsistency in digit-ui-health-css.
The version of digit-ui-health-css (0.2.2) is inconsistent with the version used in the example/public/index.html file (0.2.3). This could lead to styling inconsistencies.
Apply this update:
- <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" />
+ <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" />Also, consider implementing the same security improvements suggested for the other file (integrity hashes and fallback sources).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> | |
| <!-- added below css for hcm-workbench module inclusion--> | |
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].2/dist/index.css" /> | |
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> | |
| <!-- added below css for hcm-workbench module inclusion--> | |
| <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].3/dist/index.css" /> |
jagankumar-egov
changed the title
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
Documentation