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.

Sign up for GitHub

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

Jump to bottom

useViewportData - re-subscribes to table any time ViewportData changes #2003

Closed
bmingles opened this issue May 10, 2024 · 0 comments · Fixed by #2008
Closed

useViewportData - re-subscribes to table any time ViewportData changes #2003

bmingles opened this issue May 10, 2024 · 0 comments · Fixed by #2008
Assignees
@bmingles
Labels
bug Something isn't working
Milestone
May 2024

Comments

@bmingles
Copy link
Contributor

bmingles commented May 10, 2024 •
edited
Loading

useViewportData subscribes to the underlying table via useTableListener. It currently re-registers listeners any time the onTableUpdated callback changes which happens any timeviewportData changes. We should be able to get around this by passing a ref callback instead of one that changes.
https://github.com/deephaven/web-client-ui/blob/main/packages/jsapi-components/src/useViewportData.ts#L122
@bmingles bmingles added bug Something isn't working triage Issue requires triage labels May 10, 2024
@bmingles bmingles self-assigned this May 10, 2024
This was referenced May 10, 2024
Merged
Merged
@bmingles bmingles changed the title useViewportData re-subscribes to table any time ViewportData changes useViewportData - re-subscribes to table any time ViewportData changes May 13, 2024
@vbabich vbabich added this to the May 2024 milestone May 14, 2024
@vbabich vbabich removed the triage Issue requires triage label May 14, 2024
bmingles added a commit that referenced this issue May 14, 2024
@bmingles
fix: useViewportData - memoize subscriptions and first row of viewport (
2246a4a 
#2008)

fixes #2003 - useViewportData - re-subscribes to table any time
ViewportData changes
- Memoized `dh.Table.EVENT_SIZECHANGED` handler
- Memoized `dh.Table.EVENT_UPDATED` handler

fixes #1928 - useViewportData - setViewport should maintain last
firstRow value when table size changes
- Track last set "first row" to be re-used when table size changes but
table reference remains the same

**Tested**
- Ticking table 
- table update subscriptions no longer occur on each viewportData change
   - size change handler no longer re-subscribes on every render
- ACL Editor - scrolling viewport data still works. Filtering still
properly resets viewport to zero
@mofojed mofojed mentioned this issue May 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bmingles bmingles

Labels
bug Something isn't working
Projects
None yet
Milestone
May 2024
Development

Successfully merging a pull request may close this issue.

fix: useViewportData - memoize subscriptions and first row of viewport bmingles/web-client-ui
2 participants
@bmingles @vbabich