-
Notifications
You must be signed in to change notification settings - Fork 8
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
Table virtualization #966
Table virtualization #966
Conversation
…ple app, tweak large sample data
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.
First round of feedback is mostly based on testing the demo page in a browser; I haven't looked at the implementation yet but wanted to get my initial feedback sent.
angular-workspace/projects/example-client-app/src/app/tabledemo/tabledemo.component.ts
Outdated
Show resolved
Hide resolved
angular-workspace/projects/example-client-app/src/app/customapp/customapp.component.html
Outdated
Show resolved
Hide resolved
angular-workspace/projects/example-client-app/src/app/customapp/customapp.component.html
Outdated
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.
Performance feels great in Firefox and Safari on macOS 🚀
Because there are conflicts with main, I'm going to wait to re-review until you've updated & resolved. |
Pull Request
🤨 Rationale
Related Issue:
👩💻 Implementation
Uses TanStack Virtualizer as mentioned in the spec, and as done in the prototype branch.
Major differences from the prototype:
ResizeObserver
in the Nimble table code. TanStack Virtualizer is already using a ResizeObserver internally (due to us using its providedobserveElementOffset
andobserveElementRect
functions as inputs in ourVirtualizerOptions
object), so I concluded we didn't need a separate resize observer.🧪 Testing
✅ Checklist