Moving inbox search composer: Updated react components and workbench version #263
Conversation
WalkthroughWalkthroughThe recent updates across the micro-ui project encompass improvements in UI components and session management. Changes involve style adjustments, enhanced logic for button states and file uploads, and improved handling of user sessions and search functionalities. The updates also include transitioning to local sources for component imports and refining hooks for API interactions. Changes
Poem
Possibly related issues
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional comments not posted (1)
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 15
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
micro-ui/web/micro-ui-internals/packages/modules/workbench/package.jsonis excluded by!**/*.jsonmicro-ui/web/micro-ui-internals/packages/react-components/package.jsonis excluded by!**/*.json
Files selected for processing (28)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/CheckBox.js (2 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/HorizontalNavV2.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/InboxSearchLinks.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/MultiSelectDropdown.js (5 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/RemovableTagNew.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/SearchComponent.js (6 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/SubmitBar.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/Table.js (6 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/UploadFile.js (5 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/FormComposerV2.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/InboxSearchComposer.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/InboxSearchComposerReducer.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/InboxSearchComposerV2.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/MobileView/MobileSearchComponent.js (7 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/MobileView/MobileSearchResultsv1.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/RemovableTags.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/ResultsTable.js (8 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/index.js (2 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/ApiCheckboxes.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/ApiDropdown.js (2 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/CustomDropdown.js (4 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/DateRangeNew.js (3 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/DetailsCard.js (3 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/FilterAction.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/MultiUploadWrapper.js (3 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/RenderFormFields.js (8 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/SearchAction.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/WorkflowStatusFilter.js (2 hunks)
Additional Context Used
Path-based Instructions (28)
micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/SearchAction.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/molecules/FilterAction.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/atoms/RemovableTagNew.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/atoms/SubmitBar.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/molecules/ApiCheckboxes.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/atoms/InboxSearchLinks.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/molecules/WorkflowStatusFilter.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/atoms/CheckBox.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/molecules/ApiDropdown.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/atoms/HorizontalNavV2.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/hoc/InboxSearchComposerReducer.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/hoc/MobileView/MobileSearchResultsv1.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/molecules/DetailsCard.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/atoms/MultiSelectDropdown.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/molecules/CustomDropdown.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/molecules/MultiUploadWrapper.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/hoc/RemovableTags.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/molecules/DateRangeNew.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/atoms/UploadFile.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/atoms/SearchComponent.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/hoc/MobileView/MobileSearchComponent.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/atoms/Table.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/hoc/ResultsTable.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/hoc/InboxSearchComposer.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/index.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/molecules/RenderFormFields.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/hoc/InboxSearchComposerV2.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/hoc/FormComposerV2.js (1)
Pattern
**/*.js: check
Additional comments not posted (45)
micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/SearchAction.js (1)
2-6: Ensure the specified fill color forFilterIconaligns with the project's design guidelines.micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/FilterAction.js (1)
2-8: Ensure the specified fill color forSortSvgaligns with the project's design guidelines.micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/RemovableTagNew.js (1)
4-14: Consider adding PropTypes forRemovableTagNewto document and validate the expected shape of props, especially for thetextprop which is expected to be an object.micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/SubmitBar.js (1)
15-15: Ensure that theSubmitBarcomponent behaves as expected whenheaderStyleis not provided, to avoid any unintended side effects.micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/ApiCheckboxes.js (1)
8-46: Ensure that error handling is in place for the API call inApiCheckboxesand that the component has a graceful fallback in case of errors or empty data.micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/InboxSearchLinks.js (1)
1-1: Ensure that the removal of unused imports inInboxSearchLinksdoes not affect other parts of the application.micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/WorkflowStatusFilter.js (1)
1-39: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [6-54]
Ensure accurate mapping of status data in
WorkflowStatusFilterand proper handling of null or emptyinboxResponseto avoid any unintended behavior.micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/CheckBox.js (1)
1-15: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [5-55]
Conduct accessibility testing on the
CheckBoxcomponent to ensure that the changes do not negatively impact users, especially those relying on screen readers.micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/ApiDropdown.js (2)
16-16: Consider renaming the variablereqCriteriato more accurately reflect its purpose, such asrequestCriteria, for improved code readability.
45-45: Ensure the localization keys used with thetfunction are defined in your localization files to avoid missing translations.micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/HorizontalNavV2.js (2)
1-4: Ensure theuseStatehook is actually used within the component. If not, consider removing the import to keep the code clean.
36-36: Verify if the commented-out code is necessary for future reference. If not, consider removing it to maintain code cleanliness.micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/InboxSearchComposerReducer.js (1)
16-22: Ensure the initial state structure aligns with the expected state management strategy for the inbox search composer. Consider adding comments to describe the purpose of each state property for better maintainability.micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/MobileView/MobileSearchResultsv1.js (2)
24-33: Ensure the functionconvertRowToDetailCardDataproperly handles all edge cases and formats the data as expected for theDetailsCardcomponent. Consider adding error handling for unexpected data structures.
76-76: Verify the custom logic for generating the link to push to history inhandleDetailCardClickandhandleSelectfunctions. Ensure it aligns with the application's routing strategy.micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/DetailsCard.js (2)
19-19: Ensure the new propsmodeandapiDetailsare documented in the component's PropTypes or TypeScript interface to improve type safety and developer experience.
114-114: Consider using a more descriptive variable name thanmodeto indicate the purpose of this prop, such asdisplayModeorcardMode.micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/MultiSelectDropdown.js (2)
5-5: Ensure the new propsisSurvey,placeholder, anddisableare documented in the component's PropTypes or TypeScript interface for better type safety and developer experience.
90-90: Validate theisDisabledproperty of options before rendering to ensure that disabled options are handled correctly in the UI.Verification successful
The search across the entire codebase for the
isDisabledproperty has yielded multiple results, including the specific file and line of interest (micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/MultiSelectDropdown.js). This confirms that theisDisabledproperty is indeed utilized within theMultiSelectDropdowncomponent to conditionally disable options based on their state. This aligns with the review comment's intention to ensure that disabled options are handled correctly in the UI.Given this confirmation, the review comment regarding the validation of the
isDisabledproperty before rendering to ensure correct UI handling of disabled options is verified to be consistent with the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Assuming 'optionsData.js' contains the options data. Adjust the file name as necessary. rg 'isDisabled' ./src/data/optionsData.jsLength of output: 107
Script:
#!/bin/bash # Broad search for 'isDisabled' property across JavaScript and JSON files in the src directory rg 'isDisabled' ./src --type js --type jsonLength of output: 89
Script:
#!/bin/bash # Generalized search for 'isDisabled' property across the entire codebase without assuming specific directory paths rg 'isDisabled' --type js --type jsonLength of output: 9611
micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/CustomDropdown.js (2)
66-66: Ensure themdmsv2prop is clearly documented and its purpose is explained, as it affects the behavior of theuseCustomMDMShook.
96-96: Verify the logic for disabling options based onconfig.optionsDisableandconfig.defaultOptions. Ensure it aligns with the expected behavior for all use cases.Verification successful
The logic within
CustomDropdown.jsfor disabling options based onconfig.optionsDisableandconfig.defaultOptionshas been verified. It correctly checks if these properties are truthy and then selectively disables options inconfig.optionsthat match criteria specified inconfig.defaultOptions. This implementation aligns with the expected behavior for selectively disabling dropdown options based on the provided configuration.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Assuming 'configData.js' contains the configuration data. Adjust the file name as necessary. rg 'optionsDisable' ./src/data/configData.jsLength of output: 109
Script:
#!/bin/bash # Search the entire codebase for references to 'optionsDisable' to locate the configuration data. rg 'optionsDisable'Length of output: 326
Script:
#!/bin/bash # Inspect 'CustomDropdown.js' to understand the logic related to 'config.optionsDisable' and 'config.defaultOptions'. fd 'CustomDropdown.js' --exec cat {}Length of output: 5931
micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/MultiUploadWrapper.js (2)
57-57: Ensure theallowedFileTypesRegexprop is correctly configured to match the supported file types. Consider adding a comment to document the supported file types for future reference.
136-136: Validate the use of theerror-msgclass for styling error messages. Ensure that it aligns with the application's CSS conventions.micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/UploadFile.js (1)
209-232: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [197-225]
Refactor conditional rendering for improved readability.
- {!hasFile || props.error ? ( - <h2 className="file-upload-status">{props.message}</h2> - ) : ( + {props.error ? ( + <h2 className="file-upload-status error">{props.message}</h2> + ) : hasFile ? ( <div className="tag-container" style={extraStyles ? extraStyles?.tagContainerStyles : null}> <div className="tag" style={extraStyles ? extraStyles?.tagStyles : null}> <span className="text" style={extraStyles ? extraStyles?.textStyles : null}> {(typeof inpRef.current.files[0]?.name !== "undefined") && !(props?.file) ? inpRef.current.files[0]?.name : props.file?.name} </span> <span onClick={() => handleDelete()} style={extraStyles ? extraStyles?.closeIconStyles : null}> <Close style={props.Multistyle} className="close" /> </span> </div> </div> - )} + ) : null}Refactor the conditional rendering logic for the file upload status and tag container to separate the concerns of displaying an error message and showing the uploaded file's name. This enhances code readability and maintainability.
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/Table.js (3)
1-16: Verify the removal of unused imports and features.
Ensure that the removal of features such as checkboxes, toast notifications, and action bars is intentional and aligns with the component's updated requirements. If these features are no longer needed, the current changes are appropriate. Otherwise, consider reintroducing or refactoring these features to meet the new component specifications.
41-52: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [21-49]
Refine the props structure for clarity and maintainability.
Consider documenting the props with comments or using TypeScript for type definitions to enhance code readability and maintainability. Clear documentation or type definitions can help developers understand the purpose and usage of each prop, especially for complex components likeTable.
64-81: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [67-98]
Optimize the useTable hook configuration.
- manualSortBy: autoSort ? false : true, + manualSortBy: !autoSort,Simplify the assignment of
manualSortByby directly negating theautoSortvalue. This change enhances code readability without altering the functionality.micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/ResultsTable.js (6)
12-12: Ensure the newly added propstype,activeLink, andbrowserSessionare documented in the component's prop types for better type checking and developer experience.
16-17: Consider using React'suseReducerfor managing complex state logic involving multiple sub-states likeshowResultsTableand session management, to improve code readability and maintainability.
66-66: Ensure thedisableSortByproperty is properly documented in the component's prop types or in the documentation for theTablecomponent, to clarify its purpose and usage.
84-84: Validate the logic for setting default values from the session inuseForm. Ensure that the session data structure aligns with the form's expectations to prevent potential issues.
140-141: Ensure that theregisterfunction calls foroffsetandlimitare necessary and correctly implemented. If these values are always expected to be part of the form, consider initializing them in theuseFormhook'sdefaultValues.
190-200: Verify the logic for pagination and sorting in theTablecomponent, especially the calculation forcurrentPageand the handling ofpageSizeLimit. Ensure these calculations align with the expected behavior and data structure.micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/InboxSearchComposer.js (1)
23-23: Ensure the removal of theconfigsparameter from theinitialInboxStatefunction call in theuseReducerhook does not impact the initial state setup, especially ifconfigswas previously used to set initial values.micro-ui/web/micro-ui-internals/packages/react-components/src/index.js (1)
274-274: Ensure thatInboxSearchComposerV2is properly documented and tested, given its addition to the exports.micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/RenderFormFields.js (3)
17-17: Add a brief comment explaining the purpose of theisFalsyOrEmptyfunction for better code readability.
163-164: Verify that the addition ofmdmsv2andpropsproperties to theCustomDropdowncomponent aligns with the component's expected behavior and usage.
250-271: Ensure that the new case for"apicheckboxes"in the switch statement is thoroughly tested, especially the dynamic rendering based on API data.micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/InboxSearchComposerV2.js (4)
37-37: Ensure that theInboxSearchComposerV2component's props are validated using PropTypes or TypeScript for type safety and better documentation.
86-88: Double-check the logic for enabling the request criteria. Ensure that the conditions for settingenableto true are correctly capturing all intended scenarios.
136-136: If customizations are applied to the request criteria, ensure that they do not inadvertently remove or alter essential parameters required for the API call.
139-139: Confirm that theuseCustomAPIHookhook properly handles all states (loading, error, data) and that these states are correctly reflected in the UI.micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/FormComposerV2.js (3)
34-34: Ensure that the new local import path forButtonis correct and that the component is properly exported from the specified location.Verification successful
The verification process has confirmed the existence of the
Button.jsfile at the specified path within themicro-ui/web/micro-ui-internals/packages/react-components/src/atomsdirectory. This aligns with the import statement provided in the review comment, ensuring that the new local import path forButtonis correct.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that Button component exists at the specified path fd "Button.js" micro-ui/web/micro-ui-internals/packages/react-components/src/atoms --exec stat {}Length of output: 315
Script:
#!/bin/bash # Verify that Button component exists at the specified path by listing the file fd "Button.js" micro-ui/web/micro-ui-internals/packages/react-components/src/atomsLength of output: 577
34-34: Since you've changed the import source for theButtoncomponent, verify that all instances ofButtonusage in this file are compatible with the newly imported component's API.
34-34: To enhance code maintainability, consider abstracting complex logic or repeated patterns into smaller, reusable functions or components.
micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/FormComposerV2.js
Show resolved
Hide resolved
| const result = { ..._.get(apiDetails, apiDetails?.searchFormJsonPath, {}), ...state.searchForm } | ||
| Object.keys(result).forEach(key => { | ||
| if (!result[key]) delete result[key] | ||
| }); | ||
| _.set(apiDetails, apiDetails?.searchFormJsonPath, result) | ||
| } |
There was a problem hiding this comment.
Consider using a more functional approach with lodash's _.omitBy to clean up falsy values in the result object, which can simplify the logic.
- Object.keys(result).forEach(key => {
- if (!result[key]) delete result[key]
- });
+ const cleanedResult = _.omitBy(result, _.isEmpty);
+ _.set(apiDetails, apiDetails?.searchFormJsonPath, cleanedResult)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.
| const result = { ..._.get(apiDetails, apiDetails?.searchFormJsonPath, {}), ...state.searchForm } | |
| Object.keys(result).forEach(key => { | |
| if (!result[key]) delete result[key] | |
| }); | |
| _.set(apiDetails, apiDetails?.searchFormJsonPath, result) | |
| } | |
| const result = { ..._.get(apiDetails, apiDetails?.searchFormJsonPath, {}), ...state.searchForm } | |
| const cleanedResult = _.omitBy(result, _.isEmpty); | |
| _.set(apiDetails, apiDetails?.searchFormJsonPath, cleanedResult) | |
| } |
| const generateTagsFromFields = (fields, sessionData, t,data) => { | ||
| //filetering the fields | ||
|
|
||
| const fieldsToShow = fields | ||
| ?.filter((row) => row?.removableTagConf) | ||
| ?.map((row) => row?.removableTagConf); | ||
|
|
||
| const crumbs = []; | ||
| fieldsToShow?.forEach((field, idx) => { | ||
| //one field can have multiple crumbs | ||
| // we need to fill | ||
|
|
||
| //setting the text | ||
| const value = _.get(sessionData, field.sessionJsonPath, ''); | ||
| if (!value || value?.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| //convert this to switch case and write a separate fn for it | ||
| switch (field?.type) { | ||
| case 'multi': | ||
| value?.forEach((val) => { | ||
| crumbs?.push({ | ||
| label: t(field.label) || '', | ||
| value: `${t( | ||
| Digit.Utils.locale.getTransformedLocale( | ||
| _.get(val, field.valueJsonPath, '') | ||
| ) | ||
| )}`, | ||
| removableTagConf: { | ||
| ...field, | ||
| value: val, | ||
| }, |
There was a problem hiding this comment.
Consider refactoring the tag generation logic to improve readability and maintainability.
- //convert this to switch case and write a separate fn for it
- switch (field?.type) {
+ const generateCrumbsForField = (field, value) => {
+ switch (field?.type) {Refactor the tag generation logic into a separate function to enhance code readability and maintainability. This approach allows for easier updates and testing of the tag generation logic.
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.
| const generateTagsFromFields = (fields, sessionData, t,data) => { | |
| //filetering the fields | |
| const fieldsToShow = fields | |
| ?.filter((row) => row?.removableTagConf) | |
| ?.map((row) => row?.removableTagConf); | |
| const crumbs = []; | |
| fieldsToShow?.forEach((field, idx) => { | |
| //one field can have multiple crumbs | |
| // we need to fill | |
| //setting the text | |
| const value = _.get(sessionData, field.sessionJsonPath, ''); | |
| if (!value || value?.length === 0) { | |
| return; | |
| } | |
| //convert this to switch case and write a separate fn for it | |
| switch (field?.type) { | |
| case 'multi': | |
| value?.forEach((val) => { | |
| crumbs?.push({ | |
| label: t(field.label) || '', | |
| value: `${t( | |
| Digit.Utils.locale.getTransformedLocale( | |
| _.get(val, field.valueJsonPath, '') | |
| ) | |
| )}`, | |
| removableTagConf: { | |
| ...field, | |
| value: val, | |
| }, | |
| const generateTagsFromFields = (fields, sessionData, t,data) => { | |
| //filetering the fields | |
| const fieldsToShow = fields | |
| ?.filter((row) => row?.removableTagConf) | |
| ?.map((row) => row?.removableTagConf); | |
| const crumbs = []; | |
| fieldsToShow?.forEach((field, idx) => { | |
| //one field can have multiple crumbs | |
| // we need to fill | |
| //setting the text | |
| const value = _.get(sessionData, field.sessionJsonPath, ''); | |
| if (!value || value?.length === 0) { | |
| return; | |
| } | |
| const generateCrumbsForField = (field, value) => { | |
| switch (field?.type) { |
| return stateObj; | ||
| case 'workflowStatusFilter': | ||
| //if we are here then we have dynamic ids to delete from state | ||
| _.set(stateObj,`${removableTagConf?.sessionJsonPath}.${removableTagConf?.dynamicId}`, false) |
There was a problem hiding this comment.
Use template literals for constructing dynamic object paths to improve code readability.
- _.set(stateObj,`${removableTagConf?.sessionJsonPath}.${removableTagConf?.dynamicId}`, false)
+ _.set(stateObj, `${removableTagConf?.sessionJsonPath}.${removableTagConf?.dynamicId}`, false)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.
| _.set(stateObj,`${removableTagConf?.sessionJsonPath}.${removableTagConf?.dynamicId}`, false) | |
| _.set(stateObj, `${removableTagConf?.sessionJsonPath}.${removableTagConf?.dynamicId}`, false) |
| if(!data || !value || Object?.keys(value)?.length===0){ | ||
| return | ||
| } | ||
| const statusIds = Object?.keys(value)?.map(key => value[key] ? key : false)?.filter(val => val) | ||
| const statusObj = data?.statusMap?.map(status => { | ||
| if(statusIds?.includes(status?.statusid)){ | ||
| return { | ||
| ...status | ||
| } | ||
| }else { | ||
| return false | ||
| } | ||
| })?.filter(val => val) | ||
| statusObj?.forEach(obj => { | ||
| crumbs?.push({ | ||
| label: t(field.label) || '', | ||
| value: field?.valuePrefix ? t(Digit.Utils.locale.getTransformedLocale(`${field.valuePrefix}${obj?.applicationstatus}`)):t(Digit.Utils.locale.getTransformedLocale(obj?.applicationstatus)) , | ||
| removableTagConf: { ...field, dynamicId:obj.statusid }, | ||
| }); | ||
| }) | ||
| break; |
There was a problem hiding this comment.
Optimize the handling of workflow status filters.
- const statusObj = data?.statusMap?.map(status => {
- if(statusIds?.includes(status?.statusid)){
- return {
- ...status
- }
- }else {
- return false
- }
- })?.filter(val => val)
+ const statusObj = data?.statusMap?.filter(status => statusIds?.includes(status?.statusid));Simplify the mapping and filtering of statusObj by directly filtering data?.statusMap based on statusIds. This reduces complexity and improves performance by eliminating unnecessary mapping and filtering steps.
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.
| if(!data || !value || Object?.keys(value)?.length===0){ | |
| return | |
| } | |
| const statusIds = Object?.keys(value)?.map(key => value[key] ? key : false)?.filter(val => val) | |
| const statusObj = data?.statusMap?.map(status => { | |
| if(statusIds?.includes(status?.statusid)){ | |
| return { | |
| ...status | |
| } | |
| }else { | |
| return false | |
| } | |
| })?.filter(val => val) | |
| statusObj?.forEach(obj => { | |
| crumbs?.push({ | |
| label: t(field.label) || '', | |
| value: field?.valuePrefix ? t(Digit.Utils.locale.getTransformedLocale(`${field.valuePrefix}${obj?.applicationstatus}`)):t(Digit.Utils.locale.getTransformedLocale(obj?.applicationstatus)) , | |
| removableTagConf: { ...field, dynamicId:obj.statusid }, | |
| }); | |
| }) | |
| break; | |
| if(!data || !value || Object?.keys(value)?.length===0){ | |
| return | |
| } | |
| const statusIds = Object?.keys(value)?.map(key => value[key] ? key : false)?.filter(val => val) | |
| const statusObj = data?.statusMap?.filter(status => statusIds?.includes(status?.statusid)); | |
| statusObj?.forEach(obj => { | |
| crumbs?.push({ | |
| label: t(field.label) || '', | |
| value: field?.valuePrefix ? t(Digit.Utils.locale.getTransformedLocale(`${field.valuePrefix}${obj?.applicationstatus}`)):t(Digit.Utils.locale.getTransformedLocale(obj?.applicationstatus)) , | |
| removableTagConf: { ...field, dynamicId:obj.statusid }, | |
| }); | |
| }) | |
| break; |
| startDate: typeof values?.startDate === "string" ? new Date(values?.startDate) : values?.startDate, | ||
| endDate: typeof values?.endDate === "string" ? new Date(values?.endDate) : values?.endDate |
There was a problem hiding this comment.
Ensure proper handling of date strings.
- startDate: typeof values?.startDate === "string" ? new Date(values?.startDate) : values?.startDate,
- endDate: typeof values?.endDate === "string" ? new Date(values?.endDate) : values?.endDate
+ startDate: values?.startDate ? new Date(values?.startDate) : null,
+ endDate: values?.endDate ? new Date(values?.endDate) : nullRefactor the conversion of startDate and endDate to Date objects to handle cases where values?.startDate or values?.endDate might be falsy. This ensures that null is used for the date values when they are not provided, maintaining consistency and avoiding potential errors with invalid date objects.
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.
| startDate: typeof values?.startDate === "string" ? new Date(values?.startDate) : values?.startDate, | |
| endDate: typeof values?.endDate === "string" ? new Date(values?.endDate) : values?.endDate | |
| startDate: values?.startDate ? new Date(values?.startDate) : null, | |
| endDate: values?.endDate ? new Date(values?.endDate) : null |
| maxDate={populators?.maxDate} | ||
| minDate={populators?.minDate} | ||
| startDatePlaceholder={t("EVENTS_START_DATE_LABEL")} | ||
| endDatePlaceholder={t("EVENTS_END_DATE_LABEL")} |
There was a problem hiding this comment.
Validate maxDate and minDate prop usage.
- maxDate={populators?.maxDate}
- minDate={populators?.minDate}
+ maxDate={populators?.maxDate ? new Date(populators?.maxDate) : undefined}
+ minDate={populators?.minDate ? new Date(populators?.minDate) : undefined}Ensure that maxDate and minDate are converted to Date objects if they are provided, and fallback to undefined if not. This prevents potential issues with invalid date formats and enhances the robustness of date range handling.
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.
| maxDate={populators?.maxDate} | |
| minDate={populators?.minDate} | |
| startDatePlaceholder={t("EVENTS_START_DATE_LABEL")} | |
| endDatePlaceholder={t("EVENTS_END_DATE_LABEL")} | |
| maxDate={populators?.maxDate ? new Date(populators?.maxDate) : undefined} | |
| minDate={populators?.minDate ? new Date(populators?.minDate) : undefined} | |
| startDatePlaceholder={t("EVENTS_START_DATE_LABEL")} | |
| endDatePlaceholder={t("EVENTS_END_DATE_LABEL")} |
| const SearchComponent = ({ uiConfig, header = "", screenType = "search", fullConfig, data,activeLink,setActiveLink,browserSession}) => { | ||
|
|
||
| //whenever activeLink changes we'll change uiConfig | ||
| // const [activeLink,setActiveLink] = useState(uiConfig?.configNavItems?.filter(row=>row.activeByDefault)?.[0]?.name) | ||
| const [navConfig,setNavConfig] = useState(uiConfig?.configNavItems) | ||
| const [allUiConfigs,setAllUiConfigs] = useState(setUIConf(uiConfig)) |
There was a problem hiding this comment.
Ensure proper initialization of state variables.
- const [navConfig,setNavConfig] = useState(uiConfig?.configNavItems)
- const [allUiConfigs,setAllUiConfigs] = useState(setUIConf(uiConfig))
+ const [navConfig, setNavConfig] = useState(uiConfig?.configNavItems || [])
+ const [allUiConfigs, setAllUiConfigs] = useState(setUIConf(uiConfig) || [])Initialize navConfig and allUiConfigs state variables with default empty arrays if uiConfig?.configNavItems or setUIConf(uiConfig) return falsy values. This ensures that the component does not encounter runtime errors when accessing these state variables.
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.
| const SearchComponent = ({ uiConfig, header = "", screenType = "search", fullConfig, data,activeLink,setActiveLink,browserSession}) => { | |
| //whenever activeLink changes we'll change uiConfig | |
| // const [activeLink,setActiveLink] = useState(uiConfig?.configNavItems?.filter(row=>row.activeByDefault)?.[0]?.name) | |
| const [navConfig,setNavConfig] = useState(uiConfig?.configNavItems) | |
| const [allUiConfigs,setAllUiConfigs] = useState(setUIConf(uiConfig)) | |
| const SearchComponent = ({ uiConfig, header = "", screenType = "search", fullConfig, data,activeLink,setActiveLink,browserSession}) => { | |
| //whenever activeLink changes we'll change uiConfig | |
| // const [activeLink,setActiveLink] = useState(uiConfig?.configNavItems?.filter(row=>row.activeByDefault)?.[0]?.name) | |
| const [navConfig, setNavConfig] = useState(uiConfig?.configNavItems || []) | |
| const [allUiConfigs, setAllUiConfigs] = useState(setUIConf(uiConfig) || []) |
| const MobileSearchComponent = ({ uiConfig, modalType, header = "", screenType = "search", fullConfig, data, onClose, defaultValues,browserSession }) => { | ||
| const { t } = useTranslation(); | ||
| const { state, dispatch } = useContext(InboxContext) | ||
| const [showToast,setShowToast] = useState(null) | ||
| let updatedFields = []; | ||
| const {apiDetails} = fullConfig | ||
|
|
||
|
|
||
| if (fullConfig?.postProcessResult){ | ||
| //conditions can be added while calling postprocess function to pass different params | ||
| Digit?.Customizations?.[apiDetails?.masterName]?.[apiDetails?.moduleName]?.postProcess(data, uiConfig) | ||
| } | ||
|
|
||
| //define session for modal form | ||
| const mobileSearchSession = Digit.Hooks.useSessionStorage("MOBILE_SEARCH_MODAL_FORM", | ||
| defaultValues | ||
| ); | ||
| const [sessionFormData, setSessionFormData, clearSessionFormData] = mobileSearchSession; | ||
| //uiConfig.type === filter || sort | ||
| //we need to sync browsersession and mobileSearchSession | ||
| // const mobileSearchSession = Digit.Hooks.useSessionStorage(`MOBILE_SEARCH_MODAL_FORM_${uiConfig?.type}_${fullConfig?.label}`, | ||
| // {...uiConfig?.defaultValues} | ||
| // ); | ||
|
|
||
| // const [sessionFormData, setSessionFormData, clearSessionFormData] = mobileSearchSession; | ||
| const [session,setSession,clearSession] = browserSession || [] | ||
|
|
There was a problem hiding this comment.
Validate session handling logic for mobile search component.
- const [session,setSession,clearSession] = browserSession || []
+ const [session, setSession, clearSession] = browserSession || [null, noop, noop]Ensure that the fallback values for setSession and clearSession are no-operation functions (noop) instead of empty arrays. This prevents potential runtime errors when attempting to call these functions if browserSession is not provided.
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.
| const MobileSearchComponent = ({ uiConfig, modalType, header = "", screenType = "search", fullConfig, data, onClose, defaultValues,browserSession }) => { | |
| const { t } = useTranslation(); | |
| const { state, dispatch } = useContext(InboxContext) | |
| const [showToast,setShowToast] = useState(null) | |
| let updatedFields = []; | |
| const {apiDetails} = fullConfig | |
| if (fullConfig?.postProcessResult){ | |
| //conditions can be added while calling postprocess function to pass different params | |
| Digit?.Customizations?.[apiDetails?.masterName]?.[apiDetails?.moduleName]?.postProcess(data, uiConfig) | |
| } | |
| //define session for modal form | |
| const mobileSearchSession = Digit.Hooks.useSessionStorage("MOBILE_SEARCH_MODAL_FORM", | |
| defaultValues | |
| ); | |
| const [sessionFormData, setSessionFormData, clearSessionFormData] = mobileSearchSession; | |
| //uiConfig.type === filter || sort | |
| //we need to sync browsersession and mobileSearchSession | |
| // const mobileSearchSession = Digit.Hooks.useSessionStorage(`MOBILE_SEARCH_MODAL_FORM_${uiConfig?.type}_${fullConfig?.label}`, | |
| // {...uiConfig?.defaultValues} | |
| // ); | |
| // const [sessionFormData, setSessionFormData, clearSessionFormData] = mobileSearchSession; | |
| const [session,setSession,clearSession] = browserSession || [] | |
| const MobileSearchComponent = ({ uiConfig, modalType, header = "", screenType = "search", fullConfig, data, onClose, defaultValues,browserSession }) => { | |
| const { t } = useTranslation(); | |
| const { state, dispatch } = useContext(InboxContext) | |
| const [showToast,setShowToast] = useState(null) | |
| let updatedFields = []; | |
| const {apiDetails} = fullConfig | |
| if (fullConfig?.postProcessResult){ | |
| //conditions can be added while calling postprocess function to pass different params | |
| Digit?.Customizations?.[apiDetails?.masterName]?.[apiDetails?.moduleName]?.postProcess(data, uiConfig) | |
| } | |
| //define session for modal form | |
| //uiConfig.type === filter || sort | |
| //we need to sync browsersession and mobileSearchSession | |
| // const mobileSearchSession = Digit.Hooks.useSessionStorage(`MOBILE_SEARCH_MODAL_FORM_${uiConfig?.type}_${fullConfig?.label}`, | |
| // {...uiConfig?.defaultValues} | |
| // ); | |
| // const [sessionFormData, setSessionFormData, clearSessionFormData] = mobileSearchSession; | |
| const [session, setSession, clearSession] = browserSession || [null, noop, noop] | |
| switch(uiConfig?.typeMobile) { | ||
| case "filter" : { | ||
| return ( | ||
| <div className="popup-label" style={{ display: "flex", paddingBottom: "20px" }}> | ||
| <span className="header" style={{ display : "flex" }}> | ||
| <span className="icon" style ={{ marginRight: "12px", marginTop: "5px", paddingBottom: "3px" }}><FilterIcon/></span> | ||
| <span style ={{ fontSize: "large" }}>{t("ES_COMMON_FILTER_BY")}:</span> | ||
| <span style ={{ fontSize: "1.5rem", fontWeight:"700" ,marginRight: "12px" }}>{t(`${uiConfig?.headerLabel || "TQM_INBOX_SORTBY"}`)}</span> | ||
| <span className="clear-search refresh-icon-container" onClick={clearSearch}><RefreshIcon/></span> | ||
| </span> | ||
| <span className="clear-search" onClick={clearSearch}><RefreshIcon/></span> | ||
| {/* <span className="clear-search" onClick={clearSearch}><RefreshIcon/></span> */} | ||
| <span onClick={onClose}> | ||
| <CloseSvg /> | ||
| </span> | ||
| </div> | ||
| ) | ||
| } | ||
| case "sort" : { | ||
| return ( | ||
| <div className="popup-label" style={{ display: "flex", paddingBottom: "20px" }}> | ||
| <span className="header" style={{ display : "flex" }}> | ||
| <span className="icon" style ={{ marginRight: "12px", marginTop: "5px", paddingBottom: "3px" }}><SortSvg/></span> | ||
| <span style ={{ fontSize: "1.5rem", fontWeight:"700" ,marginRight: "12px" }}>{t(`${uiConfig?.headerLabel || "TQM_INBOX_SORTBY"}`)}</span> | ||
| <span className="clear-search refresh-icon-container" onClick={clearSearch}><RefreshIcon/></span> | ||
| </span> | ||
| <span onClick={onClose}> | ||
| <CloseSvg /> | ||
| </span> | ||
| </div> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Refactor header rendering logic for clarity.
- switch(uiConfig?.typeMobile) {
+ const renderHeaderContent = () => {
+ let IconComponent = null;
+ let headerLabel = t(`${uiConfig?.headerLabel || "TQM_INBOX_SORTBY"}`);
+ switch(uiConfig?.typeMobile) {
+ case "filter":
+ IconComponent = FilterIcon;
+ break;
+ case "sort":
+ IconComponent = SortSvg;
+ break;
+ default:
+ IconComponent = SearchIcon;
+ }
+ return (
+ <div className="popup-label" style={{ display: "flex", paddingBottom: "20px" }}>
+ <span className="header" style={{ display : "flex" }}>
+ <span className="icon" style ={{ marginRight: "12px", marginTop: "5px", paddingBottom: "3px" }}><IconComponent/></span>
+ <span style ={{ fontSize: "1.5rem", fontWeight:"700" ,marginRight: "12px" }}>{headerLabel}</span>
+ <span className="clear-search refresh-icon-container" onClick={clearSearch}><RefreshIcon/></span>
+ </span>
+ <span onClick={onClose}>
+ <CloseSvg />
+ </span>
+ </div>
+ );
+ };Refactor the header rendering logic into a separate function renderHeaderContent to improve readability and maintainability. This approach centralizes the decision-making logic for selecting the icon and label based on uiConfig.typeMobile, making the component easier to understand and modify.
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.
| switch(uiConfig?.typeMobile) { | |
| case "filter" : { | |
| return ( | |
| <div className="popup-label" style={{ display: "flex", paddingBottom: "20px" }}> | |
| <span className="header" style={{ display : "flex" }}> | |
| <span className="icon" style ={{ marginRight: "12px", marginTop: "5px", paddingBottom: "3px" }}><FilterIcon/></span> | |
| <span style ={{ fontSize: "large" }}>{t("ES_COMMON_FILTER_BY")}:</span> | |
| <span style ={{ fontSize: "1.5rem", fontWeight:"700" ,marginRight: "12px" }}>{t(`${uiConfig?.headerLabel || "TQM_INBOX_SORTBY"}`)}</span> | |
| <span className="clear-search refresh-icon-container" onClick={clearSearch}><RefreshIcon/></span> | |
| </span> | |
| <span className="clear-search" onClick={clearSearch}><RefreshIcon/></span> | |
| {/* <span className="clear-search" onClick={clearSearch}><RefreshIcon/></span> */} | |
| <span onClick={onClose}> | |
| <CloseSvg /> | |
| </span> | |
| </div> | |
| ) | |
| } | |
| case "sort" : { | |
| return ( | |
| <div className="popup-label" style={{ display: "flex", paddingBottom: "20px" }}> | |
| <span className="header" style={{ display : "flex" }}> | |
| <span className="icon" style ={{ marginRight: "12px", marginTop: "5px", paddingBottom: "3px" }}><SortSvg/></span> | |
| <span style ={{ fontSize: "1.5rem", fontWeight:"700" ,marginRight: "12px" }}>{t(`${uiConfig?.headerLabel || "TQM_INBOX_SORTBY"}`)}</span> | |
| <span className="clear-search refresh-icon-container" onClick={clearSearch}><RefreshIcon/></span> | |
| </span> | |
| <span onClick={onClose}> | |
| <CloseSvg /> | |
| </span> | |
| </div> | |
| ) | |
| } | |
| const renderHeaderContent = () => { | |
| let IconComponent = null; | |
| let headerLabel = t(`${uiConfig?.headerLabel || "TQM_INBOX_SORTBY"}`); | |
| switch(uiConfig?.typeMobile) { | |
| case "filter": | |
| IconComponent = FilterIcon; | |
| break; | |
| case "sort": | |
| IconComponent = SortSvg; | |
| break; | |
| default: | |
| IconComponent = SearchIcon; | |
| } | |
| return ( | |
| <div className="popup-label" style={{ display: "flex", paddingBottom: "20px" }}> | |
| <span className="header" style={{ display : "flex" }}> | |
| <span className="icon" style ={{ marginRight: "12px", marginTop: "5px", paddingBottom: "3px" }}><IconComponent/></span> | |
| <span style ={{ fontSize: "1.5rem", fontWeight:"700" ,marginRight: "12px" }}>{headerLabel}</span> | |
| <span className="clear-search refresh-icon-container" onClick={clearSearch}><RefreshIcon/></span> | |
| </span> | |
| <span onClick={onClose}> | |
| <CloseSvg /> | |
| </span> | |
| </div> | |
| ); | |
| }; |
There was a problem hiding this comment.
@nipunarora-eGov check again some changes are getting removed, will lead issue to others
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/CheckBox.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/MultiSelectDropdown.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/SearchComponent.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/SearchComponent.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/Table.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (2)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/CheckBox.js (2 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/MultiSelectDropdown.js (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/MultiSelectDropdown.js
Additional Context Used
Path-based Instructions (1)
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/CheckBox.js (1)
Pattern
**/*.js: check
Additional comments not posted (2)
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/CheckBox.js (2)
12-12: Ensure the inline style for the label (maxWidth: "80%", marginLeft: "10px") aligns with the design requirements. Consider using a CSS class for better maintainability and to avoid inline styles when possible.
49-49: The conditional style for the custom checkbox (props?.checkboxWidth ? {...props?.checkboxWidth} : null) introduces a dependency onprops.checkboxWidthfor styling. Ensure this prop is documented and validated, or consider a more generic approach to styling customization.
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/CheckBox.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (4)
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/InboxSearchComposer.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/InboxSearchComposerReducer.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/InboxSearchComposerV2.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/ResultsTable.js (8 hunks)
Files skipped from review as they are similar to previous changes (2)
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/InboxSearchComposer.js
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/InboxSearchComposerV2.js
Additional Context Used
Path-based Instructions (2)
micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/InboxSearchComposerReducer.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/hoc/ResultsTable.js (1)
Pattern
**/*.js: check
Additional comments not posted (2)
micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/InboxSearchComposerReducer.js (1)
63-93: Consider the performance implications of using_.cloneDeepfor state cloning in the reducer.
While_.cloneDeepis a powerful utility, it can be computationally expensive for large or complex state objects. If performance becomes a concern, explore alternative methods to achieve the desired state manipulation without deep cloning, or ensure that the state structure is optimized for shallow copying where possible.micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/ResultsTable.js (1)
40-69: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [57-83]
The table column configuration logic is correctly implemented and makes good use of
useMemofor optimization.
| function removeObjectFromArray(array, key, value) { | ||
| // Find the index of the object with the specified key and value | ||
| const indexToRemove = array.findIndex((obj) => obj[key] === value); | ||
|
|
||
| // If the object is found, remove it from the array | ||
| if (indexToRemove !== -1) { | ||
| array.splice(indexToRemove, 1); | ||
| } | ||
|
|
||
| // Return the modified array | ||
| return array; | ||
| } |
There was a problem hiding this comment.
Consider using a functional programming approach to remove an object from an array without mutating the original array.
- if (indexToRemove !== -1) {
- array.splice(indexToRemove, 1);
- }
+ return indexToRemove !== -1 ? [...array.slice(0, indexToRemove), ...array.slice(indexToRemove + 1)] : array;This change avoids direct mutation of the input array, aligning with functional programming principles and potentially reducing side effects.
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.
| function removeObjectFromArray(array, key, value) { | |
| // Find the index of the object with the specified key and value | |
| const indexToRemove = array.findIndex((obj) => obj[key] === value); | |
| // If the object is found, remove it from the array | |
| if (indexToRemove !== -1) { | |
| array.splice(indexToRemove, 1); | |
| } | |
| // Return the modified array | |
| return array; | |
| } | |
| function removeObjectFromArray(array, key, value) { | |
| // Find the index of the object with the specified key and value | |
| const indexToRemove = array.findIndex((obj) => obj[key] === value); | |
| // Return the modified array without mutating the original array | |
| return indexToRemove !== -1 ? [...array.slice(0, indexToRemove), ...array.slice(indexToRemove + 1)] : array; | |
| } |
micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/InboxSearchComposerReducer.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (2)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/Table.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/InboxSearchComposerV2.js (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/Table.js
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/InboxSearchComposerV2.js
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/CheckBox.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/MultiSelectDropdown.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/SearchComponent.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/SearchComponent.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (2)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/MultiSelectDropdown.js (5 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/SearchComponent.js (6 hunks)
Files skipped from review as they are similar to previous changes (1)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/MultiSelectDropdown.js
Additional Context Used
Path-based Instructions (1)
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/SearchComponent.js (1)
Pattern
**/*.js: check
Additional comments not posted (2)
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/SearchComponent.js (2)
1-1: Verify the usage ofuseMemoanduseWatchimports.If these hooks are not used within the component, consider removing them to avoid unused import warnings.
228-231: Dynamic rendering based onactiveLinkenhances UI flexibility.The conditional rendering of form fields based on
activeLinkis a significant improvement, allowing for dynamic UI changes and enhancing user experience.
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/SearchComponent.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/SearchComponent.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (1)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/CheckBox.js (2 hunks)
Additional Context Used
Path-based Instructions (1)
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/CheckBox.js (1)
Pattern
**/*.js: check
Additional comments not posted (2)
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/CheckBox.js (2)
5-5: Therefprop is included but not used within the component. Consider removing it or usingReact.forwardRefif needed.
56-56: Ensure translation keys like"COMMON_CERTIFY_ONE"are correctly defined in your localization files.
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/CheckBox.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/CheckBox.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/UploadFile.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/UploadFile.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/UploadFile.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/FormComposerV2.js
Outdated
Show resolved
Hide resolved
...web/micro-ui-internals/packages/react-components/src/hoc/MobileView/MobileSearchComponent.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/ResultsTable.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/ResultsTable.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/ResultsTable.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/CustomDropdown.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/MultiUploadWrapper.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (4)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/UploadFile.js (5 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/FormComposerV2.js (2 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/ResultsTable.js (8 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/MultiUploadWrapper.js (2 hunks)
Files skipped from review as they are similar to previous changes (3)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/UploadFile.js
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/FormComposerV2.js
- micro-ui/web/micro-ui-internals/packages/react-components/src/molecules/MultiUploadWrapper.js
Additional Context Used
Path-based Instructions (1)
micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/ResultsTable.js (1)
Pattern
**/*.js: check
Additional comments not posted (6)
micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/ResultsTable.js (6)
12-17: Ensure proper handling ofbrowserSessionprop when it's not provided.Consider adding a default value or a check to ensure
setSessionandclearSessiondo not cause errors whenbrowserSessionisundefined.
44-53: Refactor theuseEffecthook to simplify the conditional logic for settingshowResultsTable.This comment has been previously addressed. Ensure the refactoring improves readability and maintainability.
40-69: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [57-83]
Optimize the
useMemohook for definingtableColumns.Consider extracting complex logic within the
Cellrenderer into separate functions for clarity and maintainability.
101-116: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [84-113]
Verify the handling of form default values and session updates.
Ensure that the form's default values are correctly set from the session or custom pagination settings and that updates to the session correctly reset the form.
175-175: Simplify the conditional rendering logic for better readability.This comment has been previously addressed. Ensure the rendering logic is concise and easy to understand.
191-201: Ensure theTablecomponent's props are correctly configured for sorting and pagination.Verify that the sorting and pagination functionalities work as expected with the provided props. Consider adding checks or fallbacks for potential edge cases.
There was a problem hiding this comment.
Actionable comments posted: 4
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (3)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/UploadFile.js (6 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/MobileView/MobileSearchComponent.js (7 hunks)
- micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/ResultsTable.js (9 hunks)
Files skipped from review as they are similar to previous changes (1)
- micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/UploadFile.js
Additional Context Used
Path-based Instructions (2)
micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/MobileView/MobileSearchComponent.js (1)
Pattern
**/*.js: checkmicro-ui/web/micro-ui-internals/packages/react-components/src/hoc/ResultsTable.js (1)
Pattern
**/*.js: check
Additional comments not posted (7)
micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/MobileView/MobileSearchComponent.js (4)
5-5: Consider consolidating SVG imports for efficiency.Grouping SVG imports from the same source can reduce import statements and improve code readability.
12-12: AddedButtonimport.This addition supports the new button handling functionality introduced in the component.
15-15: AddedbrowserSessionparameter toMobileSearchComponent.This change enables session handling within the component, enhancing its functionality.
35-35: Validate session handling logic for mobile search component.Ensure that the fallback values for
setSessionandclearSessionare no-operation functions (noop) instead of empty arrays. This prevents potential runtime errors when attempting to call these functions ifbrowserSessionis not provided.micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/ResultsTable.js (3)
12-12: Introduced additional props toResultsTable.The addition of
type,activeLink, andbrowserSessionprops enhances the component's functionality and flexibility.
16-16: Added logic to show/hide the results table.This update improves the user experience by conditionally rendering the results table based on search form parameters.
76-76: AddeddisableSortByproperty to table columns.This enhancement allows for more control over the sorting behavior of individual columns, improving the component's usability.
Also applies to: 86-86
...web/micro-ui-internals/packages/react-components/src/hoc/MobileView/MobileSearchComponent.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/ResultsTable.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (1)
- micro-ui/web/micro-ui-internals/packages/libraries/src/hooks/useCustomMDMS.js (2 hunks)
Additional Context Used
Path-based Instructions (1)
micro-ui/web/micro-ui-internals/packages/libraries/src/hooks/useCustomMDMS.js (1)
Pattern
**/*.js: check
Additional comments not posted (3)
micro-ui/web/micro-ui-internals/packages/libraries/src/hooks/useCustomMDMS.js (3)
3-5: Ensure that the lodash (_) import is utilized within the file; if not, consider removing it to optimize bundle size.
30-30: Consider documenting themdmsv2parameter to clarify its purpose and usage within theuseCustomMDMShook.
31-64: The implementation for handling themdmsv2parameter looks solid. Verify the performance and correctness of the API call and response processing to ensure it meets the application's requirements.
micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/ResultsTable.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/ResultsTable.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/UploadFile.js
Show resolved
Hide resolved
| import { Loader } from '../atoms/Loader'; | ||
| import NoResultsFound from '../atoms/NoResultsFound'; | ||
| import { InfoIcon,EditIcon } from "../atoms/svgindex"; | ||
|
|
||
| const ResultsTable = ({ tableContainerClass, config,data,isLoading,isFetching,fullConfig,revalidate,additionalConfig }) => { | ||
| const ResultsTable = ({ tableContainerClass, config,data,isLoading,isFetching,fullConfig,revalidate,type,activeLink,browserSession,additionalConfig }) => { |
There was a problem hiding this comment.
Ensure the destructuring of browserSession is handled safely.
- const [session,setSession,clearSession] = browserSession || []
+ const [session,setSession,clearSession] = browserSession || [undefined, undefined, undefined]This change prevents potential runtime errors when browserSession is undefined.
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.
| const ResultsTable = ({ tableContainerClass, config,data,isLoading,isFetching,fullConfig,revalidate,type,activeLink,browserSession,additionalConfig }) => { | |
| const ResultsTable = ({ tableContainerClass, config,data,isLoading,isFetching,fullConfig,revalidate,type,activeLink,browserSession,additionalConfig }) => { | |
| const [session,setSession,clearSession] = browserSession || [undefined, undefined, undefined] |
| useEffect(() => { | ||
| if(apiDetails?.minParametersForSearchForm !== 0 && Object.keys(state.searchForm).length > 0 && !Object.keys(state.searchForm).some(key => state.searchForm[key]!=="") && type==="search" && activeLink?.minParametersForSearchForm !== 0){ | ||
| setShowResultsTable(false) | ||
| } | ||
| // else{ | ||
| // setShowResultsTable(true) | ||
| // } | ||
| return ()=>{ | ||
| setShowResultsTable(true) | ||
| } |
There was a problem hiding this comment.
Refactor the useEffect hook to simplify the conditional logic for setting showResultsTable.
- useEffect(() => {
- if(apiDetails?.minParametersForSearchForm !== 0 && Object.keys(state.searchForm).length > 0 && !Object.keys(state.searchForm).some(key => state.searchForm[key]!=="") && type==="search" && activeLink?.minParametersForSearchForm !== 0){
- setShowResultsTable(false)
- }
- return ()=>{
- setShowResultsTable(true)
- }
- }, [state])
+ useEffect(() => {
+ const shouldShow = !(apiDetails?.minParametersForSearchForm !== 0 && Object.keys(state.searchForm).length > 0 && !Object.keys(state.searchForm).some(key => state.searchForm[key]!=="") && type==="search" && activeLink?.minParametersForSearchForm !== 0);
+ setShowResultsTable(shouldShow);
+ }, [apiDetails, state.searchForm, type, activeLink])This refactor simplifies the logic by directly setting the state based on the condition and ensures all dependencies are correctly listed.
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.
| useEffect(() => { | |
| if(apiDetails?.minParametersForSearchForm !== 0 && Object.keys(state.searchForm).length > 0 && !Object.keys(state.searchForm).some(key => state.searchForm[key]!=="") && type==="search" && activeLink?.minParametersForSearchForm !== 0){ | |
| setShowResultsTable(false) | |
| } | |
| // else{ | |
| // setShowResultsTable(true) | |
| // } | |
| return ()=>{ | |
| setShowResultsTable(true) | |
| } | |
| useEffect(() => { | |
| const shouldShow = !(apiDetails?.minParametersForSearchForm !== 0 && Object.keys(state.searchForm).length > 0 && !Object.keys(state.searchForm).some(key => state.searchForm[key]!=="") && type==="search" && activeLink?.minParametersForSearchForm !== 0); | |
| setShowResultsTable(shouldShow); | |
| }, [apiDetails, state.searchForm, type, activeLink]) |
| @@ -162,6 +187,7 @@ const ResultsTable = ({ tableContainerClass, config,data,isLoading,isFetching,fu | |||
|
|
|||
| if (isLoading || isFetching ) return <Loader /> | |||
| if(!data) return <></> | |||
| if(!showResultsTable) return <></> | |||
There was a problem hiding this comment.
Simplify the conditional rendering logic for better readability.
- if(!showResultsTable) return <></>
- if (searchResult?.length === 0) return <NoResultsFound/>
+ if(!showResultsTable || searchResult?.length === 0) return searchResult?.length === 0 ? <NoResultsFound/> : <></>This change combines the conditions into a single line, improving readability and reducing redundancy.
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.
| if(!showResultsTable) return <></> | |
| if(!showResultsTable || searchResult?.length === 0) return searchResult?.length === 0 ? <NoResultsFound/> : <></> |
| const setUIConf = (uiConfig) => { | ||
| if(uiConfig.additionalTabs) | ||
| return [{...uiConfig},...uiConfig?.additionalTabs] | ||
| return [{uiConfig}] | ||
| } |
There was a problem hiding this comment.
Ensure immutability in setUIConf function.
- return [{uiConfig},...uiConfig?.additionalTabs]
+ return [{...uiConfig},...uiConfig?.additionalTabs.map(tab => ({...tab}))]This change ensures that the original uiConfig and its additionalTabs are not mutated, which could lead to unintended side effects.
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.
| const setUIConf = (uiConfig) => { | |
| if(uiConfig.additionalTabs) | |
| return [{...uiConfig},...uiConfig?.additionalTabs] | |
| return [{uiConfig}] | |
| } | |
| const setUIConf = (uiConfig) => { | |
| if(uiConfig.additionalTabs) | |
| return [{...uiConfig},...uiConfig?.additionalTabs.map(tab => ({...tab}))] | |
| return [{uiConfig}] | |
| } |
| useEffect(() => { | ||
| clearSearch() | ||
| }, [activeLink]) |
There was a problem hiding this comment.
Add cleanup logic to useEffect hooks to prevent potential memory leaks.
useEffect(() => {
const timeoutId = setTimeout(clearSearch, 3000);
+ return () => clearTimeout(timeoutId);
}, [activeLink])This addition ensures that any pending timeouts are cleared if the component unmounts before they complete.
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.
| useEffect(() => { | |
| clearSearch() | |
| }, [activeLink]) | |
| useEffect(() => { | |
| const timeoutId = setTimeout(clearSearch, 3000); | |
| return () => clearTimeout(timeoutId); | |
| }, [activeLink]) |
There was a problem hiding this comment.
Actionable comments posted: 2
Actionable comments outside the diff hunks (5)
micro-ui/web/micro-ui-internals/packages/libraries/README.md (3)
Line range hint
3-3: Capitalize the first letter of the section title for consistency.- # digit-ui-svg-components + # Digit-ui-svg-components
Line range hint
19-19: Consider adding a comma after "DIGIT-UI's" for grammatical correctness.- This Package is more specific to DIGIT-UI's can be used across mission's + This Package is more specific to DIGIT-UI's, can be used across mission's
Line range hint
63-63: Remove duplicate mention of "DIGIT Frontend" to improve clarity.- ### Published from DIGIT Frontend DIGIT Frontend Repo (https://github.com/egovernments/Digit-Frontend/tree/develop) + ### Published from DIGIT Frontend Repo (https://github.com/egovernments/Digit-Frontend/tree/develop)micro-ui/web/micro-ui-internals/packages/react-components/README.md (2)
Line range hint
5-5: Capitalize the first letter of the section title for consistency.- # digit-ui-react-components + # Digit-ui-react-components
Line range hint
183-183: Remove duplicate mention of "DIGIT Frontend" to improve clarity.- ### Published from DIGIT Frontend DIGIT Frontend Repo (https://github.com/egovernments/Digit-Frontend/tree/develop) + ### Published from DIGIT Frontend Repo (https://github.com/egovernments/Digit-Frontend/tree/develop)
micro-ui/web/micro-ui-internals/packages/react-components/README.md
Outdated
Show resolved
Hide resolved
…ME.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Actionable comments outside the diff hunks (3)
micro-ui/web/micro-ui-internals/packages/libraries/README.md (3)
Line range hint
3-3: Capitalize the first letter of the section title.- # digit-ui-svg-components + # Digit-ui-svg-components
Line range hint
19-19: Consider adding a comma for better readability.- This Package is more specific to DIGIT-UI's can be used across mission's + This Package is more specific to DIGIT-UI's, can be used across mission's
Line range hint
63-63: Remove the duplicated phrase to improve clarity.- ### Published from DIGIT Frontend DIGIT Frontend Repo (https://github.com/egovernments/Digit-Frontend/tree/develop) + ### Published from DIGIT Frontend Repo (https://github.com/egovernments/Digit-Frontend/tree/develop)
Summary by CodeRabbit
InboxSearchComposerV2React component for managing inbox search features with state management, API interactions, and mobile responsiveness.ResultsTablecomponent with additional props liketype,activeLink, andbrowserSessionfor improved functionality.