fix: reset table selection when navigating away from index page #9140
Conversation
- Add RecordIndexResetSelectionEffect component to handle selection cleanup - Integrate effect into RecordIndexPage - Ensures checkbox selections are cleared when navigating away
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Adds a new cleanup effect component to reset table row selections when navigating away from index pages, fixing the issue of persisting selections between page navigations.
- Added new
packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexResetSelectionEffect.tsxto handle selection cleanup on unmount - Integrated
RecordIndexResetSelectionEffectintoRecordIndexPage.tsxwithin the component hierarchy - Uses
useResetTableRowSelectionhook to clear row selections and close action menu dropdown - Leverages React's useEffect cleanup function for proper selection state management
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
Welcome!
Hello there, congrats on your first PR! We're excited to have you contributing to this project. |
|
@samyakpiya I have approved first but we actually would like to test a different pattern. We are trying to get rid of the useEffects completely on the codebase The right way to do that is to let the caller synchronously reset the table selection. But how could the link in RecordDetailSectionHeader know that it should reset the table selection: let's introduce a component that will include a <Link to={...} onClick{resetTableSelection} /> |
|
@charlesBochet Thanks for the feedback! I understand we want to move away from useEffect and handle the selection reset synchronously through the Link component's onClick. I'll update the PR to implement this pattern instead. |
|
@charlesBochet I've considered the pattern you suggested but have some technical concerns to discuss. The current suggestion to handle reset via
Additionally, implementing this through Links would require lifting state management up the component tree to make the reset function available to both index and show pages. Would it make more sense to handle this at the routing/navigation level instead? This would ensure consistent behavior regardless of how the user leaves the index page. I'm happy to explore alternative patterns that don't use useEffect while still covering all these edge cases. |
Fixes #9132
Purpose
Currently, when navigating away from an index page and returning, previously checked selections remain checked. This fix ensures the selection context is properly reset on navigation.
Changes
RecordIndexResetSelectionEffectcomponent to handle selection cleanupRecordIndexPageuseResetTableRowSelectionhook to clear selectionsTesting
Demo
I've recorded a short video demonstrating how this PR fixes the issue:
Loom Video Link
Before fix: Selections persist after navigation
After fix: Selections are properly reset