-
Notifications
You must be signed in to change notification settings - Fork 3
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
Use Session Storage to hold temp state for DataGrid #1618
base: develop
Are you sure you want to change the base?
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces several enhancements to the grid components across the application, primarily focusing on state management through session storage. 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 (
|
src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsExpandedGrid.tsx
Outdated
Show resolved
Hide resolved
src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsExpandedGrid.tsx
Outdated
Show resolved
Hide resolved
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: 9
🧹 Outside diff range and nitpick comments (7)
src/hooks/useSessionStorage.tsx (2)
21-27
: Optimize storage updates and prevent race conditions.The current implementation has duplicated storage logic and potential race conditions.
Consider extracting the storage logic and implementing debouncing:
+import { useMemo } from 'react'; +import debounce from 'lodash/debounce'; + +const updateStorage = (key: string, value: unknown) => { + if (typeof window !== 'undefined') { + try { + sessionStorage.setItem(key, JSON.stringify(value)); + } catch (error) { + console.error('Failed to save to session storage:', error); + } + } +}; const setValue = (value: T | ((val: T) => T)) => { const valueToStore = value instanceof Function ? value(storedValue) : value; setStoredValue(valueToStore); - if (typeof window !== 'undefined') { - sessionStorage.setItem(storageKey, JSON.stringify(valueToStore)); - } }; +const debouncedUpdate = useMemo( + () => debounce((value: T) => updateStorage(storageKey, value), 300), + [storageKey] +); + +useEffect(() => { + debouncedUpdate(storedValue); + return () => { + debouncedUpdate.cancel(); + }; +}, [debouncedUpdate, storedValue]);
29-30
: Consider adding reset functionality and explicit return type.The hook could benefit from additional utility functions and explicit typing.
Consider this enhancement:
+type StorageHookResult<T> = readonly [ + T, + (value: T | ((val: T) => T)) => void, + () => void +]; + -return [storedValue, setValue] as const; +const reset = () => { + setStoredValue(defaultValue); + if (typeof window !== 'undefined') { + sessionStorage.removeItem(storageKey); + } +}; + +return [storedValue, setValue, reset] as StorageHookResult<T>;src/scenes/Projects/List/ProjectsPanel.tsx (2)
26-31
: Consider making the storage key more specificThe current storage key 'projects-grid-state' might be too generic if this component is used in different contexts (e.g., different routes or views). Consider making it more specific or dynamic.
- 'projects-grid-state', + `projects-grid-state-${context}`, // where context could be route/view identifier
77-77
: Consider memoizing the sx prop arrayThe sx prop array is recreated on every render. Consider memoizing it with useMemo to prevent unnecessary re-renders.
+ const gridSx = useMemo( + () => [flexLayout, noHeaderFilterButtons, noFooter], + [] + ); return ( <DataGrid<Project> {...DefaultDataGridStyles} {...dataGridProps} slots={slots} slotProps={slotProps} columns={ProjectColumns} initialState={savedGridState} headerFilters hideFooter onStateChange={onStateChange.run} - sx={[flexLayout, noHeaderFilterButtons, noFooter]} + sx={gridSx} /> );Also applies to: 80-81
src/scenes/Partners/Detail/Tabs/Projects/PartnerDetailProjects.tsx (1)
63-72
: Consider performance optimizations for the state change handler.While the current implementation is functional, consider these improvements:
- The
maxWait
being equal towait
might not provide enough protection against rapid updates.- The function could be memoized to prevent unnecessary recreations.
Consider this optimization:
- const onStateChange = useDebounceFn( + const onStateChange = useDebounceFn( + useCallback( () => { const gridState = props.apiRef.current.exportState(); if (!isEqual(gridState, prevState.current)) { prevState.current = gridState; setSavedGridState(gridState); } - }, - { wait: 500, maxWait: 500 } + }, + [props.apiRef, setSavedGridState] + ), + { wait: 500, maxWait: 1000 } );src/components/Grid/useDataGridSource.tsx (1)
393-396
: LGTM! Consider queueing scroll operations.The mount check is a good defensive programming practice. However, consider queueing the scroll operation for when the grid becomes available, especially if maintaining scroll position is critical for UX.
Example enhancement:
-if (apiRef.current.rootElementRef.current) { - apiRef.current.scrollToIndexes({ rowIndex: 0 }); -} +const scrollToTop = () => { + if (apiRef.current.rootElementRef.current) { + apiRef.current.scrollToIndexes({ rowIndex: 0 }); + } else { + // Queue scroll operation for next render + requestAnimationFrame(scrollToTop); + } +}; +scrollToTop();src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsExpandedGrid.tsx (1)
141-150
: Optimize state comparison inonStateChange
The use of
isEqual
to comparegridState
withprevState.current
can be computationally expensive for large state objects. Consider optimizing the comparison by checking only the parts of the state that are prone to change or implementing a more efficient comparison mechanism to improve performance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
src/components/Grid/useDataGridSource.tsx
(2 hunks)src/hooks/useSessionStorage.tsx
(1 hunks)src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsExpandedGrid.tsx
(5 hunks)src/scenes/Partners/Detail/Tabs/Engagements/PartnerDetailEngagements.tsx
(4 hunks)src/scenes/Partners/Detail/Tabs/Projects/PartnerDetailProjects.tsx
(4 hunks)src/scenes/Projects/List/EngagementsPanel.tsx
(3 hunks)src/scenes/Projects/List/ProjectsPanel.tsx
(3 hunks)
🔇 Additional comments (16)
src/scenes/Projects/List/ProjectsPanel.tsx (1)
5-8
: LGTM: Clean import additions
The new imports are well-organized and properly scoped for the grid state management implementation.
Also applies to: 22-22
src/scenes/Partners/Detail/Tabs/Projects/PartnerDetailProjects.tsx (4)
6-8
: LGTM: Appropriate imports added for grid state management.
The new imports provide the necessary types and utilities for implementing grid state persistence.
Also applies to: 28-28
41-41
: LGTM: Appropriate use of useRef for previous state tracking.
The implementation correctly uses useRef to track the previous state without causing re-renders.
89-89
: LGTM: Proper integration with DataGrid component.
The onStateChange handler is correctly connected to the DataGrid component.
36-40
: LGTM: Well-implemented session storage with proper isolation.
The implementation correctly isolates grid state per partner and maintains type safety.
Let's verify the ProjectInitialState import and structure:
✅ Verification successful
LGTM: Session storage implementation is correctly using a well-defined initial state
The implementation is properly using ProjectInitialState
which is a type-safe grid configuration that:
- Pins the first column (name) to the left
- Sets default column visibility with some columns hidden by default (isMember, pinned)
- Satisfies the DataGridProps['initialState'] type
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the structure of ProjectInitialState
ast-grep --pattern 'const ProjectInitialState = {
$$$
}'
Length of output: 60
Script:
#!/bin/bash
# Let's try a broader search for ProjectInitialState
rg "ProjectInitialState" -A 5
# Also search for any grid state initialization patterns
ast-grep --pattern 'const $_ = {
sortModel: $_,
filterModel: $_,
$$$
}'
Length of output: 3294
Script:
#!/bin/bash
# Let's check the actual structure of ProjectInitialState
cat src/components/ProjectDataGrid/ProjectColumns.tsx
Length of output: 3975
src/components/Grid/useDataGridSource.tsx (1)
Line range hint 393-418
: Consider comprehensive error handling for grid operations.
While the mount checks improve reliability, consider implementing a more comprehensive error handling strategy for grid operations. This could include:
- Logging failed operations for debugging
- Retrying failed operations where appropriate
- Providing user feedback for failed interactions
- Handling edge cases like rapid sort/filter changes
This would make the grid more resilient to various failure modes and provide a better debugging experience.
Let's verify the current error handling coverage:
src/scenes/Projects/List/EngagementsPanel.tsx (6)
5-7
: Imports are correctly integrated
The necessary modules (GridInitialStatePro
, useDebounceFn
, isEqual
, merge
) are imported and utilized appropriately in the code.
22-22
: Session storage hook is imported properly
The useSessionStorage
hook is imported from '~/hooks/useSessionStorage'
and is ready for use in managing the grid state persistence.
26-31
: Grid state persistence is effectively implemented
The savedGridState
and setSavedGridState
state variables are correctly initialized using useSessionStorage
, and prevState
is set up to track the previous grid state using useRef
.
55-65
: Debounced state change handler is properly implemented
The onStateChange
function uses useDebounceFn
to debounce the grid state changes, minimizing unnecessary updates to session storage. This enhances performance by reducing the frequency of state saves.
66-69
: Grid state restoration is handled correctly
The useEffect
hook restores the grid state from savedGridState
when it changes, ensuring the grid reflects the persisted state upon re-rendering. The dependency array is accurately specified.
77-80
: DataGrid component updated with persistent state management
The DataGrid
component's initialState
prop is updated to use savedGridState
, and the onStateChange
prop is linked to the debounced onStateChange.run
function, enabling state persistence across sessions.
src/scenes/Partners/Detail/Tabs/Engagements/PartnerDetailEngagements.tsx (1)
6-6
: Ensure ahooks
is included in project dependencies.
Please verify that ahooks
is added to your package.json
dependencies to ensure it is properly installed and available.
Run the following script to confirm that ahooks
is listed in package.json
dependencies:
✅ Verification successful
ahooks
dependency is correctly included in package.json
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if 'ahooks' is listed in package.json dependencies.
grep -q '"ahooks"' package.json && echo "'ahooks' is present in dependencies." || echo "'ahooks' is missing from dependencies."
Length of output: 122
src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsExpandedGrid.tsx (3)
10-10
: Confirm the necessity of new external dependencies
The imports of useDebounceFn
from ahooks
and isEqual
from lodash
introduce new external dependencies to the project. Please verify whether adding these libraries is essential for the implementation. If possible, consider using native React hooks or utilities to minimize dependency bloat and potential maintenance overhead.
Also applies to: 12-14, 24-24
152-155
: Verify the impact of restoring state in useEffect
Restoring the grid state using apiRef.current.restoreState(savedGridState)
inside a useEffect
hook may lead to unintended side effects, such as unnecessary re-renders or conflicts with other grid operations. Please ensure that this implementation does not adversely affect the component's behavior.
If you'd like assistance in reviewing this implementation or exploring alternative solutions, I'm happy to help.
123-128
:
Ensure proper serialization in useSessionStorage
When storing complex objects like GridInitialStatePro
in session storage, it's important to ensure that serialization and deserialization are handled correctly. Please confirm that the useSessionStorage
hook manages this process without data loss or corruption.
src/scenes/Partners/Detail/Tabs/Projects/PartnerDetailProjects.tsx
Outdated
Show resolved
Hide resolved
src/scenes/Partners/Detail/Tabs/Engagements/PartnerDetailEngagements.tsx
Outdated
Show resolved
Hide resolved
src/scenes/Partners/Detail/Tabs/Engagements/PartnerDetailEngagements.tsx
Outdated
Show resolved
Hide resolved
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 (4)
src/scenes/Partners/Detail/Tabs/Engagements/PartnerDetailEngagements.tsx (1)
34-37
: Consider memoizing sessionStorageProps.The sessionStorageProps object is recreated on every render. While this might not cause performance issues due to the stable partnerId, it's a good practice to memoize objects passed to hooks.
Consider this improvement:
+ const sessionStorageProps = useMemo( + () => ({ + key: `partners-engagements-grid-state-${partnerId}`, + defaultValue: EngagementInitialState, + }), + [partnerId] + ); const [props] = useDataGridSource({ query: PartnerDetailEngagementsDocument, variables: { id: partnerId }, listAt: 'partner.engagements', initialInput: { sort: EngagementColumns[0]!.field, }, - sessionStorageProps: { - key: `partners-engagements-grid-state-${partnerId}`, - defaultValue: EngagementInitialState, - }, + sessionStorageProps, });src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsGrid.tsx (1)
180-180
: LGTM! Consider adding type annotation for initialState.The addition of the
initialState
parameter with a default empty object is a good approach for optional state initialization.Consider adding explicit type annotation for better type safety:
- initialState = {}, + initialState: Partial<DataGridProps['initialState']> = {},src/components/Grid/useDataGridSource.tsx (2)
428-432
: Add JSDoc documentation for session storage integration.The session storage integration looks good, but would benefit from documentation explaining:
- What grid state properties are being persisted
- How the persistence behavior interacts with the existing local storage functionality
- Any cleanup/expiration strategy for the session storage
Example documentation:
+/** + * Persists grid state (e.g., column widths, visibility) in session storage. + * Note: Sorting and filtering are handled separately via local storage. + * @param key - Unique identifier for the grid state in session storage + * @param defaultValue - Default grid state if nothing exists in session storage + */ const [savedGridState = {}, onStateChange] = usePersistedGridState({ key: sessionStateProps.key, apiRef: apiRef, defaultValue: sessionStateProps.defaultValue, });Also applies to: 449-449, 454-454
428-432
: Document the dual storage strategy.The component now uses both local storage (for view state like sorting/filtering) and session storage (for grid state). This dual storage approach needs clarification:
- What is the rationale for using different storage mechanisms?
- Are there any potential synchronization issues between the two storage types?
- How should other developers decide which storage to use for new state properties?
Consider adding architectural documentation to explain these decisions.
Also applies to: 449-449, 454-454
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
src/components/Grid/useDataGridSource.tsx
(6 hunks)src/hooks/usePersistedGridState.tsx
(1 hunks)src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsExpandedGrid.tsx
(0 hunks)src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsGrid.tsx
(2 hunks)src/scenes/Partners/Detail/Tabs/Engagements/PartnerDetailEngagements.tsx
(1 hunks)src/scenes/Partners/Detail/Tabs/Projects/PartnerDetailProjects.tsx
(1 hunks)src/scenes/Projects/List/EngagementsPanel.tsx
(1 hunks)src/scenes/Projects/List/ProjectsPanel.tsx
(1 hunks)
💤 Files with no reviewable changes (1)
- src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsExpandedGrid.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/scenes/Partners/Detail/Tabs/Projects/PartnerDetailProjects.tsx
- src/scenes/Projects/List/EngagementsPanel.tsx
- src/scenes/Projects/List/ProjectsPanel.tsx
🔇 Additional comments (4)
src/scenes/Partners/Detail/Tabs/Engagements/PartnerDetailEngagements.tsx (1)
34-37
: Validate partnerId before using in storage key.
The storage key uses partnerId
which could be an empty string from useParams()
. This might lead to conflicts if multiple partner detail pages are opened without an ID.
Let's verify the partner ID handling in similar components:
src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsGrid.tsx (1)
218-221
: Consider storage key management and data size implications.
The session storage implementation looks good, but consider these suggestions:
- Move the storage key to a constants file to maintain consistency and prevent typos
- Consider implementing size limits or cleanup for the stored state to prevent performance issues with large datasets
Let's verify the storage key usage and potential data size:
src/components/Grid/useDataGridSource.tsx (2)
21-21
: LGTM: Type definitions and imports are well-structured.
The new imports and type definitions for session storage integration are properly organized and strongly typed.
Also applies to: 43-43, 104-115
400-403
: 🛠️ Refactor suggestion
Extract duplicate scroll behavior.
The scroll-to-top logic is duplicated between onSortModelChange
and onFilterModelChange
. While the mount check fixes the immediate issue, we should still extract this into a reusable function.
+const scrollGridToTopIfMounted = (apiRef: MutableRefObject<GridApiPro>) => {
+ if (apiRef.current.rootElementRef.current) {
+ apiRef.current.scrollToIndexes({ rowIndex: 0 });
+ }
+};
const onSortModelChange: DataGridProps['onSortModelChange'] & {} =
useMemoizedFn((next) => {
// ... existing sort logic ...
- if (apiRef.current.rootElementRef.current) {
- apiRef.current.scrollToIndexes({ rowIndex: 0 });
- }
+ scrollGridToTopIfMounted(apiRef);
});
const onFilterModelChange: DataGridProps['onFilterModelChange'] & {} =
useMemoizedFn((filterModel) => {
// ... existing filter logic ...
- if (apiRef.current.rootElementRef.current) {
- apiRef.current.scrollToIndexes({ rowIndex: 0 });
- }
+ scrollGridToTopIfMounted(apiRef);
});
Also applies to: 422-425
const prevGridState = usePrevious( | ||
savedGridState, | ||
(prev, next) => !isEqual(prev, next) | ||
); |
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.
Incorrect usage of the compare function in usePrevious
The compare function in usePrevious
should return true
to keep the previous value unchanged when the values are equal. Currently, it returns true
when the values are not equal, which prevents prevGridState
from updating correctly. Adjust the compare function to isEqual(prev, next)
to ensure that prevGridState
updates only when the values differ.
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: 1
🧹 Outside diff range and nitpick comments (5)
src/hooks/usePersistedGridState.tsx (3)
25-26
: Consider using a more robust key generation methodThe string concatenation for the filter model key could lead to conflicts. Consider using a more structured approach.
-`${key}-api-filter` +`grid-${key}-filter-model`
33-50
: Consider making debounce configuration configurableThe debounce wait times are hardcoded. Consider making them configurable through options.
+interface UsePersistedGridStateOptions { + key: string; + apiRef: MutableRefObject<GridApiPro>; + defaultValue: GridInitialStatePro; + debounceWait?: number; +} const { run: handleStateChange } = useDebounceFn( () => { // ... }, - { wait: 500, maxWait: 500 } + { wait: debounceWait ?? 500, maxWait: debounceWait ?? 500 } );Improve readability of filter model update logic
The nested ternary operation makes the code harder to read. Consider restructuring for clarity.
-setPersistedFilterModel((prev) => - isEqual(prev, gridState) - ? prev - : convertMuiFiltersToApi( - apiRef.current, - gridState.filter?.filterModel - ) -); +setPersistedFilterModel((prev) => { + const newFilterModel = convertMuiFiltersToApi( + apiRef.current, + gridState.filter?.filterModel + ); + return isEqual(prev, gridState) ? prev : newFilterModel; +});
64-65
: Consider adding explicit return typeWhile the
as const
assertion works, an explicit return type would make the API more clear.+type PersistedGridStateReturn = readonly [ + GridInitialStatePro, + GridEventListener<'stateChange'>, + Record<string, any> +]; -return [savedGridState, onStateChange, persistedFilterModel] as const; +return [savedGridState, onStateChange, persistedFilterModel] as PersistedGridStateReturn;src/components/Grid/useDataGridSource.tsx (2)
210-215
: Add error handling for session storage failuresThe session storage integration looks good, but consider adding error handling for cases where session storage might not be available (e.g., private browsing mode, storage quota exceeded).
Example error handling:
const [savedGridState = {}, onStateChange, persistedFilterModel] = usePersistedGridState({ key: sessionStateProps.key, apiRef: apiRef, defaultValue: sessionStateProps.defaultValue, + onError: (error) => { + console.warn('Failed to persist grid state:', error); + // Fallback to default value + return sessionStateProps.defaultValue; + } });
257-261
: Consider adding type safety to filter model mergingWhile the merge operations work correctly, consider adding type safety to prevent potential runtime issues with undefined or null values.
Example implementation:
filterModel: merge( {}, - rest.filterModel, - savedGridState.filter?.filterModel + rest.filterModel ?? {}, + savedGridState.filter?.filterModel ?? {} ),const initialFilterModel = { - ...storedView?.apiFilterModel, - ...persistedFilterModel, + ...(storedView?.apiFilterModel ?? {}), + ...(persistedFilterModel ?? {}), };Also applies to: 279-282
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/components/Grid/useDataGridSource.tsx
(9 hunks)src/hooks/usePersistedGridState.tsx
(1 hunks)
🧰 Additional context used
🪛 Biome
src/components/Grid/useDataGridSource.tsx
[error] 292-292: Avoid redundant Boolean
call
It is not necessary to use Boolean
call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean
call
(lint/complexity/noExtraBooleanCast)
🔇 Additional comments (6)
src/hooks/usePersistedGridState.tsx (3)
8-12
: Improve type safety for apiRef
The previous review comment about avoiding the any
type for apiRef
is still valid. Consider using a more specific type from @mui/x-data-grid-pro
.
28-31
: Fix the inverted comparison logic
The previous review comment about incorrect comparison logic in usePrevious
is still valid. The comparison function should return true
when values are equal.
52-62
: LGTM! Clean and efficient state restoration logic
The effect correctly handles state restoration with proper dependency tracking and efficient update checks.
src/components/Grid/useDataGridSource.tsx (3)
21-21
: LGTM: Clean imports and type definitions
The new imports and SessionStorageProps interface are well-structured and properly typed.
Also applies to: 43-43, 57-60
458-458
: LGTM: Clean DataGrid props integration
The initialState and onStateChange props are properly integrated with the session storage functionality.
Also applies to: 463-463
Line range hint 401-402
: Verify scroll behavior during sort and filter operations
Based on past review comments, there were concerns about scroll behavior. The current implementation still scrolls to top on sort and filter changes. Let's verify this behavior works correctly now.
Also applies to: 436-437
✅ Verification successful
Let me gather more context about the scroll behavior implementation.
Let me check if there are any configuration options or props being passed to control this scroll behavior.
Scroll behavior during sort and filter operations is working as designed
The scroll behavior is intentionally implemented to reset to the top (rowIndex: 0) when sorting or filtering occurs. This is a common UX pattern in data grids as it helps users see the newly sorted/filtered results from the beginning. The implementation is consistent across both sort and filter operations.
Key findings:
- Scroll reset is implemented in both
onSortModelChange
andonFilterModelChange
handlers - The behavior is consistent with MUI DataGrid's default patterns
- No configuration props are being passed to override this behavior
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other scroll implementations to ensure consistency
rg -A 3 'scrollToIndexes|scrollTo'
Length of output: 933
Script:
#!/bin/bash
# Search for scroll-related code and context
rg -B 3 -A 5 'scrollToIndexes|scrollTo' src/components/Grid/useDataGridSource.tsx
# Look for any scroll-related configuration or props
ast-grep --pattern 'interface $_ {
$$$
scroll$_?: $_
$$$
}'
Length of output: 756
Script:
#!/bin/bash
# Look for DataGrid component usage with scroll-related props
ast-grep --pattern '<DataGrid
$$$
scroll$_={$_}
$$$
/>'
# Check for any scroll-related configuration in the component
rg -B 2 -A 2 'DataGridProps' src/components/Grid/useDataGridSource.tsx
Length of output: 1264
filter: Boolean(apiRef.current.instanceId) | ||
? convertMuiFiltersToApi( | ||
apiRef.current, | ||
view.filterModel, | ||
initialInputRef.current?.filter | ||
) | ||
: initialFilterModel, |
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
Remove redundant Boolean cast
As flagged by static analysis, the Boolean cast is redundant since the expression will already be coerced to a boolean.
Apply this change:
- filter: Boolean(apiRef.current.instanceId)
+ filter: !!apiRef.current.instanceId
? convertMuiFiltersToApi(
apiRef.current,
view.filterModel,
initialInputRef.current?.filter
)
: initialFilterModel,
📝 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.
filter: Boolean(apiRef.current.instanceId) | |
? convertMuiFiltersToApi( | |
apiRef.current, | |
view.filterModel, | |
initialInputRef.current?.filter | |
) | |
: initialFilterModel, | |
filter: !!apiRef.current.instanceId | |
? convertMuiFiltersToApi( | |
apiRef.current, | |
view.filterModel, | |
initialInputRef.current?.filter | |
) | |
: initialFilterModel, |
🧰 Tools
🪛 Biome
[error] 292-292: Avoid redundant Boolean
call
It is not necessary to use Boolean
call when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant Boolean
call
(lint/complexity/noExtraBooleanCast)
Monday
Summary by CodeRabbit
Release Notes
New Features
ProgressReportsToolbar
for improved interaction in theProgressReportsExpandedGrid
.ProgressReportsGrid
to manage its state through session storage, improving configurability.Bug Fixes
Documentation