feat: forward and back buttons for organize column search#1620
feat: forward and back buttons for organize column search#1620ethanalvizo merged 15 commits intodeephaven:mainfrom
Conversation
| treeItems | ||
| ); | ||
|
|
||
| const queryParams = { |
There was a problem hiding this comment.
i thought this would be nicer to pass in to SearchInput rather than creating two new optional props that would only be used by VisibilityOrderingBuilder. Could revert to that if preferred though
| * @param direction The direction to move the selection | ||
| */ | ||
|
|
||
| changeSelectedColumn(direction: 'forward' | 'back'): void { |
There was a problem hiding this comment.
i feel this could use a better name but not too sure what. This does change the selected column as the name implies but only from the previously queried list
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1620 +/- ##
==========================================
- Coverage 46.63% 46.62% -0.02%
==========================================
Files 591 592 +1
Lines 36406 36516 +110
Branches 9113 9154 +41
==========================================
+ Hits 16979 17026 +47
- Misses 19375 19438 +63
Partials 52 52
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
dsmmcken
left a comment
dsmmcken
left a comment
There was a problem hiding this comment.
The column search going back to the selected item, instead of picking up from your current selection seems counter intuitive to most find interfaces. To wit, if I search for “log” in intellij; hit the button a few times to cycle; then select something else with my mouse and hit the button again it picks up the search from my cursor not the “n / m” I was on.
Arrows should go to the previous/next one before/after a manually selected index:
ex.
Search A, click BB, hit next get AC.
AA
AB
BB
AC
AD
|
Scroll into view behaviour doesn't appear to be functioning correctly.
|
|
My guess is a react re-render is interrupting the scroll. |
fixed in the latest version now. It wasn't a re-render. My newest changes just broke the previous way I was passing in which item to focus on. All good now though! |
dsmmcken
left a comment
dsmmcken
left a comment
There was a problem hiding this comment.
Approved for functionality, didn't code review.
2 participants
Closes #1529