Auto-scroll the search result list to keep the selection in view #35
Conversation
|
The downwards scroll looks very weird, especially if the next selection doesn't seem like it will go off the screen. Perhaps try calculating the position of the next selection before scrolling?
|
src/Searchbar.vue
Outdated
| this.scrollWindow(); | ||
| } | ||
| }, | ||
| scrollWindow() { |
There was a problem hiding this comment.
scrollListView() might be a bit more accurate. (I think the term "window" might be more suitable for the entire browser's window, rather than the component's window).
src/Searchbar.vue
Outdated
| }, | ||
| scrollWindow() { | ||
| const { dropdown } = this.$els; | ||
| const curEntry = dropdown.children[this.current]; |
There was a problem hiding this comment.
Try to spell out the abbreviation. Should be currentEntry.
src/Searchbar.vue
Outdated
| const curEntry = dropdown.children[this.current]; | ||
| const upperBound = dropdown.scrollTop; | ||
| const lowerBound = upperBound + dropdown.clientHeight; | ||
| const curBottom = curEntry.offsetTop + curEntry.offsetHeight; |
There was a problem hiding this comment.
I think you should just name it literally to what it should have been provided by them: currentEntryOffsetBottom. Makes it more consistent.
src/Searchbar.vue
Outdated
| }, | ||
| scrollListView() { | ||
| const { dropdown } = this.$els; | ||
| const curEntry = dropdown.children[this.current]; |
There was a problem hiding this comment.
Try to spell out the abbreviation. Should be
currentEntry.
Reminder to fix this one. :P
src/Searchbar.vue
Outdated
| if (curEntry.offsetTop < upperBound) { | ||
| dropdown.scrollTop = curEntry.offsetTop; | ||
| } else if (currentEntryOffsetBottom > lowerBound) { | ||
| dropdown.scrollTop = (upperBound + currentEntryOffsetBottom) - lowerBound; |
There was a problem hiding this comment.
This can be simplified to just dropdown.scrollTop = currentEntryOffsetBottom - dropdown.clientHeight;
|
@nusjzx Can you update the deployed website? |
|
done @ |
|
Please squash the commits. |
|
|
nusjzx
force-pushed
the
258-search-dropdown-autoscroll
branch
from
fffcade
to
48f8c18
Compare
What is the purpose of this pull request? (put "X" next to an item, remove the rest)
• [ ] Documentation update
• [ ] Bug fix
• [ ] New feature
• [X] Enhancement to an existing feature
• [ ] Other, please explain:
Resolves MarkBind/markbind#258
What is the rationale for this request?
Currently, when the user navigates down the result list using the down arrow, the selection goes out of the view port of the list. The dropdown should auto-scroll to the entry
What changes did you make? (Give an overview)
update up and down event handler to scroll to current item
Testing instructions: