-
Notifications
You must be signed in to change notification settings - Fork 32
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
feat: forward and back buttons for organize column search #1620
feat: forward and back buttons for organize column search #1620
Conversation
@@ -1049,6 +1109,14 @@ class VisibilityOrderingBuilder extends Component< | |||
treeItems | |||
); | |||
|
|||
const queryParams = { |
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
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.
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
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! |
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 functionality, didn't code review.
Closes #1529