side bar with correct order #1394
Conversation
WalkthroughWalkthroughThe changes in the 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: 0
Outside diff range and nitpick comments (1)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js (1)
93-94: Consider moving thehostUrldeclaration inside theelseblockSince
hostUrlis not used within theif (isMultiRootTenant)block, you can improve clarity by declaring it only where it's needed.Apply this diff to adjust the
hostUrldeclaration:let updatedUrl = null; if(isMultiRootTenant){ url = url.replace("/sandbox-ui/employee", `/sandbox-ui/${tenantId}/employee`); updatedUrl = url; } else{ + const hostUrl = window.location.origin; updatedUrl = DIGIT_UI_CONTEXTS?.every((e) => url?.indexOf(`/${e}`) === -1) ? hostUrl + "/employee/" + url : hostUrl + url; }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js (1 hunks)
Additional context used
Path-based instructions (1)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js (1)
Pattern
**/*.js: check
Additional comments not posted (1)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js (1)
94-94: LGTMThe assignment
updatedUrl = url;correctly adjusts the URL for multi-root tenant scenarios. This ensures that the navigation URL reflects the tenant-specific path without adding unnecessary prefixes.
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (1)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js (1)
12-12: UnusedmobileViewPropThe
mobileViewprop is destructured frompropsbut is not used anywhere in the component. Consider removing it if it's unnecessary or implement its intended functionality.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js (2 hunks)
Additional context used
Path-based instructions (1)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js (1)
Pattern
**/*.js: check
...o-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js
Outdated
Show resolved
Hide resolved
...o-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js (2)
75-76: Use a More Robust Check forleftIconCurrently, the condition checks if
item?.leftIconis not an empty string. This might overlook cases whereleftIconisnullorundefined. To ensure all falsy values are handled, consider modifying the condition to check for the existence ofleftIcon.Apply this diff to make the check more robust:
- if (key === "item" && item?.leftIcon !== "") { + if (key === "item" && item?.leftIcon) {
Line range hint
126-140: Ensure Consistent Indentation and FormattingThere are inconsistencies in the indentation within the JSX returned by the component. Consistent formatting improves readability and maintainability. Consider using a formatter like Prettier to automatically format the code.
Example of formatted code:
return ( <MediaQuery minWidth={768}> <Sidebar items={transformedData} hideAccessbilityTools={true} onSelect={({ item, index, parentIndex }) => onItemSelect({ item, index, parentIndex })} theme={"dark"} variant={"primary"} transitionDuration={""} className="" styles={{}} expandedWidth="" collapsedWidth="" onBottomItemClick={() => {}} /> </MediaQuery> );
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js (2 hunks)
Additional context used
Path-based instructions (1)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js (1)
Pattern
**/*.js: check
...o-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js
Outdated
Show resolved
Hide resolved
mithunhegde-egov
dismissed stale reviews from Swathi-eGov and coderabbitai[bot]
via
28a517c
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js (5 hunks)
Additional context used
Path-based instructions (1)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js (1)
Pattern
**/*.js: check
Additional comments not posted (1)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js (1)
5-6: Good use ofMediaQueryfor responsive designWrapping the
Sidebarcomponent withMediaQueryenhances the user experience by rendering it only on screens wider than 768 pixels.
| <MediaQuery minWidth={768}> | ||
| <Sidebar | ||
| items={sortedTransformedData} | ||
| hideAccessbilityTools={true} |
There was a problem hiding this comment.
Typo in prop name: hideAccessbilityTools should be hideAccessibilityTools
The prop name hideAccessbilityTools is misspelled, which may cause the prop to be ineffective.
Apply this diff to correct the typo:
- hideAccessbilityTools={true}
+ hideAccessibilityTools={true}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.
| hideAccessbilityTools={true} | |
| hideAccessibilityTools={true} |
| @@ -117,6 +119,7 @@ const EmployeeSideBar = () => { | |||
| label: value.item.displayName, | |||
| icon: { icon: value.item.leftIcon, width: "1.5rem", height: "1.5rem" }, | |||
| navigationUrl: value.item.navigationURL, | |||
| orderNumber:value.item.orderNumber, | |||
There was a problem hiding this comment.
Handle undefined orderNumber in transformed items
When adding orderNumber to each item in transformData, ensure that value.item.orderNumber is defined. If it's undefined, the sorting function may not behave as expected.
Apply this diff to provide a default value:
navigationUrl: value.item.navigationURL,
- orderNumber: value.item.orderNumber,
+ orderNumber: value.item.orderNumber || 0,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.
| orderNumber:value.item.orderNumber, | |
| navigationUrl: value.item.navigationURL, | |
| orderNumber: value.item.orderNumber || 0, |
| const sortDataByOrderNumber = (data) => { | ||
| // Sort the current level of data by orderNumber | ||
| data.sort((a, b) => { | ||
| return a.orderNumber - b.orderNumber; | ||
| }); | ||
|
|
||
| // Recursively sort the children if they exist | ||
| data.forEach((item) => { | ||
| if (item.children && item.children.length > 0) { | ||
| sortDataByOrderNumber(item.children); | ||
| } | ||
| }); | ||
|
|
||
| return data; | ||
| }; |
There was a problem hiding this comment.
Ensure robust sorting in sortDataByOrderNumber function
To prevent potential issues when orderNumber is undefined or not a number, consider handling these cases to ensure the sort operation functions correctly.
Apply this diff to handle undefined or non-numeric orderNumber values:
data.sort((a, b) => {
- return a.orderNumber - b.orderNumber;
+ return (a.orderNumber || 0) - (b.orderNumber || 0);
});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 sortDataByOrderNumber = (data) => { | |
| // Sort the current level of data by orderNumber | |
| data.sort((a, b) => { | |
| return a.orderNumber - b.orderNumber; | |
| }); | |
| // Recursively sort the children if they exist | |
| data.forEach((item) => { | |
| if (item.children && item.children.length > 0) { | |
| sortDataByOrderNumber(item.children); | |
| } | |
| }); | |
| return data; | |
| }; | |
| const sortDataByOrderNumber = (data) => { | |
| // Sort the current level of data by orderNumber | |
| data.sort((a, b) => { | |
| return (a.orderNumber || 0) - (b.orderNumber || 0); | |
| }); | |
| // Recursively sort the children if they exist | |
| data.forEach((item) => { | |
| if (item.children && item.children.length > 0) { | |
| sortDataByOrderNumber(item.children); | |
| } | |
| }); | |
| return data; | |
| }; |
side bar url redirection