-
Notifications
You must be signed in to change notification settings - Fork 22
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-0000 : fixed import issues of new ui-components #2153
Conversation
📝 WalkthroughWalkthroughThis pull request involves updates to multiple components across the micro-ui project, primarily focusing on component replacements and import changes. The modifications include switching from Changes
Possibly related PRs
Suggested Reviewers
Poem
✨ Finishing Touches
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: 2
🔭 Outside diff range comments (4)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/ConfigUploaderComponent.js (2)
Line range hint
58-67
: Clean up commented code and consider component consolidation.
- Remove the commented code for consistency.
- Consider refactoring LogoUploaderComponent and ConfigUploaderComponent into a single reusable component as they share nearly identical code.
Example refactor approach:
// FileUploaderComponent.js const FileUploaderComponent = ({ onSelect, componentName, uploadLabel, errorLabel, ...props }) => { // ... shared state and functions ... return ( <> <LabelFieldPair className={"uploader-label-field"}> <CardLabel>{t(uploadLabel)}</CardLabel> <FileUpload uploadedFiles={[]} variant="uploadField" onUpload={(files) => selectFile(files)} iserror={uploadErrorMessage} accept=".jpg, .png, .jpeg" /> </LabelFieldPair> {/* ... Toast component ... */} </> ); }; // Usage: <FileUploaderComponent componentName="LogoUploaderComponent" uploadLabel="LOGO_UPLOAD_LABEL" errorLabel="LOGO_UPLOAD_FAILED" onSelect={onSelect} {...props} />
Line range hint
1-82
: Consider implementing proper TypeScript types.Both components would benefit from TypeScript types for better maintainability and type safety, especially given their identical structure and prop usage.
Example type definitions:
interface FileUploaderProps { onSelect: ( componentName: string, data: { fileStoreId: string; type: string } ) => void; config?: { customProps?: { type: string; }; }; }micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js (1)
Line range hint
136-149
: Consider optimizing the sort function.The
sortDataByOrderNumber
function recursively sorts data but could be optimized:
- It mutates the input array directly
- The Infinity fallback might cause unexpected ordering
Consider this immutable approach:
-const sortDataByOrderNumber = (data) => { - data.sort((a, b) => { - const aOrder = a.orderNumber !== undefined ? a.orderNumber : Infinity; - const bOrder = b.orderNumber !== undefined ? b.orderNumber : Infinity; - return aOrder - bOrder; - }); - data.forEach((item) => { - if (item.children && item.children.length > 0) { - sortDataByOrderNumber(item.children); - } - }); - return data; +const sortDataByOrderNumber = (data) => { + return [...data].sort((a, b) => { + const aOrder = a.orderNumber ?? Number.MAX_SAFE_INTEGER; + const bOrder = b.orderNumber ?? Number.MAX_SAFE_INTEGER; + return aOrder - bOrder; + }).map(item => ({ + ...item, + children: item.children?.length > 0 + ? sortDataByOrderNumber(item.children) + : item.children + })); };micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/citizen/Home/UserProfile.js (1)
Line range hint
31-39
: Consider extracting validation configuration.The default validation configuration could be moved to a separate configuration file for better maintainability.
+// validationConfig.js +export const defaultValidationConfig = { + tenantId: `${Digit.ULBService.getStateId()}`, + UserProfileValidationConfig: [{ + name: "/^[a-zA-Z ]+$/i", + mobileNumber: "/^[6-9]{1}[0-9]{9}$/", + password: "/^([a-zA-Z0-9@#$%]{8,15})$/i", + }], +};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (5)
micro-ui/web/micro-ui-internals/example/package.json
is excluded by!**/*.json
micro-ui/web/micro-ui-internals/packages/modules/core/package.json
is excluded by!**/*.json
micro-ui/web/package.json
is excluded by!**/*.json
micro-ui/web/sandbox/package.json
is excluded by!**/*.json
micro-ui/web/workbench/package.json
is excluded by!**/*.json
📒 Files selected for processing (6)
micro-ui/web/micro-ui-internals/packages/modules/core/CHANGELOG.md
(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/CitizenSideBar.js
(2 hunks)micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js
(2 hunks)micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/citizen/Home/UserProfile.js
(2 hunks)micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/ConfigUploaderComponent.js
(2 hunks)micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/LogoUploaderComponent.js
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js (1)
Pattern **/*.js
: check
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/CitizenSideBar.js (1)
Pattern **/*.js
: check
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/LogoUploaderComponent.js (1)
Pattern **/*.js
: check
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/citizen/Home/UserProfile.js (1)
Pattern **/*.js
: check
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/ConfigUploaderComponent.js (1)
Pattern **/*.js
: check
🪛 Markdownlint (0.37.0)
micro-ui/web/micro-ui-internals/packages/modules/core/CHANGELOG.md
3-3: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
(MD022, blanks-around-headings)
4-4: null
Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
🪛 Biome (1.9.4)
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/citizen/Home/UserProfile.js
[error] 895-895: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (8)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/LogoUploaderComponent.js (2)
2-2
: LGTM: Import change aligns with UI component updates.The change from
Uploader
toFileUpload
aligns with the component replacement initiative.
Line range hint
55-64
: Clean up commented code and verify component behavior.A few suggestions:
- Remove the commented code as it's no longer needed and might cause confusion.
- Verify that setting
uploadedFiles
to an empty array doesn't affect the display of previously uploaded files.✅ Verification successful
Remove commented code as it's redundant with the useEffect implementation
The commented code can be safely removed because:
- The same file upload handling is already implemented in the useEffect hook
- The pattern of using empty array for
uploadedFiles
is consistent with other similar components in the codebase- Previously uploaded files are managed separately through the component's state management
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other instances of FileUpload to verify consistent usage rg -U "FileUpload.*uploadedFiles" --type jsLength of output: 45
Script:
#!/bin/bash # Search for FileUpload component usage with broader pattern echo "=== FileUpload Component Usage ===" rg -U "FileUpload" --type js -A 5 # Search for file upload related patterns echo -e "\n=== File Upload Patterns ===" rg -U "onUpload|handleUpload|uploadFile" --type js -A 3 # Search for FileUpload component definition echo -e "\n=== FileUpload Component Definition ===" fd -e js -e jsx FileUploadLength of output: 47266
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/tenantMgmt/ConfigUploaderComponent.js (1)
2-2
: LGTM: Import change consistent with LogoUploaderComponent.The change from
Uploader
toFileUpload
maintains consistency across components.micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/EmployeeSideBar.js (2)
2-2
: Component import updated correctly.The
SideNav
component has been imported from the correct package to replace the oldSidebar
component.
Line range hint
167-179
: Verify the SideNav component props.The component migration from
Sidebar
toSideNav
looks good, but some props have empty string values which might affect the component's behavior:
transitionDuration=""
expandedWidth=""
collapsedWidth=""
✅ Verification successful
Empty string props in SideNav component are acceptable
The
transitionDuration
,expandedWidth
, andcollapsedWidth
props are optional styling parameters. Empty string values will fall back to the component's internal defaults, which is acceptable behavior especially within a responsive layout.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for SideNav component usage to verify prop values ast-grep --pattern 'SideNav transitionDuration={$_}'Length of output: 54
Script:
#!/bin/bash # Search for SideNav component definition and usage echo "=== Looking for SideNav component definition ===" ast-grep --pattern 'export $_ function SideNav' ast-grep --pattern 'export $_ class SideNav' echo -e "\n=== Looking for SideNav imports ===" rg "import.*SideNav" -A 2 echo -e "\n=== Looking for SideNav usage patterns ===" rg "<SideNav" -A 5 -B 1Length of output: 2496
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/TopBarSideBar/SideBar/CitizenSideBar.js (2)
9-9
: Component import updated correctly.The
MobileSideNav
component has been imported from the correct package.
Line range hint
408-420
: Verify mobile navigation behavior.The mobile navigation implementation looks good, but ensure that the profile image handling is consistent with the new component's requirements.
✅ Verification successful
Profile image handling is implemented correctly ✅
The profile image implementation in MobileSideNav follows best practices for optional prop handling, using a safe conditional pattern that passes
undefined
when no profile image is available.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for MobileSideNav usage patterns ast-grep --pattern 'MobileSideNav profile={$_}'Length of output: 49
Script:
#!/bin/bash # Search for MobileSideNav component definition and usage ast-grep --pattern 'const MobileSideNav = $_' # Backup search using ripgrep echo "=== Searching for MobileSideNav usage ===" rg "MobileSideNav" -A 5 -B 5 # Search for the component file echo "=== Looking for MobileSideNav file ===" fd "MobileSideNav" --type fLength of output: 3504
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/citizen/Home/UserProfile.js (1)
12-12
: Component import updated correctly.The
Footer
component has been imported from the correct package.
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
Component Updates
MobileSidebar
withMobileSideNav
Sidebar
withSideNav
ActionBar
withFooter
Uploader
withFileUpload
Improvements