-
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
Refacto scroll + Aggregate queries for view groups #9089
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 refactors scroll management to prevent table-wide re-renders when opening dropdowns, while also adding aggregate query functionality for view groups.
- Replaces global scroll states with component-scoped states using
ScrollWrapperComponentInstanceContext
and newscrollWrapper*ComponentState
implementations - Adds
useRecordGroupFilter
hook to handle record filtering in aggregate queries for view groups - Removes
aggregateOperation
from column definitions in favor of a new implementation for aggregate queries - Adds unique
componentInstanceId
props to allScrollWrapper
instances for isolated scroll state management - Introduces
useToggleScrollWrapper
hook for granular control over scroll behavior when dropdowns are opened
47 file(s) reviewed, 21 comment(s)
Edit PR Review Bot Settings | Greptile
packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupFilter.ts
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupFilter.ts
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/object-record/record-table/components/RecordTable.tsx
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/object-record/record-table/components/RecordTable.tsx
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/ui/utilities/scroll/hooks/useToggleScrollWrapper.ts
Show resolved
Hide resolved
packages/twenty-front/src/modules/ui/utilities/scroll/hooks/useToggleScrollWrapper.ts
Show resolved
Hide resolved
packages/twenty-front/src/modules/ui/utilities/scroll/states/scrollPositionState.ts
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/ui/utilities/scroll/states/overlayScrollbarsState.ts
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/views/utils/mapColumnDefinitionToViewField.ts
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.
LGTM
- Fixed icon shrinking on tabs. The introduction of EllipsisDisplay takes 100% of the width, thus shrinking the icons. - Removed scroll wrapper tablist. This was removed in #9016 but reintroduced in #9089. This reintroduction made the dark border below the active tab disappear. - Used Avatar for icon and logo rendering following the changes made in #9093
Closes https://github.com/twentyhq/private-issues/issues/217.
Refactoring scroll not to cause table-wide re-render when opening a dropdown (triggering a scroll lock) in the table.