[Platform] Data download enhancement #2550
Conversation
📝 WalkthroughWalkthroughThis pull request introduces layout and performance enhancements across several components. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DataDownload
participant DataTable
User->>DataDownload: Select filter option ("Sites" or "Favorites")
DataDownload->>DataDownload: Execute handleFilter()
DataDownload->>DataTable: Pass updated filtered data and column definitions
DataTable->>User: Render updated table view
Suggested labels
Suggested reviewers
Poem
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
src/platform/src/common/components/Modal/dataDownload/components/DataTable.jsx (4)
28-42: Discuss default prop usage.
The function signature now has default values for many props, which is helpful. However, if these defaults become more complex, you might extract them outside the component or define them as static class members to avoid repeated creation per render cycle.
63-69: Consider partial resets.
Triggering theuseEffectonclearSelectionTriggerresets all selected rows. If there is ever a need for partial unselection, consider adjusting this logic or introducing a more granular reset.
119-140: Efficient row selection logic.
Your default toggle method is concise but does a linear search inselectedRows. If performance becomes a bottleneck for large datasets, consider a hash-based or set-based approach for O(1) lookups.
193-199: Error fallback design.
This block provides a simple error message. Optionally consider including a retry button or more context to guide users to resolve errors.src/platform/src/common/components/Modal/dataDownload/modules/DataDownload.jsx (4)
73-76: Local selection state in parent component.
Storing selection inselectedSitesaligns well with the newDataTableprops. For large or shared data among multiple components, consider a global store solution (e.g., Redux) if the selection state must be synchronized.
80-80: Introduce meaningful naming for “edit” state.
If the use case becomes more complex, renamingedittoisTitleEditModeor similar might provide immediate clarity for future maintainers.
121-131: Fetch site summaries on organization changes.
This effect ensures data fetching is triggered at the right time. Consider adding error handling or retries iffetchSitesSummaryfails (although you do handle fetchError for display).
133-155: Reset form data and clear selection.
When clearing the selection, you’re also resetting form data to defaults. This is consistent. If you only want to reset the selection, consider decoupling that logic from the form reset for finer control.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/platform/src/common/components/Calendar/Calendar.jsx(1 hunks)src/platform/src/common/components/Calendar/DatePicker.jsx(5 hunks)src/platform/src/common/components/Modal/dataDownload/components/CustomFields.jsx(5 hunks)src/platform/src/common/components/Modal/dataDownload/components/DataTable.jsx(2 hunks)src/platform/src/common/components/Modal/dataDownload/modules/AddLocations.jsx(5 hunks)src/platform/src/common/components/Modal/dataDownload/modules/DataDownload.jsx(16 hunks)
🔇 Additional comments (44)
src/platform/src/common/components/Calendar/Calendar.jsx (1)
199-199: Improved padding consistency in calendar layout.The padding adjustments enhance the visual balance of the calendar component, ensuring more consistent spacing when toggling between single and dual calendar views.
src/platform/src/common/components/Modal/dataDownload/components/CustomFields.jsx (7)
8-8: More accurate function description.This comment now better reflects what the function actually does - it formats strings in general, not just names.
18-18: Simplified organization field formatting logic.The redundant string replacement has been removed since
formatNamealready handles the replacement of underscores and hyphens.
51-53: Improved state initialization with intermediate variable.Using
initialOptionimproves code readability by making the initialization logic more explicit and easier to follow.
68-69: Simplified class structure.The class structure has been streamlined, maintaining the same layout while reducing complexity.
72-74: Enhanced input attributes.Added proper HTML attributes for the input field to improve accessibility and semantics.
84-85: Concise DatePicker onChange handler.The handler has been simplified to directly pass the date object to the parent handler.
113-113: Cleaner prop syntax for CheckIcon.The fill property now uses a more concise syntax, which is consistent with modern React patterns.
src/platform/src/common/components/Modal/dataDownload/modules/AddLocations.jsx (8)
44-44: More precise comment about data source.The comment now clearly indicates that the data is coming from the Redux store rather than a custom hook.
51-52: Clearer filtering comment.The updated comment better describes the purpose of the filteredSites computation.
80-91: Improved initialization condition logic.The condition for initializing selected sites has been refined to be more precise, only setting values when all necessary data is available. The dependency array has also been properly updated to include all relevant variables.
105-116: Simplified site toggle handling.The site toggling logic has been significantly improved:
- Using functional state updates for both selectedSites and sidebarSites
- More straightforward conditional logic
- Better handling of state dependencies
This approach is more maintainable and less prone to bugs.
118-133: Added custom filtering capability.A new filter handler has been implemented to support filtering by favorites, which enhances the user experience by providing more ways to view and manage site data.
138-191: Structured filter and column definitions.The code now properly defines and organizes filters and column configurations using useMemo to optimize performance. The column definitions for different filter types (sites vs favorites) improve the component's flexibility.
240-240: Updated dependency array with missing dependency.Added
activeGroupIdto the dependency array of handleSubmit to ensure the function is properly recreated when the active group changes.
294-308: Consistent prop naming convention in DataTable.The props have been renamed from site-specific terms to more generic row-based terms, making the component more reusable across different contexts. Additional filter-related props have been added to support the new filtering capabilities.
src/platform/src/common/components/Calendar/DatePicker.jsx (8)
1-1: Added useCallback for performance optimization.Importing useCallback will help memoize callback functions and prevent unnecessary rerenders.
13-17: Added default prop values for better usability.Default values for customPopperStyle and alignment make the component more robust and easier to use without requiring all props to be explicitly provided.
21-21: Enhanced popper with arrow element.Added state management for the arrow element and properly configured it in the popper modifiers. This improves the UI by providing a visual indicator for the dropdown direction.
Also applies to: 49-54
58-63: Optimized toggle function with useCallback.Renamed handleToggle to toggleOpen and implemented it with useCallback to prevent unnecessary recreation of the function on each render.
65-74: Optimized value change handler with useCallback.The value change handler has been refactored to use useCallback and now includes optional chaining for the onChange callback, ensuring it only gets called if defined.
76-91: Improved click handler with defensive programming.The click outside handler now:
- Uses useCallback for performance
- Checks for the existence of referenceElement before accessing it
- Applies proper dependency tracking
These changes make the component more robust against unexpected scenarios.
103-103: Updated useEffect dependency to prevent stale closures.The dependency array now includes handleClickOutside instead of referenceElement directly, ensuring that the effect is properly synchronized with its dependencies.
154-166: Added styled arrow element to popper.A properly styled and positioned arrow element has been added to the popper, providing visual context to users about where the popup originated from.
src/platform/src/common/components/Modal/dataDownload/components/DataTable.jsx (10)
6-6: Encapsulate the search component import.
This import ofTopBarSearchlooks fine. For performance-sensitive cases, consider lazy-loading if it's not always needed, but otherwise this is good.
9-27: Keep documentation in sync with future enhancements.
The updated prop documentation accurately describes the new props (filters, onFilter, selectedRows, etc.). Ensure to keep these doc comments aligned with any subsequent modifications, so that future maintainers can rely on them.
72-83: Clarify filter vs. search precedence.
Currently, search results override the filtered data whensearchResultsare present. If the intent is for both filter and search to be applied in tandem, you might want to filter thesearchResultsagain or combine the logic differently.Could you confirm whether this override is the desired behavior? If not, we can propose a combined filter-and-search approach.
100-117: Pagination calculation and slicing.
UsingfilteredData.lengthfor pagination is straightforward. The slice approach is efficient for moderate data sizes. If the dataset grows large, you could consider virtual scrolling or server-side pagination.
146-155: Page-based "select all" semantics.
Selecting all rows on the current page is a valid UX choice. If users expect to select rows across multiple pages, consider clarifying this behavior in the UI or offering a “select all across pages” option.
158-176: Indeterminate checkbox state.
These conditions properly handleselectAllvs.indeterminatefor the current page. Everything looks solid, and the logic is easy to follow for multi-row selections.
201-207: Loading skeleton.
Great for enhancing user feedback while data loads. No issues here.
209-246: Filter and search UI updates.
Neatly displays filter buttons and integrates theTopBarSearch. Code is organized, and usage ofactiveFilteris consistent with the logic in the table. This approach is good for modular filtering.
249-347: Table rendering structure.
The dynamic checkbox, row rendering, and column iteration are cleanly implemented. The try/catch for custom render functions helps guard against errors. Overall, this is well-designed.
351-376: Pagination controls.
Straightforward navigation logic with disabled states for edge pages. This is a user-friendly approach.src/platform/src/common/components/Modal/dataDownload/modules/DataDownload.jsx (10)
9-9: Enhance location icon usage.
The addition ofLocationIconis a nice visual improvement, helping users recognize site data more intuitively.
13-14: Import reorganization.
Rearranging theFooterimport near the top clarifies dependencies, which helps keep related imports together.
84-88: Active group handling.
Your logic for retrieving the active group and defaulting togroupTitleis straightforward. Confirm that the fallback references (ORGANIZATION_OPTIONS[0]) are valid ifactiveGroupis undefined or null.
97-106: Initialize form data.
The default form data is descriptive and includes a “title” object. Everything appears consistent with your new selection and filtering logic.
167-174: Toggle site selection logic.
This merges well with the changes made inDataTable. No issues found, but for large sets, watch out for performance if your site array gets big.
186-290: Robust form validation and download handling.
Your validation covers:
• Checking the date range for start/end.
• Ensuring at least one location is selected.
• Handling frequency constraints for hourly/daily durations.
• Generating a filename and MIME type.
• Handling multiple file types (CSV, JSON, PDF).The logic is cohesive, though consider making the duration validation logic reusable if multiple components need it.
311-378: Filter logic for 'sites' vs. 'favorites'.
Defining two simple filters makes sense. ThehandleFilterfunction references user preferences to identify favorites. This is a neat approach. Keep an eye on possible data changes in preferences.
387-387: Editable report title UI.
This button toggles an “edit” state, providing a good user experience for adjusting the report title. The approach is simple and effective.
460-475: Updated DataTable props.
PassingselectedRows,setSelectedRows,clearSelectionTrigger, andonToggleRowharmonizes with the DataTable’s new selection logic. Clean and consistent with your design approach.
482-491: Footer integration.
The footer now properly referencesselectedSitesand the form error messages. This is a logical step toward modularizing your UI. No concerns here.
|
New next-platform changes available for preview here |
Summary of Changes (What does this PR do?)
Status of maturity (all need to be checked before merging):
Screenshots (optional)
Summary by CodeRabbit
Style
New Features
Bug Fixes
Refactor