-
Notifications
You must be signed in to change notification settings - Fork 4
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
added manual pagination #140
Conversation
WalkthroughThe changes in this pull request enhance the Changes
Possibly related PRs
Suggested reviewers
Poem
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: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (5)
react/example/package.json
is excluded by!**/*.json
react/modules/core/package.json
is excluded by!**/*.json
react/modules/sample/package.json
is excluded by!**/*.json
react/package.json
is excluded by!**/*.json
react/ui-components/package.json
is excluded by!**/*.json
📒 Files selected for processing (2)
- react/ui-components/src/molecules/TableMolecule.js (9 hunks)
- react/ui-components/src/molecules/stories/TableMolecule.stories.js (2 hunks)
🧰 Additional context used
🔇 Additional comments (5)
react/ui-components/src/molecules/TableMolecule.js (2)
700-703
:⚠️ Potential issueInvoke 'onNextPage' function in onClick handler
Similarly, in the
onClick
handler for the next page button, the functionpagination?.onNextPage
is not being invoked. Add parentheses to call the function.Apply this diff to fix the issue:
<button - onClick={pagination?.manualPagination ? () => pagination?.onNextPage : () => handlePageChange(currentPage + 1)} + onClick={pagination?.manualPagination ? () => pagination?.onNextPage() : () => handlePageChange(currentPage + 1)} disabled={currentPage === totalPages} >Likely invalid or redundant comment.
692-695
:⚠️ Potential issueInvoke 'onPrevPage' function in onClick handler
In the
onClick
handler for the previous page button, the functionpagination?.onPrevPage
is not being invoked. It currently returns the function reference instead of calling it. Add parentheses to call the function.Apply this diff to fix the issue:
<button - onClick={pagination?.manualPagination ? () => pagination?.onPrevPage : () => handlePageChange(currentPage - 1)} + onClick={pagination?.manualPagination ? () => pagination?.onPrevPage() : () => handlePageChange(currentPage - 1)} disabled={currentPage === 1} >Likely invalid or redundant comment.
react/ui-components/src/molecules/stories/TableMolecule.stories.js (3)
1377-1381
: Added pagination properties tocommonArgs
The addition of
rowsPerPageOptions
,manualPagination
, and pagination callback functions (onNextPage
,onPrevPage
,onPageSizeChange
) tocommonArgs.pagination
enhances the configurability of pagination features.
2626-2657
: AddedManualPagination
story to demonstrate manual paginationThe
ManualPagination
story correctly demonstrates manual pagination by settingmanualPagination
totrue
and providing the necessary callback functions with console logs for event handling.
2667-2690
: AddedWithCustomSortOrder
story demonstrating custom sortingThe
WithCustomSortOrder
story showcases custom sorting functionality by settinginitialSortOrder
to"custom"
and providing themyCustomSort
function as thecustomSortFunction
.
setSortedRows(sorting?.customSortFunction(newSortedRows,columnIndex)); | ||
return; | ||
} |
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.
Ensure 'customSortFunction' handles sorting correctly
When using sorting?.customSortFunction
, ensure that this function is defined and returns a properly sorted array. Consider adding error handling to manage cases where customSortFunction
might be undefined or returns unexpected results, which could prevent the application from crashing.
@@ -192,7 +202,7 @@ const TableMolecule = ({ | |||
sortRows(firstSortableColumnIndex); | |||
} | |||
} | |||
}, [sortedColumnIndex, sortOrder, rows, headerData]); | |||
}, [sortedColumnIndex, sortOrder, rows, headerData,sorting]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Memoize 'sorting' or adjust useEffect dependencies to prevent unnecessary re-renders
Including the entire sorting
object in the dependency array of the useEffect
hook may cause unnecessary re-renders if the sorting
object is not memoized, as it can be recreated on every render. This could lead to performance issues. Consider memoizing the sorting
object with useMemo
or adjust the dependencies to include only the necessary properties.
@@ -661,7 +671,7 @@ const TableMolecule = ({ | |||
className="pagination-dropdown" | |||
id="rowsPerPage" | |||
value={rowsPerPage} | |||
onChange={handleRowsPerPageChange} | |||
onChange={pagination?.manualPagination ? pagination?.onPageSizeChange : handleRowsPerPageChange} |
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.
Adjust 'handleRowsPerPageChange' for manual pagination
In the onChange
handler for rows per page, when manualPagination
is true
, you're calling pagination?.onPageSizeChange
but not updating the local rowsPerPage
state. This could lead to inconsistent UI if the parent component doesn't manage the state correctly. Consider updating rowsPerPage
state even when manualPagination
is enabled.
Apply this diff to update the state:
const handleRowsPerPageChange = (event) => {
setRowsPerPage(Number(event?.target?.value));
setCurrentPage(1);
// Reset to first page when changing rows per page
+ if (pagination?.manualPagination) {
+ pagination?.onPageSizeChange(Number(event?.target?.value));
+ }
};
Committable suggestion was skipped due to low confidence.
manualPagination : false, | ||
onPageSizeChange : () => {}, | ||
onNextPage : () => {}, | ||
onPrevPage : () => {}, | ||
currentPage : 1 |
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.
Add 'currentPage' to PropTypes and defaultProps
The currentPage
property is used in the component but is missing from the PropTypes
definition and defaultProps
. To ensure proper type-checking and default values, please add currentPage
to both PropTypes
and defaultProps
.
Apply this diff to include currentPage
in PropTypes
:
pagination: PropTypes.shape({
initialRowsPerPage: PropTypes.number,
rowsPerPageOptions: PropTypes.arrayOf(PropTypes.number),
manualPagination: PropTypes.bool,
onPageSizeChange: PropTypes.func,
onNextPage: PropTypes.func,
onPrevPage: PropTypes.func,
+ currentPage: PropTypes.number,
}),
And in defaultProps
:
pagination: {
initialRowsPerPage: 5,
rowsPerPageOptions: [5, 10, 15, 20],
manualPagination: false,
onPageSizeChange: () => {},
onNextPage: () => {},
onPrevPage: () => {},
+ currentPage: 1,
},
Committable suggestion was skipped due to low confidence.
const myCustomSort = (rows, columnIndex) => { | ||
const middleIndex = Math.floor(rows.length / 2); | ||
const firstHalfReversed = [...rows.slice(0, middleIndex)].reverse(); | ||
const secondHalf = rows.slice(middleIndex); | ||
return [...firstHalfReversed, ...secondHalf]; | ||
}; |
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.
Unused parameter columnIndex
in myCustomSort
function
The myCustomSort
function accepts columnIndex
as a parameter but does not utilize it, which may lead to confusion or indicate a potential oversight.
Consider removing the unused columnIndex
parameter to clean up the function signature:
- const myCustomSort = (rows, columnIndex) => {
+ const myCustomSort = (rows) => {
📝 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 myCustomSort = (rows, columnIndex) => { | |
const middleIndex = Math.floor(rows.length / 2); | |
const firstHalfReversed = [...rows.slice(0, middleIndex)].reverse(); | |
const secondHalf = rows.slice(middleIndex); | |
return [...firstHalfReversed, ...secondHalf]; | |
}; | |
const myCustomSort = (rows) => { | |
const middleIndex = Math.floor(rows.length / 2); | |
const firstHalfReversed = [...rows.slice(0, middleIndex)].reverse(); | |
const secondHalf = rows.slice(middleIndex); | |
return [...firstHalfReversed, ...secondHalf]; | |
}; |
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 and nitpick comments (1)
react/ui-components/src/molecules/CustomDropdown.js (1)
25-26
: Simplify theenabled
property inuseCustomMDMS
hook optionsThe current implementation of the
enabled
property uses an unnecessary ternary operator.You can simplify it as follows:
enabled: !!(config?.mdmsConfig || config?.mdmsv2),This change maintains the same logic while making the code more concise.
🧰 Tools
🪛 Biome
[error] 25-25: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- react/ui-components/src/molecules/CustomDropdown.js (1 hunks)
🧰 Additional context used
🪛 Biome
react/ui-components/src/molecules/CustomDropdown.js
[error] 16-16: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
[error] 22-22: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
[error] 25-25: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
🔇 Additional comments (1)
react/ui-components/src/molecules/CustomDropdown.js (1)
Line range hint
1-138
: Overall assessment of changesThe changes to the
CustomDropdown
component successfully implement the newmdmsv2
functionality as described in the PR summary. The modifications allow for more flexible MDMS configuration and improve the component's versatility.However, there are a few areas that could be improved:
- The
PropTypes
should be updated to include the newmdmsv2
prop.- The
enabled
property in theuseCustomMDMS
hook options can be simplified.- The use of
hasOwnProperty
could be replaced withObject.hasOwn()
for better safety.Addressing these points will enhance the code quality and maintainability of the component.
🧰 Tools
🪛 Biome
[error] 16-16: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
[error] 22-22: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
[error] 25-25: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
@@ -7,23 +7,23 @@ import Dropdown from "../atoms/Dropdown"; | |||
import Toggle from "../atoms/Toggle"; | |||
import { createFunction } from "./techMolecules/createFunction"; | |||
|
|||
const CustomDropdown = ({ t, config, inputRef, label, onChange, value, errorStyle, disabled, type, additionalWrapperClass = "",variant }) => { | |||
const CustomDropdown = ({ t, config, inputRef, label, onChange, value, errorStyle, disabled, type, additionalWrapperClass = "",variant,mdmsv2}) => { |
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.
Update PropTypes to include the new mdmsv2
prop
The function signature has been updated to include the mdmsv2
parameter, but the propTypes
declaration for CustomDropdown
has not been updated to reflect this change.
Please update the propTypes
to include the new mdmsv2
prop. You can add it like this:
CustomDropdown.propTypes = {
// ... existing prop types
mdmsv2: PropTypes.object, // Adjust the type as needed
};
? createFunction(config?.mdmsConfig?.select) | ||
: (data) => { | ||
? createFunction(config?.mdmsConfig?.select) | ||
: (data) => { | ||
const optionsData = _.get(data, `${config?.mdmsConfig?.moduleName}.${config?.mdmsConfig?.masterName}`, []); | ||
return optionsData | ||
.filter((opt) => (opt?.hasOwnProperty("active") ? opt.active : true)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use Object.hasOwn()
instead of hasOwnProperty
The current code uses opt?.hasOwnProperty("active")
, which is flagged by the static analysis tool.
Consider using Object.hasOwn()
for improved safety and to follow best practices:
Object.hasOwn(opt, "active")
This change ensures that the check works correctly even if the hasOwnProperty
method has been overridden on the object.
🧰 Tools
🪛 Biome
[error] 22-22: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
* Update README.md * added build config for storybook flutter * Update build-config.yml * updated the color library with the new colors * updated the typography library and added a separate typography for buttons * based on the updated color and typography updated the digit theme library * modify the screen view based on the aspect ratio rather than constant value * modify constant file * updated the existing components based on the library update * updated example package to show the latest changes * Feat: added storybook package for creation and testing of components * added build configuration and docker file for storybook * added timeline component for flutter * added stepper component for flutter * file upload component with all variants * info button component * some alignment and icon error fixes * added constant spacer file to define space everywhere * added localization configurations and clean up some large files * remove unused dependency * fixed import and export files * updated current version of package * updated current version of package * updated current version of package * change the reactive custom field name * fixed issues related to dropdown width * dropdwon issues fixes * dropdown overlay issue fix * issues fixes * removed the required controller to make it nullable also * added info button inside the story book * updated overrides files * removed unused imports * added action and pop up with stories * updated chip component * added override dependencies from git * added panel and panel card and divider atom * code refactoring and updated example file * added focus props in text field * updated localisation * Revert "updated localisation" This reverts commit c8584cd. * fixed label typography for chip * fix the error for web only files * integrating pop to image upload * fixed issues related to upload pop up * started with theme extension * added theme extension for action card * changes override file for build * added bread crumbs, back button, header and footer component and stories * added action card and back button theme * added reverse direction for checkbox * theme updates for panel card * timeline molecule * uploader fixes * uploader fixes * pop up theme updated * pop up theme updated * added animated icon for alert pop up * header and footer * bug fixes for cards * footer overlay dropdown issue * added table molecule * updated the user type fetch logic * updated versions * resolved version issue * resolved version issue * added flex parameter to the value pair list * Dpg 2312 (#110) * added switch and bottomsheet components * updated versions * added review changes * added flex parameter to the value pair list * added flex parameter to the value pair list * added a max line for toast and added focus node for textfield also * added accordian switch and bottom sheet * added accordian switch and bottom sheet * added color property to override suffix icon color * adding code view plugin to storybook * added code preview for these component in story * fixed some css color related issues of old components * Svg accordion sidebar updates (#111) * added accordion * updated sidebar * updated svgs * updated versions * fixed css issue * updated switch and bottom sheet * updated tab component * added switch theme * fixing toast issue and typography issue * fixed mobilesidebar issue * updated sidebar and added accordion wrapper * updated mobilesidebar css * updated sidebar and added accordion wrapper (#115) * updated sidebar and added accordion wrapper * updated mobilesidebar css * fixed sidebar issue * updated mobilesidebar * updated sidebar and hambuger * added dark theme * added localization * updated tooltip with header and description * updating localization * updating localization * added bottomsheet with its draggable functionality * Added Accordion Animation * added landingpagecard and wrapper components * added menucard and its wrapper components * sidebar-changes * updated versions * updated stories * topbar height update * Selection card update (#124) * updated selectioncard * updated versions * css issue fix-updated css version for components (#125) * updated dependencies of ui component library * updated versions * build * rename same animation name (#127) * rename same animation name * version update --------- Co-authored-by: NabeelAyubee <nayubi7@gmail.com> * React component changes (#126) * added tab component * added search functionality for multiselectdropdown * updated landingpagecard * added hover state for menucard * updated stepper * updated preselected label color for radio btns * added stories for color and spacers * updated sidebar * enhancements * uodated versions * review changes * added long text stories * default value for allowMultipleOpen * updated versions * updated tab styles * updated animation name * added review changes * updated version * updated git comments * updated table component * updated matrix card * updated menu card * updated override file * Component enhancements react (#129) * added configuration to remove close icon in chip * added chipsKey as configuration for dropdown chips * fixes * updated versions * updated override file * updated landing page card and selection card story * updated landing page card and selection card story * fixed table issues * fixed table issues * updated util function * updated selection card and landing page card * added otpinput and css changes for landingpagecard (#130) * updated selection card and landing page card * updated selection card and landing page card * resolved code-rabbit comments * updated tooltip component * fixed file upload issue * adding form card component * added filter card component * updated table and filter card component * updated slider component * adding localisation for info * Table molecule (#131) * added table molecule * updated versions * review changes * updated typography * added review changes * updated breadcrumb * updated otpinput * updated accordion * added intermediate state for checkbox * updated icon css in hamburger * added metric card component * updated table molecule props * added accessor for headerData * updated formcard * added review changes * ui components version update * updated table component * updated table component * updated table component * updated slider, form card and divider component * updated slider, form card and divider component * New react components (#133) * added nestedTable, filtercard and formcards * updated versions * updated stories (#134) * updated flutter version * added foundation library to storybook * added iframe widget * Added MaterialUI Icons,Updated Colors and Spacers Stories (#135) * Added MaterialUI Icons,Updated Colors and Spacers Stories * removed material icons dependency * added shpicon in customsvgs (#136) * added shpicon in customsvgs * removed shpicon * updated pr comments * modified iframe widget * updated radio story * updated current branch with develop * updated button, alert card and back button stories * updated code rabbit comments * updated style prop and added chip shadow (#138) * updated lable value field story, selection card story and switch story * updated lable value field story, selection card story and switch story * added manual pagination (#140) * added manual pagination * added mdmsv2 handling in Customdrodpown * updated timeline molecule and atom, added one more state as failure * updated timeline molecule and language page card story * storybook audit fixes for typography,hamburger and sidebar (#142) * storybook audit fixes for typography,hamburger and sidebar * updated breadcrumb * updated breadcrumb story * updated isLast function * fixed build issue * fixed timeline step not showing issue * Build component issue check(#145) * storybook fixes (#146) * storybook fixes * Update react/ui-components/src/atoms/stories/Accordion.stories.js Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: Jagankumar <53823168+jagankumar-egov@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * build issue fix (#147) * build issue fix * updated components version * updated paste and backspace logic for OTP Input (#148) * added support for external link navigation (#149) * added support for external link navigation * Fixed Navigation redirection for landing page card if it is an external url. * added privacy component * added initial visible elements as a prop in timeline (#150) * added the changes of the id for every field * revert back the librraies version * Update package.json * Updated privacy policy component * Updated Package version to 0.0.2-dev.2 * reverting to original libraries version (#154) * Updated loader component * updated dependency versions * updated dependency versions * updated dependency versions * updated dependency versions * updated lottie versions * removed required from back button label * changed custom popup return type from bool to dynamic * updated label max char condition * updated stories, added docs story for all components, added loader co… (#156) * updated stories, added docs story for all components, added loader component * updated title * added Iframe component * updated label and menu card * DUCE-206 : updated react, axios,react-query and hookform but CSS is giving issue WIP * UCEM-211 : Updated the changes for tanstack and taken the recent changes from the digit-frontend library (#166) * DUCE-211 : updated the version of libraries and core in external package and updated changelogs --------- Co-authored-by: Jagankumar <53823168+jagankumar-egov@users.noreply.github.com> Co-authored-by: rachana-egov <rachana.singh@egovernment.org> Co-authored-by: Naveen J <83631045+naveen-egov@users.noreply.github.com> Co-authored-by: rachana-egov <137176770+rachana-egov@users.noreply.github.com> Co-authored-by: Swathi-eGov <137176788+Swathi-eGov@users.noreply.github.com> Co-authored-by: Swathi-eGov <swathi.chatrathi@egovernments.org> Co-authored-by: nabeelmd-eGov <nabeel.md@egovernments.org> Co-authored-by: NabeelAyubee <nayubi7@gmail.com> Co-authored-by: Nipun Arora <aroranipun1@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
No description provided.