-
Notifications
You must be signed in to change notification settings - Fork 33
feat: DH-18960: Improve column selection functionality for large tables #2555
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
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2555 +/- ##
==========================================
- Coverage 44.34% 44.28% -0.06%
==========================================
Files 764 768 +4
Lines 43043 43367 +324
Branches 11049 10973 -76
==========================================
+ Hits 19086 19206 +120
- Misses 23941 24145 +204
Partials 16 16
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dsmmcken
left a comment
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.
- "select matching" not scrolling into view first selected option
- "select matching" does not unhide selection
- enter key doesn't select first item
Thoughts on should the column order in the popover be sorted or something?
Weird state where menu doesn't close:
- open popover
- click focus out of browser window to desktop
- click focus back onto window, by clicking onto table
- drag an item, and menu is stuck open
packages/iris-grid/src/sidebar/visibility-ordering-builder/SearchItem.tsx
Outdated
Show resolved
Hide resolved
|
One small issue: when I drop the item after dragging, it transitions back to its original position with a fade-out, not where I'm dropping it. dnd.mov |
|
@vbabich I also noticed that, but checked without this change and it seems to be an issue caused by the React 18 upgrade. Couldn't find anything on the dnd-kit GitHub about it |
dsmmcken
left a comment
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.
Approved for behaviour.
Things I tested
Probably some other things I'm forgetting, but it should be pretty robust