-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fix broken relation picker in Kanban #8377
Conversation
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.
PR Summary
This PR fixes re-rendering issues in the Kanban view's relation picker by optimizing state management and component structure to allow proper search input functionality.
- Moved search filter initialization to new
SearchPickerInitialValueEffect
component to prevent unwanted re-renders ofRecordBoardCard
- Simplified
useRelationPicker
hook in/hooks/useRelationPicker.ts
by removing unnecessary Recoil state dependencies - Added debug logging in
/hooks/useEntitySelectSearch.ts
to track search filter changes - Modified
RelationPicker
component to use the new effect-based initialization pattern
4 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile
useEffect(() => { | ||
setRelationPickerSearchFilter(initialValueForSearchFilter ?? ''); | ||
}, [initialValueForSearchFilter, setRelationPickerSearchFilter]); |
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.
logic: This effect will run on every relationPickerScopeId change since getRelationPickerScopedStates is called in render. Consider memoizing the state or adding relationPickerScopeId to effect deps.
setRelationPickerSearchFilter(initialValueForSearchFilter ?? ''); | ||
}, [initialValueForSearchFilter, setRelationPickerSearchFilter]); | ||
|
||
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.
style: Empty fragment is unnecessary since this is an effect-only component. Consider returning null instead.
relationPickerScopeId: string; | ||
}) => { | ||
const { relationPickerSearchFilterState } = getRelationPickerScopedStates({ | ||
relationPickerScopeId: relationPickerScopeId, |
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.
style: Redundant object property name. Can be shortened to just relationPickerScopeId
@@ -26,6 +26,7 @@ export const useEntitySelectSearch = ({ | |||
const handleSearchFilterChange = ( | |||
event: React.ChangeEvent<HTMLInputElement>, | |||
) => { | |||
console.log('set search filter', event.currentTarget.value); |
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.
style: Remove debug console.log before merging to production
const { relationPickerSearchFilterState, relationPickerPreselectedIdState } = | ||
useRelationPickerScopedStates({ | ||
relationPickerScopedId: scopeId, | ||
}); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Fixes #8233
Typing a search input was triggering a re-render of the whole RecordBoardCard, resetting the search input to its initial value, an empty string, making it impossible to actually type anything.
useRelationPicker had (legacy?) useless dependencies to recoil states that caused the rerenders.