Skip to content
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

Clean up object record filter #9550

Merged
merged 4 commits into from
Jan 10, 2025
Merged

Conversation

lucasbordeau
Copy link
Contributor

Last clean up of object dropdown filter states v1.

Removed old state scope context component.

@lucasbordeau lucasbordeau force-pushed the refactor/object-filter-states-3 branch from 885ffd7 to f0074a2 Compare January 10, 2025 16:57
@lucasbordeau lucasbordeau enabled auto-merge (squash) January 10, 2025 17:09
Copy link
Contributor

@greptile-apps greptile-apps bot left a 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 refactors the object filter dropdown state management across the application, moving from a scope-based context approach to direct Recoil component state management.

  • Removed ObjectFilterDropdownScope component and associated hooks (useFilterDropdown, useFilterDropdownStates, useSetFilterDefinitionUsedInDropdownInScope) in favor of direct ObjectFilterDropdownComponentInstanceContext.Provider
  • Replaced useRecoilValue with useRecoilComponentValueV2 for more granular state management across filter components
  • Migrated filter state management from centralized hooks to individual component-level Recoil states
  • Standardized state access patterns by using component-specific state atoms instead of shared hooks
  • Simplified component hierarchy by removing redundant context providers while maintaining functionality

35 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines 39 to 40
<AdvancedFilterViewFilterFieldSelect viewFilterId={filter.id} />
<AdvancedFilterViewFilterOperandSelect viewFilterId={filter.id} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: using filter.id here but viewFilterId was passed as prop - ensure these IDs are always equivalent

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Deleting core state management code. Make sure there are replacement mechanisms in place for all these filter states to maintain functionality.

newFilter.id,
filterDefinition,
);
selectFilterDefinitionUsedInDropdown({ filterDefinition });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: selectFilterDefinitionUsedInDropdown is called before checking if it succeeded

@charlesBochet charlesBochet merged commit ed51bff into main Jan 10, 2025
21 of 23 checks passed
@charlesBochet charlesBochet deleted the refactor/object-filter-states-3 branch January 10, 2025 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants